diff --git a/framework/src/android/net/util/MultinetworkPolicyTracker.java b/framework/src/android/net/util/MultinetworkPolicyTracker.java index 2dcf932345..4217921060 100644 --- a/framework/src/android/net/util/MultinetworkPolicyTracker.java +++ b/framework/src/android/net/util/MultinetworkPolicyTracker.java @@ -39,6 +39,7 @@ import android.telephony.TelephonyManager; import android.util.Log; import com.android.internal.annotations.VisibleForTesting; +import com.android.modules.utils.build.SdkLevel; import java.util.Arrays; import java.util.List; @@ -79,6 +80,29 @@ public class MultinetworkPolicyTracker { private int mActiveSubId = SubscriptionManager.INVALID_SUBSCRIPTION_ID; private volatile long mTestAllowBadWifiUntilMs = 0; + /** + * 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. + * if mAvoidBadWifi is true. If it's not, then no network ever yields to bad wifis + * ({@see FullScore#POLICY_YIELD_TO_BAD_WIFI}) and this setting has therefore no effect. + * + * If this is false, when ranking a bad wifi that never validated against cell data (or any + * network that yields to bad wifis), the ranker will prefer cell data. It will prefer wifi + * if wifi loses validation later. This behavior avoids the device losing internet access when + * walking past a wifi network with no internet access. + * This is the default behavior up to Android T, but it can be overridden through an overlay + * to behave like below. + * + * If this is true, then in the same scenario, the ranker will prefer cell data until + * the wifi completes its first validation attempt (or the attempt times out after + * ConnectivityService#PROMPT_UNVALIDATED_DELAY_MS), then it will prefer the wifi even if it + * doesn't provide internet access, unless there is a captive portal on that wifi. + * This is the behavior in U and above. + */ + // TODO : implement the behavior. + private boolean mActivelyPreferBadWifi; + // Mainline module can't use internal HandlerExecutor, so add an identical executor here. private static class HandlerExecutor implements Executor { @NonNull @@ -158,6 +182,10 @@ public class MultinetworkPolicyTracker { return mAvoidBadWifi; } + public boolean getActivelyPreferBadWifi() { + return mActivelyPreferBadWifi; + } + // TODO: move this to MultipathPolicyTracker. public int getMeteredMultipathPreference() { return mMeteredMultipathPreference; @@ -179,6 +207,29 @@ public class MultinetworkPolicyTracker { return (getResourcesForActiveSubId().getInteger(id) == 0); } + /** + * Whether the device config prefers bad wifi actively, when it doesn't avoid them + * + * This is only relevant when the device is configured not to avoid bad wifis. In this + * case, "actively" preferring a bad wifi means that the device will switch to a bad + * wifi it just connected to, as long as it's not a captive portal. + * + * On U and above this always returns true. On T and below it reads a configuration option. + */ + public boolean configActivelyPrefersBadWifi() { + // See the definition of config_activelyPreferBadWifi for a description of its meaning. + // On U and above, the config is ignored, and bad wifi is always actively preferred. + if (SdkLevel.isAtLeastU()) return true; + // TODO: use R.integer.config_activelyPreferBadWifi directly + final int id = mResources.get().getIdentifier("config_activelyPreferBadWifi", + "integer", mResources.getResourcesContext().getPackageName()); + // On T and below, 1 means to actively prefer bad wifi, 0 means not to prefer + // 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().getInteger(id); + } + /** * Temporarily allow bad wifi to override {@code config_networkAvoidBadWifi} configuration. * The value works when the time set is more than {@link System.currentTimeMillis()}. @@ -224,9 +275,13 @@ public class MultinetworkPolicyTracker { public boolean updateAvoidBadWifi() { final boolean settingAvoidBadWifi = "1".equals(getAvoidBadWifiSetting()); - final boolean prev = mAvoidBadWifi; + final boolean prevAvoid = mAvoidBadWifi; mAvoidBadWifi = settingAvoidBadWifi || !configRestrictsAvoidBadWifi(); - return mAvoidBadWifi != prev; + + final boolean prevActive = mActivelyPreferBadWifi; + mActivelyPreferBadWifi = configActivelyPrefersBadWifi(); + + return mAvoidBadWifi != prevAvoid || mActivelyPreferBadWifi != prevActive; } /** diff --git a/service/ServiceConnectivityResources/res/values/config.xml b/service/ServiceConnectivityResources/res/values/config.xml index bff69530e6..7d777ec8f2 100644 --- a/service/ServiceConnectivityResources/res/values/config.xml +++ b/service/ServiceConnectivityResources/res/values/config.xml @@ -78,6 +78,27 @@ Settings.Global.NETWORK_AVOID_BAD_WIFI. This is the default value of that setting. --> 1 + + 1 + diff --git a/service/ServiceConnectivityResources/res/values/overlayable.xml b/service/ServiceConnectivityResources/res/values/overlayable.xml index 3389d63b08..4c85e8c30f 100644 --- a/service/ServiceConnectivityResources/res/values/overlayable.xml +++ b/service/ServiceConnectivityResources/res/values/overlayable.xml @@ -24,6 +24,7 @@ + diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 93265e51ca..cb209e8ae7 100755 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -783,7 +783,8 @@ public class ConnectivityService extends IConnectivityManager.Stub final ConnectivityDiagnosticsHandler mConnectivityDiagnosticsHandler; private final DnsManager mDnsManager; - private final NetworkRanker mNetworkRanker; + @VisibleForTesting + final NetworkRanker mNetworkRanker; private boolean mSystemReady; private Intent mInitialBroadcast; @@ -1417,7 +1418,6 @@ public class ConnectivityService extends IConnectivityManager.Stub new RequestInfoPerUidCounter(MAX_NETWORK_REQUESTS_PER_SYSTEM_UID - 1); mMetricsLog = logger; - mNetworkRanker = new NetworkRanker(); final NetworkRequest defaultInternetRequest = createDefaultRequest(); mDefaultRequest = new NetworkRequestInfo( Process.myUid(), defaultInternetRequest, null, @@ -1538,6 +1538,9 @@ public class ConnectivityService extends IConnectivityManager.Stub mMultinetworkPolicyTracker = mDeps.makeMultinetworkPolicyTracker( mContext, mHandler, () -> updateAvoidBadWifi()); + mNetworkRanker = + new NetworkRanker(new NetworkRanker.Configuration(activelyPreferBadWifi())); + mMultinetworkPolicyTracker.start(); mDnsManager = new DnsManager(mContext, mDnsResolver); @@ -5050,6 +5053,10 @@ public class ConnectivityService extends IConnectivityManager.Stub return mMultinetworkPolicyTracker.getAvoidBadWifi(); } + private boolean activelyPreferBadWifi() { + return mMultinetworkPolicyTracker.getActivelyPreferBadWifi(); + } + /** * Return whether the device should maintain continuous, working connectivity by switching away * from WiFi networks having no connectivity. @@ -5073,6 +5080,7 @@ public class ConnectivityService extends IConnectivityManager.Stub for (final NetworkOfferInfo noi : offersToUpdate) { updateOfferScore(noi.offer); } + mNetworkRanker.setConfiguration(new NetworkRanker.Configuration(activelyPreferBadWifi())); rematchAllNetworksAndRequests(); } @@ -5088,6 +5096,7 @@ public class ConnectivityService extends IConnectivityManager.Stub pw.println("Bad Wi-Fi avoidance: " + avoidBadWifi()); pw.increaseIndent(); pw.println("Config restrict: " + configRestrict); + pw.println("Actively prefer: " + activelyPreferBadWifi()); final String value = mMultinetworkPolicyTracker.getAvoidBadWifiSetting(); String description; diff --git a/service/src/com/android/server/connectivity/NetworkRanker.java b/service/src/com/android/server/connectivity/NetworkRanker.java index f2c6aa1f08..d56e171355 100644 --- a/service/src/com/android/server/connectivity/NetworkRanker.java +++ b/service/src/com/android/server/connectivity/NetworkRanker.java @@ -38,19 +38,42 @@ import android.annotation.NonNull; import android.annotation.Nullable; import android.net.NetworkCapabilities; import android.net.NetworkRequest; +import android.net.util.MultinetworkPolicyTracker; +import com.android.internal.annotations.VisibleForTesting; import com.android.net.module.util.CollectionUtils; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.List; +import java.util.Objects; import java.util.function.Predicate; /** * A class that knows how to find the best network matching a request out of a list of networks. */ public class NetworkRanker { + /** + * Home for all configurations of NetworkRanker + */ + public static final class Configuration { + private final boolean mActivelyPreferBadWifi; + + public Configuration(final boolean activelyPreferBadWifi) { + this.mActivelyPreferBadWifi = activelyPreferBadWifi; + } + + /** + * @see MultinetworkPolicyTracker#getActivelyPreferBadWifi() + */ + // TODO : implement the behavior. + public boolean activelyPreferBadWifi() { + return mActivelyPreferBadWifi; + } + } + @NonNull private volatile Configuration mConf; + // Historically the legacy ints have been 0~100 in principle (though the highest score in // AOSP has always been 90). This is relied on by VPNs that send a legacy score of 101. public static final int LEGACY_INT_MAX = 100; @@ -65,7 +88,22 @@ public class NetworkRanker { NetworkCapabilities getCapsNoCopy(); } - public NetworkRanker() { } + public NetworkRanker(@NonNull final Configuration conf) { + // Because mConf is volatile, the only way it could be seen null would be an access to it + // on some other thread during this constructor. But this is not possible because mConf is + // private and `this` doesn't escape this constructor. + setConfiguration(conf); + } + + public void setConfiguration(@NonNull final Configuration conf) { + mConf = Objects.requireNonNull(conf); + } + + // There shouldn't be a use case outside of testing + @VisibleForTesting + public Configuration getConfiguration() { + return mConf; + } /** * Find the best network satisfying this request among the list of passed networks. diff --git a/tests/unit/java/android/net/util/MultinetworkPolicyTrackerTest.kt b/tests/unit/java/android/net/util/MultinetworkPolicyTrackerTest.kt index 0dbee5d7b0..f3178639ae 100644 --- a/tests/unit/java/android/net/util/MultinetworkPolicyTrackerTest.kt +++ b/tests/unit/java/android/net/util/MultinetworkPolicyTrackerTest.kt @@ -35,6 +35,7 @@ import android.test.mock.MockContentResolver import androidx.test.filters.SmallTest import com.android.connectivity.resources.R import com.android.internal.util.test.FakeSettingsProvider +import com.android.modules.utils.build.SdkLevel import com.android.testutils.DevSdkIgnoreRule import com.android.testutils.DevSdkIgnoreRunner import org.junit.After @@ -50,7 +51,6 @@ import org.mockito.ArgumentMatchers.argThat import org.mockito.ArgumentMatchers.eq import org.mockito.Mockito.any import org.mockito.Mockito.doCallRealMethod -import org.mockito.Mockito.doNothing import org.mockito.Mockito.doReturn import org.mockito.Mockito.mock import org.mockito.Mockito.times @@ -69,7 +69,10 @@ class MultinetworkPolicyTrackerTest { private val resources = mock(Resources::class.java).also { doReturn(R.integer.config_networkAvoidBadWifi).`when`(it).getIdentifier( eq("config_networkAvoidBadWifi"), eq("integer"), any()) + doReturn(R.integer.config_activelyPreferBadWifi).`when`(it).getIdentifier( + eq("config_activelyPreferBadWifi"), eq("integer"), any()) doReturn(0).`when`(it).getInteger(R.integer.config_networkAvoidBadWifi) + doReturn(0).`when`(it).getInteger(R.integer.config_activelyPreferBadWifi) } private val telephonyManager = mock(TelephonyManager::class.java) private val subscriptionManager = mock(SubscriptionManager::class.java).also { @@ -122,6 +125,7 @@ class MultinetworkPolicyTrackerTest { @Test fun testUpdateAvoidBadWifi() { + doReturn(0).`when`(resources).getInteger(R.integer.config_activelyPreferBadWifi) Settings.Global.putString(resolver, NETWORK_AVOID_BAD_WIFI, "0") assertTrue(tracker.updateAvoidBadWifi()) assertFalse(tracker.avoidBadWifi) @@ -129,6 +133,24 @@ class MultinetworkPolicyTrackerTest { doReturn(1).`when`(resources).getInteger(R.integer.config_networkAvoidBadWifi) assertTrue(tracker.updateAvoidBadWifi()) assertTrue(tracker.avoidBadWifi) + + if (SdkLevel.isAtLeastU()) { + // On U+, the system always prefers bad wifi. + assertTrue(tracker.activelyPreferBadWifi) + } else { + assertFalse(tracker.activelyPreferBadWifi) + } + + doReturn(1).`when`(resources).getInteger(R.integer.config_activelyPreferBadWifi) + if (SdkLevel.isAtLeastU()) { + // On U+, this didn't change the setting + assertFalse(tracker.updateAvoidBadWifi()) + } else { + // On T-, this must have changed the setting + assertTrue(tracker.updateAvoidBadWifi()) + } + // In all cases, now the system actively prefers bad wifi + assertTrue(tracker.activelyPreferBadWifi) } @Test diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index d2a71354ec..5e681b6bd0 100755 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -1838,7 +1838,10 @@ public class ConnectivityServiceTest { .getIdentifier(eq("network_switch_type_name"), eq("array"), any()); doReturn(R.integer.config_networkAvoidBadWifi).when(mResources) .getIdentifier(eq("config_networkAvoidBadWifi"), eq("integer"), any()); + doReturn(R.integer.config_activelyPreferBadWifi).when(mResources) + .getIdentifier(eq("config_activelyPreferBadWifi"), eq("integer"), any()); doReturn(1).when(mResources).getInteger(R.integer.config_networkAvoidBadWifi); + doReturn(0).when(mResources).getInteger(R.integer.config_activelyPreferBadWifi); doReturn(true).when(mResources) .getBoolean(R.bool.config_cellular_radio_timesharing_capable); } @@ -5620,6 +5623,24 @@ public class ConnectivityServiceTest { testAvoidBadWifiConfig_controlledBySettings(); } + @Test + public void testActivelyPreferBadWifiSetting() throws Exception { + doReturn(1).when(mResources).getInteger(R.integer.config_activelyPreferBadWifi); + mPolicyTracker.reevaluate(); + waitForIdle(); + assertTrue(mService.mNetworkRanker.getConfiguration().activelyPreferBadWifi()); + + doReturn(0).when(mResources).getInteger(R.integer.config_activelyPreferBadWifi); + mPolicyTracker.reevaluate(); + waitForIdle(); + if (SdkLevel.isAtLeastU()) { + // U+ ignore the setting and always actively prefers bad wifi + assertTrue(mService.mNetworkRanker.getConfiguration().activelyPreferBadWifi()); + } else { + assertFalse(mService.mNetworkRanker.getConfiguration().activelyPreferBadWifi()); + } + } + @Test public void testOffersAvoidsBadWifi() throws Exception { // Normal mode : the carrier doesn't restrict moving away from bad wifi. diff --git a/tests/unit/java/com/android/server/connectivity/NetworkRankerTest.kt b/tests/unit/java/com/android/server/connectivity/NetworkRankerTest.kt index 8b1c51018c..05c648dbcc 100644 --- a/tests/unit/java/com/android/server/connectivity/NetworkRankerTest.kt +++ b/tests/unit/java/com/android/server/connectivity/NetworkRankerTest.kt @@ -42,7 +42,8 @@ private fun caps(transport: Int) = NetworkCapabilities.Builder().addTransportTyp @RunWith(DevSdkIgnoreRunner::class) @DevSdkIgnoreRule.IgnoreUpTo(Build.VERSION_CODES.R) class NetworkRankerTest { - private val mRanker = NetworkRanker() + private val mRanker = NetworkRanker(NetworkRanker.Configuration( + false /* activelyPreferBadWifi */)) private class TestScore(private val sc: FullScore, private val nc: NetworkCapabilities) : NetworkRanker.Scoreable {