From 80b7a9f1b5ba5a6117f6b020121f06a2d0cd1889 Mon Sep 17 00:00:00 2001 From: Erik Kline Date: Wed, 28 Feb 2018 15:01:35 +0900 Subject: Don't unnecessarily reevaluate tethering provisioning Registering for carrier config changes can deliver a sticky broadcast and can cause Tethering to think something has changed and reevaluate provisioning status, even though this has been checked before it entered tethering mode alive state. Additionally, move the provisioning_app{,no_ui} resources into the TetheringConfiguration, if for no other reason than now we can log it in .toString(). Test: as follows - built - flashed - booted - runtest frameworks-net passes - manual USB tethering toward WiFi works Bug: 69565814 Change-Id: Ib8b2620ce44c55e5eb0afd3f00f3f5aa4fc8a593 (cherry picked from commit 8067d78c32f545db6d35c660279ab3f2326ba41d) --- .../com/android/server/connectivity/Tethering.java | 56 ++++++++----------- .../tethering/TetheringConfiguration.java | 62 ++++++++++++++++++---- .../android/server/connectivity/TetheringTest.java | 8 +++ 3 files changed, 82 insertions(+), 44 deletions(-) diff --git a/services/core/java/com/android/server/connectivity/Tethering.java b/services/core/java/com/android/server/connectivity/Tethering.java index d37dd1858acd..df6a6f8bdcc1 100644 --- a/services/core/java/com/android/server/connectivity/Tethering.java +++ b/services/core/java/com/android/server/connectivity/Tethering.java @@ -249,6 +249,7 @@ public class Tethering extends BaseNetworkObserver { "CarrierConfigChangeListener", mContext, smHandler, filter, (Intent ignored) -> { mLog.log("OBSERVED carrier config change"); + updateConfiguration(); reevaluateSimCardProvisioning(); }); // TODO: Remove SimChangeListener altogether. For now, we retain it @@ -261,28 +262,35 @@ public class Tethering extends BaseNetworkObserver { }); mStateReceiver = new StateReceiver(); - filter = new IntentFilter(); + + // Load tethering configuration. + updateConfiguration(); + + startStateMachineUpdaters(); + } + + private void startStateMachineUpdaters() { + mCarrierConfigChange.startListening(); + + final Handler handler = mTetherMasterSM.getHandler(); + IntentFilter filter = new IntentFilter(); filter.addAction(UsbManager.ACTION_USB_STATE); filter.addAction(CONNECTIVITY_ACTION); filter.addAction(WifiManager.WIFI_AP_STATE_CHANGED_ACTION); filter.addAction(Intent.ACTION_CONFIGURATION_CHANGED); - mContext.registerReceiver(mStateReceiver, filter, null, smHandler); + mContext.registerReceiver(mStateReceiver, filter, null, handler); filter = new IntentFilter(); filter.addAction(Intent.ACTION_MEDIA_SHARED); filter.addAction(Intent.ACTION_MEDIA_UNSHARED); filter.addDataScheme("file"); - mContext.registerReceiver(mStateReceiver, filter, null, smHandler); + mContext.registerReceiver(mStateReceiver, filter, null, handler); - UserManagerInternal userManager = LocalServices.getService(UserManagerInternal.class); - - // this check is useful only for some unit tests; example: ConnectivityServiceTest - if (userManager != null) { - userManager.addUserRestrictionsListener(new TetheringUserRestrictionListener(this)); + final UserManagerInternal umi = LocalServices.getService(UserManagerInternal.class); + // This check is useful only for some unit tests; example: ConnectivityServiceTest. + if (umi != null) { + umi.addUserRestrictionsListener(new TetheringUserRestrictionListener(this)); } - - // load device config info - updateConfiguration(); } private WifiManager getWifiManager() { @@ -384,17 +392,15 @@ public class Tethering extends BaseNetworkObserver { */ @VisibleForTesting protected boolean isTetherProvisioningRequired() { - String[] provisionApp = mContext.getResources().getStringArray( - com.android.internal.R.array.config_mobile_hotspot_provision_app); + final TetheringConfiguration cfg = mConfig; if (mSystemProperties.getBoolean(DISABLE_PROVISIONING_SYSPROP_KEY, false) - || provisionApp == null) { + || cfg.provisioningApp.length == 0) { return false; } - if (carrierConfigAffirmsEntitlementCheckNotRequired()) { return false; } - return (provisionApp.length == 2); + return (cfg.provisioningApp.length == 2); } // The logic here is aimed solely at confirming that a CarrierConfig exists @@ -417,20 +423,6 @@ public class Tethering extends BaseNetworkObserver { return !isEntitlementCheckRequired; } - // Used by the SIM card change observation code. - // TODO: De-duplicate above code. - private boolean hasMobileHotspotProvisionApp() { - try { - if (!mContext.getResources().getString(com.android.internal.R.string. - config_mobile_hotspot_provision_app_no_ui).isEmpty()) { - Log.d(TAG, "re-evaluate provisioning"); - return true; - } - } catch (Resources.NotFoundException e) {} - Log.d(TAG, "no prov-check needed for new SIM"); - return false; - } - /** * Enables or disables tethering for the given type. This should only be called once * provisioning has succeeded or is not necessary. It will also schedule provisioning rechecks @@ -1187,7 +1179,7 @@ public class Tethering extends BaseNetworkObserver { } private void reevaluateSimCardProvisioning() { - if (!hasMobileHotspotProvisionApp()) return; + if (!mConfig.hasMobileHotspotProvisionApp()) return; if (carrierConfigAffirmsEntitlementCheckNotRequired()) return; ArrayList tethered = new ArrayList<>(); @@ -1546,7 +1538,6 @@ public class Tethering extends BaseNetworkObserver { return; } - mCarrierConfigChange.startListening(); mSimChange.startListening(); mUpstreamNetworkMonitor.start(); @@ -1564,7 +1555,6 @@ public class Tethering extends BaseNetworkObserver { mOffload.stop(); mUpstreamNetworkMonitor.stop(); mSimChange.stopListening(); - mCarrierConfigChange.stopListening(); notifyDownstreamsOfNewUpstreamIface(null); handleNewUpstreamNetworkState(null); } diff --git a/services/core/java/com/android/server/connectivity/tethering/TetheringConfiguration.java b/services/core/java/com/android/server/connectivity/tethering/TetheringConfiguration.java index 09bce7f4feb6..454c579ed0a7 100644 --- a/services/core/java/com/android/server/connectivity/tethering/TetheringConfiguration.java +++ b/services/core/java/com/android/server/connectivity/tethering/TetheringConfiguration.java @@ -21,14 +21,23 @@ import static android.net.ConnectivityManager.TYPE_ETHERNET; import static android.net.ConnectivityManager.TYPE_MOBILE; import static android.net.ConnectivityManager.TYPE_MOBILE_DUN; import static android.net.ConnectivityManager.TYPE_MOBILE_HIPRI; +import static com.android.internal.R.array.config_mobile_hotspot_provision_app; +import static com.android.internal.R.array.config_tether_bluetooth_regexs; +import static com.android.internal.R.array.config_tether_dhcp_range; +import static com.android.internal.R.array.config_tether_usb_regexs; +import static com.android.internal.R.array.config_tether_upstream_types; +import static com.android.internal.R.array.config_tether_wifi_regexs; +import static com.android.internal.R.string.config_mobile_hotspot_provision_app_no_ui; import android.content.Context; import android.content.res.Resources; import android.net.ConnectivityManager; -import android.telephony.TelephonyManager; import android.net.util.SharedLog; +import android.telephony.TelephonyManager; +import android.text.TextUtils; import com.android.internal.annotations.VisibleForTesting; +import com.android.internal.R; import java.io.PrintWriter; import java.util.ArrayList; @@ -51,6 +60,8 @@ import java.util.StringJoiner; public class TetheringConfiguration { private static final String TAG = TetheringConfiguration.class.getSimpleName(); + private static final String[] EMPTY_STRING_ARRAY = new String[0]; + @VisibleForTesting public static final int DUN_NOT_REQUIRED = 0; public static final int DUN_REQUIRED = 1; @@ -79,18 +90,18 @@ public class TetheringConfiguration { public final String[] dhcpRanges; public final String[] defaultIPv4DNS; + public final String[] provisioningApp; + public final String provisioningAppNoUi; + public TetheringConfiguration(Context ctx, SharedLog log) { final SharedLog configLog = log.forSubComponent("config"); - tetherableUsbRegexs = ctx.getResources().getStringArray( - com.android.internal.R.array.config_tether_usb_regexs); + tetherableUsbRegexs = getResourceStringArray(ctx, config_tether_usb_regexs); // TODO: Evaluate deleting this altogether now that Wi-Fi always passes // us an interface name. Careful consideration needs to be given to // implications for Settings and for provisioning checks. - tetherableWifiRegexs = ctx.getResources().getStringArray( - com.android.internal.R.array.config_tether_wifi_regexs); - tetherableBluetoothRegexs = ctx.getResources().getStringArray( - com.android.internal.R.array.config_tether_bluetooth_regexs); + tetherableWifiRegexs = getResourceStringArray(ctx, config_tether_wifi_regexs); + tetherableBluetoothRegexs = getResourceStringArray(ctx, config_tether_bluetooth_regexs); dunCheck = checkDunRequired(ctx); configLog.log("DUN check returned: " + dunCheckString(dunCheck)); @@ -101,6 +112,9 @@ public class TetheringConfiguration { dhcpRanges = getDhcpRanges(ctx); defaultIPv4DNS = copy(DEFAULT_IPV4_DNS); + provisioningApp = getResourceStringArray(ctx, config_mobile_hotspot_provision_app); + provisioningAppNoUi = getProvisioningAppNoUi(ctx); + configLog.log(toString()); } @@ -116,6 +130,10 @@ public class TetheringConfiguration { return matchesDownstreamRegexs(iface, tetherableBluetoothRegexs); } + public boolean hasMobileHotspotProvisionApp() { + return !TextUtils.isEmpty(provisioningAppNoUi); + } + public void dump(PrintWriter pw) { dumpStringArray(pw, "tetherableUsbRegexs", tetherableUsbRegexs); dumpStringArray(pw, "tetherableWifiRegexs", tetherableWifiRegexs); @@ -129,6 +147,10 @@ public class TetheringConfiguration { dumpStringArray(pw, "dhcpRanges", dhcpRanges); dumpStringArray(pw, "defaultIPv4DNS", defaultIPv4DNS); + + dumpStringArray(pw, "provisioningApp", provisioningApp); + pw.print("provisioningAppNoUi: "); + pw.println(provisioningAppNoUi); } public String toString() { @@ -140,6 +162,8 @@ public class TetheringConfiguration { sj.add(String.format("isDunRequired:%s", isDunRequired)); sj.add(String.format("preferredUpstreamIfaceTypes:%s", makeString(preferredUpstreamNames(preferredUpstreamIfaceTypes)))); + sj.add(String.format("provisioningApp:%s", makeString(provisioningApp))); + sj.add(String.format("provisioningAppNoUi:%s", provisioningAppNoUi)); return String.format("TetheringConfiguration{%s}", sj.toString()); } @@ -159,6 +183,7 @@ public class TetheringConfiguration { } private static String makeString(String[] strings) { + if (strings == null) return "null"; final StringJoiner sj = new StringJoiner(",", "[", "]"); for (String s : strings) sj.add(s); return sj.toString(); @@ -195,8 +220,7 @@ public class TetheringConfiguration { } private static Collection getUpstreamIfaceTypes(Context ctx, int dunCheck) { - final int ifaceTypes[] = ctx.getResources().getIntArray( - com.android.internal.R.array.config_tether_upstream_types); + final int ifaceTypes[] = ctx.getResources().getIntArray(config_tether_upstream_types); final ArrayList upstreamIfaceTypes = new ArrayList<>(ifaceTypes.length); for (int i : ifaceTypes) { switch (i) { @@ -247,14 +271,30 @@ public class TetheringConfiguration { } private static String[] getDhcpRanges(Context ctx) { - final String[] fromResource = ctx.getResources().getStringArray( - com.android.internal.R.array.config_tether_dhcp_range); + final String[] fromResource = getResourceStringArray(ctx, config_tether_dhcp_range); if ((fromResource.length > 0) && (fromResource.length % 2 == 0)) { return fromResource; } return copy(DHCP_DEFAULT_RANGE); } + private static String getProvisioningAppNoUi(Context ctx) { + try { + return ctx.getResources().getString(config_mobile_hotspot_provision_app_no_ui); + } catch (Resources.NotFoundException e) { + return ""; + } + } + + private static String[] getResourceStringArray(Context ctx, int resId) { + try { + final String[] strArray = ctx.getResources().getStringArray(resId); + return (strArray != null) ? strArray : EMPTY_STRING_ARRAY; + } catch (Resources.NotFoundException e404) { + return EMPTY_STRING_ARRAY; + } + } + private static String[] copy(String[] strarray) { return Arrays.copyOf(strarray, strarray.length); } diff --git a/tests/net/java/com/android/server/connectivity/TetheringTest.java b/tests/net/java/com/android/server/connectivity/TetheringTest.java index 6142a7ce6bde..d643c69c0e44 100644 --- a/tests/net/java/com/android/server/connectivity/TetheringTest.java +++ b/tests/net/java/com/android/server/connectivity/TetheringTest.java @@ -355,6 +355,7 @@ public class TetheringTest { @Test public void canRequireProvisioning() { setupForRequiredProvisioning(); + sendConfigurationChanged(); assertTrue(mTethering.isTetherProvisioningRequired()); } @@ -363,6 +364,7 @@ public class TetheringTest { setupForRequiredProvisioning(); when(mContext.getSystemService(Context.CARRIER_CONFIG_SERVICE)) .thenReturn(null); + sendConfigurationChanged(); // Couldn't get the CarrierConfigManager, but still had a declared provisioning app. // We therefore still require provisioning. assertTrue(mTethering.isTetherProvisioningRequired()); @@ -372,6 +374,7 @@ public class TetheringTest { public void toleratesCarrierConfigMissing() { setupForRequiredProvisioning(); when(mCarrierConfigManager.getConfig()).thenReturn(null); + sendConfigurationChanged(); // We still have a provisioning app configured, so still require provisioning. assertTrue(mTethering.isTetherProvisioningRequired()); } @@ -411,6 +414,11 @@ public class TetheringTest { mServiceContext.sendStickyBroadcastAsUser(intent, UserHandle.ALL); } + private void sendConfigurationChanged() { + final Intent intent = new Intent(Intent.ACTION_CONFIGURATION_CHANGED); + mServiceContext.sendStickyBroadcastAsUser(intent, UserHandle.ALL); + } + private void verifyInterfaceServingModeStarted() throws Exception { verify(mNMService, times(1)).getInterfaceConfig(TEST_WLAN_IFNAME); verify(mNMService, times(1)) -- cgit v1.2.3-59-g8ed1b