From e0f34033e0df61ad3227c3a1fa1d295360447dd0 Mon Sep 17 00:00:00 2001 From: Erik Kline Date: Wed, 28 Feb 2018 15:01:35 +0900 Subject: [PATCH] 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 Merged-In: If254326e892b78ef9daf620f829c1def136d695c Merged-In: I288093a1d76566e72d4889d92c7aedafc318c8b6 Merged-Id: I01c71fd971a4683bb2b6d14825d36f24a04d88a8 Change-Id: I01c71fd971a4683bb2b6d14825d36f24a04d88a8 (cherry picked from commit 1b450e3eb9106a9cc73dfc144d275141dd5017ef) --- .../server/connectivity/Tethering.java | 56 +++++++---------- .../tethering/TetheringConfiguration.java | 62 +++++++++++++++---- .../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 e520d0cba06ec..62fe218da2621 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 @@ -1186,7 +1178,7 @@ public class Tethering extends BaseNetworkObserver { } private void reevaluateSimCardProvisioning() { - if (!hasMobileHotspotProvisionApp()) return; + if (!mConfig.hasMobileHotspotProvisionApp()) return; if (carrierConfigAffirmsEntitlementCheckNotRequired()) return; ArrayList tethered = new ArrayList<>(); @@ -1545,7 +1537,6 @@ public class Tethering extends BaseNetworkObserver { return; } - mCarrierConfigChange.startListening(); mSimChange.startListening(); mUpstreamNetworkMonitor.start(); @@ -1563,7 +1554,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 09bce7f4feb67..454c579ed0a77 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 8fb87f1277a38..d241b32841cc7 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))