Add a device config for actively preferring bad wifi
Test: FrameworksNetTests
adb shell device_config put connectivity actively_prefer_bad_wifi
+ check that dumpsys updates
Test: at this patchset tests fail with :
Permission denial: reading from settings requires:android.permission.READ_DEVICE_CONFIG
Change-Id: Icfe1e64b8313921ff1dc8e4cbd45f967692e1c4a
diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java
index 1236243..84cf561 100755
--- a/service/src/com/android/server/ConnectivityService.java
+++ b/service/src/com/android/server/ConnectivityService.java
@@ -5112,22 +5112,32 @@
pw.println("Bad Wi-Fi avoidance: " + avoidBadWifi());
pw.increaseIndent();
- pw.println("Config restrict: " + configRestrict);
- pw.println("Actively prefer: " + activelyPreferBadWifi());
+ pw.println("Config restrict: " + configRestrict);
+ pw.println("Actively prefer bad wifi: " + activelyPreferBadWifi());
- final String value = mMultinetworkPolicyTracker.getAvoidBadWifiSetting();
+ final String settingValue = mMultinetworkPolicyTracker.getAvoidBadWifiSetting();
String description;
// Can't use a switch statement because strings are legal case labels, but null is not.
- if ("0".equals(value)) {
+ if ("0".equals(settingValue)) {
description = "get stuck";
- } else if (value == null) {
+ } else if (settingValue == null) {
description = "prompt";
- } else if ("1".equals(value)) {
+ } else if ("1".equals(settingValue)) {
description = "avoid";
} else {
- description = value + " (?)";
+ description = settingValue + " (?)";
}
- pw.println("User setting: " + description);
+ pw.println("Avoid bad wifi setting: " + description);
+ final Boolean configValue = mMultinetworkPolicyTracker.deviceConfigActivelyPreferBadWifi();
+ if (null == configValue) {
+ description = "unset";
+ } else if (configValue) {
+ description = "force true";
+ } else {
+ description = "force false";
+ }
+ pw.println("Actively prefer bad wifi conf: " + description);
+ pw.println();
pw.println("Network overrides:");
pw.increaseIndent();
for (NetworkAgentInfo nai : networksSortedById()) {
diff --git a/service/src/com/android/server/connectivity/MultinetworkPolicyTracker.java b/service/src/com/android/server/connectivity/MultinetworkPolicyTracker.java
index a36bf4b..58196f7 100644
--- a/service/src/com/android/server/connectivity/MultinetworkPolicyTracker.java
+++ b/service/src/com/android/server/connectivity/MultinetworkPolicyTracker.java
@@ -32,6 +32,7 @@
import android.net.Uri;
import android.os.Build;
import android.os.Handler;
+import android.provider.DeviceConfig;
import android.provider.Settings;
import android.telephony.SubscriptionManager;
import android.telephony.TelephonyCallback;
@@ -41,6 +42,7 @@
import com.android.connectivity.resources.R;
import com.android.internal.annotations.VisibleForTesting;
import com.android.modules.utils.build.SdkLevel;
+import com.android.net.module.util.DeviceConfigUtils;
import java.util.Arrays;
import java.util.List;
@@ -67,6 +69,9 @@
public class MultinetworkPolicyTracker {
private static String TAG = MultinetworkPolicyTracker.class.getSimpleName();
+ // See Dependencies#getConfigActivelyPreferBadWifi
+ public static final String CONFIG_ACTIVELY_PREFER_BAD_WIFI = "actively_prefer_bad_wifi";
+
private final Context mContext;
private final ConnectivityResources mResources;
private final Handler mHandler;
@@ -82,6 +87,43 @@
private volatile long mTestAllowBadWifiUntilMs = 0;
/**
+ * Dependencies for testing
+ */
+ @VisibleForTesting
+ public static class Dependencies {
+ /**
+ * @see DeviceConfigUtils#getDeviceConfigPropertyInt
+ */
+ protected int getConfigActivelyPreferBadWifi() {
+ // CONFIG_ACTIVELY_PREFER_BAD_WIFI is not a feature to be rolled out, but an override
+ // for tests and an emergency kill switch (which could force the behavior on OR off).
+ // As such it uses a -1/null/1 scheme, but features should use
+ // DeviceConfigUtils#isFeatureEnabled instead, to make sure rollbacks disable the
+ // feature before it's ready on R and before.
+ return DeviceConfig.getInt(DeviceConfig.NAMESPACE_CONNECTIVITY,
+ CONFIG_ACTIVELY_PREFER_BAD_WIFI, 0);
+ }
+
+ /**
+ @see DeviceConfig#addOnPropertiesChangedListener
+ */
+ protected void addOnDevicePropertiesChangedListener(@NonNull final Executor executor,
+ @NonNull final DeviceConfig.OnPropertiesChangedListener listener) {
+ DeviceConfig.addOnPropertiesChangedListener(DeviceConfig.NAMESPACE_CONNECTIVITY,
+ executor, listener);
+ }
+
+ @VisibleForTesting
+ @NonNull
+ protected Resources getResourcesForActiveSubId(
+ @NonNull final ConnectivityResources resources, final int activeSubId) {
+ return SubscriptionManager.getResourcesForSubId(
+ resources.getResourcesContext(), activeSubId);
+ }
+ }
+ private final Dependencies mDeps;
+
+ /**
* Whether to prefer bad wifi to a network that yields to bad wifis, even if it never validated
*
* This setting only makes sense if the system is configured not to avoid bad wifis, i.e.
@@ -129,15 +171,17 @@
}
}
- public MultinetworkPolicyTracker(Context ctx, Handler handler) {
- this(ctx, handler, null);
+ public MultinetworkPolicyTracker(Context ctx, Handler handler, Runnable avoidBadWifiCallback) {
+ this(ctx, handler, avoidBadWifiCallback, new Dependencies());
}
- public MultinetworkPolicyTracker(Context ctx, Handler handler, Runnable avoidBadWifiCallback) {
+ public MultinetworkPolicyTracker(Context ctx, Handler handler, Runnable avoidBadWifiCallback,
+ Dependencies deps) {
mContext = ctx;
mResources = new ConnectivityResources(ctx);
mHandler = handler;
mAvoidBadWifiCallback = avoidBadWifiCallback;
+ mDeps = deps;
mSettingsUris = Arrays.asList(
Settings.Global.getUriFor(NETWORK_AVOID_BAD_WIFI),
Settings.Global.getUriFor(NETWORK_METERED_MULTIPATH_PREFERENCE));
@@ -166,8 +210,11 @@
mContext.registerReceiverForAllUsers(mBroadcastReceiver, intentFilter,
null /* broadcastPermission */, mHandler);
+ final Executor handlerExecutor = new HandlerExecutor(mHandler);
mContext.getSystemService(TelephonyManager.class).registerTelephonyCallback(
- new HandlerExecutor(mHandler), new ActiveDataSubscriptionIdListener());
+ handlerExecutor, new ActiveDataSubscriptionIdListener());
+ mDeps.addOnDevicePropertiesChangedListener(handlerExecutor,
+ properties -> reevaluateInternal());
reevaluate();
}
@@ -201,7 +248,7 @@
// NETWORK_AVOID_BAD_WIFI setting.
if (allowBadWifi) return true;
- return getResourcesForActiveSubId()
+ return mDeps.getResourcesForActiveSubId(mResources, mActiveSubId)
.getInteger(R.integer.config_networkAvoidBadWifi) == 0;
}
@@ -223,7 +270,7 @@
// bad wifi (only stay stuck on it if already on there). This implementation treats
// any non-0 value like 1, on the assumption that anybody setting it non-zero wants
// the newer behavior.
- return 0 != getResourcesForActiveSubId()
+ return 0 != mDeps.getResourcesForActiveSubId(mResources, mActiveSubId)
.getInteger(R.integer.config_activelyPreferBadWifi);
}
@@ -237,13 +284,6 @@
reevaluateInternal();
}
- @VisibleForTesting
- @NonNull
- protected Resources getResourcesForActiveSubId() {
- return SubscriptionManager.getResourcesForSubId(
- mResources.getResourcesContext(), mActiveSubId);
- }
-
/**
* Whether we should display a notification when wifi becomes unvalidated.
*/
@@ -255,6 +295,29 @@
return Settings.Global.getString(mResolver, NETWORK_AVOID_BAD_WIFI);
}
+ /**
+ * Returns whether device config says the device should actively prefer bad wifi.
+ *
+ * {@see #configActivelyPrefersBadWifi} for a description of what this does. This device
+ * config overrides that config overlay.
+ *
+ * @return True on Android U and above.
+ * True if device config says to actively prefer bad wifi.
+ * False if device config says not to actively prefer bad wifi.
+ * null if device config doesn't have an opinion (then fall back on the resource).
+ */
+ public Boolean deviceConfigActivelyPreferBadWifi() {
+ if (SdkLevel.isAtLeastU()) return true;
+ switch (mDeps.getConfigActivelyPreferBadWifi()) {
+ case 1:
+ return Boolean.TRUE;
+ case -1:
+ return Boolean.FALSE;
+ default:
+ return null;
+ }
+ }
+
@VisibleForTesting
public void reevaluate() {
mHandler.post(this::reevaluateInternal);
@@ -276,7 +339,12 @@
mAvoidBadWifi = settingAvoidBadWifi || !configRestrictsAvoidBadWifi();
final boolean prevActive = mActivelyPreferBadWifi;
- mActivelyPreferBadWifi = configActivelyPrefersBadWifi();
+ final Boolean deviceConfigPreferBadWifi = deviceConfigActivelyPreferBadWifi();
+ if (null == deviceConfigPreferBadWifi) {
+ mActivelyPreferBadWifi = configActivelyPrefersBadWifi();
+ } else {
+ mActivelyPreferBadWifi = deviceConfigPreferBadWifi;
+ }
return mAvoidBadWifi != prevAvoid || mActivelyPreferBadWifi != prevActive;
}
@@ -285,10 +353,8 @@
* The default (device and carrier-dependent) value for metered multipath preference.
*/
public int configMeteredMultipathPreference() {
- // TODO: use R.integer.config_networkMeteredMultipathPreference directly
- final int id = mResources.get().getIdentifier("config_networkMeteredMultipathPreference",
- "integer", mResources.getResourcesContext().getPackageName());
- return mResources.get().getInteger(id);
+ return mDeps.getResourcesForActiveSubId(mResources, mActiveSubId)
+ .getInteger(R.integer.config_networkMeteredMultipathPreference);
}
public void updateMeteredMultipathPreference() {
diff --git a/tests/integration/src/com/android/server/net/integrationtests/ConnectivityServiceIntegrationTest.kt b/tests/integration/src/com/android/server/net/integrationtests/ConnectivityServiceIntegrationTest.kt
index 5b3f37d..26b058d 100644
--- a/tests/integration/src/com/android/server/net/integrationtests/ConnectivityServiceIntegrationTest.kt
+++ b/tests/integration/src/com/android/server/net/integrationtests/ConnectivityServiceIntegrationTest.kt
@@ -211,10 +211,15 @@
doReturn(TestNetIdManager()).`when`(deps).makeNetIdManager()
doReturn(mock(BpfNetMaps::class.java)).`when`(deps).getBpfNetMaps(any(), any())
doAnswer { inv ->
- object : MultinetworkPolicyTracker(inv.getArgument(0), inv.getArgument(1),
- inv.getArgument(2)) {
- override fun getResourcesForActiveSubId() = resources
- }
+ MultinetworkPolicyTracker(inv.getArgument(0),
+ inv.getArgument(1),
+ inv.getArgument(2),
+ object : MultinetworkPolicyTracker.Dependencies() {
+ override fun getResourcesForActiveSubId(
+ connResources: ConnectivityResources,
+ activeSubId: Int
+ ) = resources
+ })
}.`when`(deps).makeMultinetworkPolicyTracker(any(), any(), any())
return deps
}
diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java
index e1208ef..7993a5c 100755
--- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java
@@ -29,7 +29,6 @@
import static android.Manifest.permission.NETWORK_SETTINGS;
import static android.Manifest.permission.NETWORK_STACK;
import static android.Manifest.permission.PACKET_KEEPALIVE_OFFLOAD;
-import static android.Manifest.permission.READ_DEVICE_CONFIG;
import static android.app.PendingIntent.FLAG_IMMUTABLE;
import static android.content.Intent.ACTION_PACKAGE_ADDED;
import static android.content.Intent.ACTION_PACKAGE_REMOVED;
@@ -366,6 +365,7 @@
import com.android.server.connectivity.ClatCoordinator;
import com.android.server.connectivity.ConnectivityFlags;
import com.android.server.connectivity.MultinetworkPolicyTracker;
+import com.android.server.connectivity.MultinetworkPolicyTrackerTestDependencies;
import com.android.server.connectivity.Nat464Xlat;
import com.android.server.connectivity.NetworkAgentInfo;
import com.android.server.connectivity.NetworkNotificationManager;
@@ -1634,12 +1634,7 @@
volatile int mConfigMeteredMultipathPreference;
WrappedMultinetworkPolicyTracker(Context c, Handler h, Runnable r) {
- super(c, h, r);
- }
-
- @Override
- protected Resources getResourcesForActiveSubId() {
- return mResources;
+ super(c, h, r, new MultinetworkPolicyTrackerTestDependencies(mResources));
}
@Override
@@ -5810,16 +5805,12 @@
public void testPreferBadWifi_doNotPrefer() throws Exception {
// Starting with U this mode is no longer supported and can't actually be tested
assumeFalse(SdkLevel.isAtLeastU());
- runAsShell(READ_DEVICE_CONFIG, () -> {
- doTestPreferBadWifi(false /* preferBadWifi */);
- });
+ doTestPreferBadWifi(false /* preferBadWifi */);
}
@Test
public void testPreferBadWifi_doPrefer() throws Exception {
- runAsShell(READ_DEVICE_CONFIG, () -> {
- doTestPreferBadWifi(true /* preferBadWifi */);
- });
+ doTestPreferBadWifi(true /* preferBadWifi */);
}
@Test
diff --git a/tests/unit/java/com/android/server/connectivity/MultinetworkPolicyTrackerTest.kt b/tests/unit/java/com/android/server/connectivity/MultinetworkPolicyTrackerTest.kt
index 8348675..b52e8a8 100644
--- a/tests/unit/java/com/android/server/connectivity/MultinetworkPolicyTrackerTest.kt
+++ b/tests/unit/java/com/android/server/connectivity/MultinetworkPolicyTrackerTest.kt
@@ -27,6 +27,7 @@
import com.android.server.connectivity.MultinetworkPolicyTracker.ActiveDataSubscriptionIdListener
import android.os.Build
import android.os.Handler
+import android.os.test.TestLooper
import android.provider.Settings
import android.telephony.SubscriptionInfo
import android.telephony.SubscriptionManager
@@ -47,7 +48,6 @@
import org.junit.runner.RunWith
import org.mockito.ArgumentCaptor
import org.mockito.ArgumentMatchers.anyInt
-import org.mockito.ArgumentMatchers.argThat
import org.mockito.Mockito.any
import org.mockito.Mockito.doCallRealMethod
import org.mockito.Mockito.doReturn
@@ -55,6 +55,8 @@
import org.mockito.Mockito.times
import org.mockito.Mockito.verify
+const val HANDLER_TIMEOUT_MS = 400
+
/**
* Tests for [MultinetworkPolicyTracker].
*
@@ -91,8 +93,11 @@
Settings.Global.putString(resolver, NETWORK_AVOID_BAD_WIFI, "1")
ConnectivityResources.setResourcesContextForTest(it)
}
- private val handler = mock(Handler::class.java)
- private val tracker = MultinetworkPolicyTracker(context, handler)
+ private val csLooper = TestLooper()
+ private val handler = Handler(csLooper.looper)
+ private val trackerDependencies = MultinetworkPolicyTrackerTestDependencies(resources)
+ private val tracker = MultinetworkPolicyTracker(context, handler,
+ null /* avoidBadWifiCallback */, trackerDependencies)
private fun assertMultipathPreference(preference: Int) {
Settings.Global.putString(resolver, NETWORK_METERED_MULTIPATH_PREFERENCE,
@@ -146,6 +151,18 @@
}
// In all cases, now the system actively prefers bad wifi
assertTrue(tracker.activelyPreferBadWifi)
+
+ // Remaining tests are only useful on T-, which support both the old and new mode.
+ if (SdkLevel.isAtLeastU()) return
+
+ doReturn(0).`when`(resources).getInteger(R.integer.config_activelyPreferBadWifi)
+ assertTrue(tracker.updateAvoidBadWifi())
+ assertFalse(tracker.activelyPreferBadWifi)
+
+ // Simulate update of device config
+ trackerDependencies.putConfigActivelyPreferBadWifi(1)
+ csLooper.dispatchAll()
+ assertTrue(tracker.activelyPreferBadWifi)
}
@Test
@@ -164,6 +181,8 @@
Settings.Global.putString(resolver, NETWORK_METERED_MULTIPATH_PREFERENCE,
MULTIPATH_PREFERENCE_PERFORMANCE.toString())
+ assertTrue(tracker.avoidBadWifi)
+
val listenerCaptor = ArgumentCaptor.forClass(
ActiveDataSubscriptionIdListener::class.java)
verify(telephonyManager, times(1))
@@ -171,10 +190,6 @@
val listener = listenerCaptor.value
listener.onActiveDataSubscriptionIdChanged(testSubId)
- // Check it get resource value with test sub id.
- verify(subscriptionManager, times(1)).getActiveSubscriptionInfo(testSubId)
- verify(context).createConfigurationContext(argThat { it.mcc == 310 && it.mnc == 210 })
-
// Check if avoidBadWifi and meteredMultipathPreference values have been updated.
assertFalse(tracker.avoidBadWifi)
assertEquals(MULTIPATH_PREFERENCE_PERFORMANCE, tracker.meteredMultipathPreference)
diff --git a/tests/unit/java/com/android/server/connectivity/MultinetworkPolicyTrackerTestDependencies.kt b/tests/unit/java/com/android/server/connectivity/MultinetworkPolicyTrackerTestDependencies.kt
new file mode 100644
index 0000000..744c020
--- /dev/null
+++ b/tests/unit/java/com/android/server/connectivity/MultinetworkPolicyTrackerTestDependencies.kt
@@ -0,0 +1,47 @@
+package com.android.server.connectivity
+
+import android.content.res.Resources
+import android.net.ConnectivityResources
+import android.provider.DeviceConfig
+import android.provider.DeviceConfig.NAMESPACE_CONNECTIVITY
+import android.provider.DeviceConfig.OnPropertiesChangedListener
+import com.android.internal.annotations.GuardedBy
+import com.android.server.connectivity.MultinetworkPolicyTracker.CONFIG_ACTIVELY_PREFER_BAD_WIFI
+import java.util.concurrent.Executor
+
+class MultinetworkPolicyTrackerTestDependencies(private val resources: Resources) :
+ MultinetworkPolicyTracker.Dependencies() {
+ @GuardedBy("listeners")
+ private var configActivelyPreferBadWifi = 0
+ // TODO : move this to an actual fake device config object
+ @GuardedBy("listeners")
+ private val listeners = mutableListOf<Pair<Executor, OnPropertiesChangedListener>>()
+
+ fun putConfigActivelyPreferBadWifi(value: Int) {
+ synchronized(listeners) {
+ if (value == configActivelyPreferBadWifi) return
+ configActivelyPreferBadWifi = value
+ val p = DeviceConfig.Properties(NAMESPACE_CONNECTIVITY,
+ mapOf(CONFIG_ACTIVELY_PREFER_BAD_WIFI to value.toString()))
+ listeners.forEach { (executor, listener) ->
+ executor.execute { listener.onPropertiesChanged(p) }
+ }
+ }
+ }
+
+ override fun getConfigActivelyPreferBadWifi(): Int {
+ return synchronized(listeners) { configActivelyPreferBadWifi }
+ }
+
+ override fun addOnDevicePropertiesChangedListener(
+ e: Executor,
+ listener: OnPropertiesChangedListener
+ ) {
+ synchronized(listeners) {
+ listeners.add(e to listener)
+ }
+ }
+
+ override fun getResourcesForActiveSubId(res: ConnectivityResources, id: Int): Resources =
+ resources
+}