OSDN Git Service

Don't unnecessarily reevaluate tethering provisioning
authorErik Kline <ek@google.com>
Wed, 28 Feb 2018 06:01:35 +0000 (15:01 +0900)
committerErik Kline <ek@google.com>
Mon, 9 Apr 2018 01:28:15 +0000 (18:28 -0700)
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

services/core/java/com/android/server/connectivity/Tethering.java
services/core/java/com/android/server/connectivity/tethering/TetheringConfiguration.java
tests/net/java/com/android/server/connectivity/TetheringTest.java

index e520d0c..62fe218 100644 (file)
@@ -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<Integer> 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);
             }
index 09bce7f..454c579 100644 (file)
@@ -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<Integer> 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<Integer> 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);
     }
index 8fb87f1..d241b32 100644 (file)
@@ -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))