Implement the "actively prefer bad wifi" feature

Test: in progress
Change-Id: Ib7aba464a2f32642d434418842306dfcf90b8319

framework/src/android/net/util/MultinetworkPolicyTracker.java

@@ -100,7 +100,6 @@ public class MultinetworkPolicyTracker {
      * 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.

service/src/com/android/server/connectivity/NetworkRanker.java

@@ -16,6 +16,7 @@
 
 package com.android.server.connectivity;
 
+import static android.net.NetworkCapabilities.NET_CAPABILITY_CAPTIVE_PORTAL;
 import static android.net.NetworkCapabilities.TRANSPORT_BLUETOOTH;
 import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR;
 import static android.net.NetworkCapabilities.TRANSPORT_ETHERNET;
@@ -27,6 +28,7 @@ import static android.net.NetworkScore.POLICY_YIELD_TO_BAD_WIFI;
 import static com.android.net.module.util.CollectionUtils.filter;
 import static com.android.server.connectivity.FullScore.POLICY_ACCEPT_UNVALIDATED;
 import static com.android.server.connectivity.FullScore.POLICY_AVOIDED_WHEN_UNVALIDATED;
+import static com.android.server.connectivity.FullScore.POLICY_EVER_EVALUATED;
 import static com.android.server.connectivity.FullScore.POLICY_EVER_USER_SELECTED;
 import static com.android.server.connectivity.FullScore.POLICY_EVER_VALIDATED;
 import static com.android.server.connectivity.FullScore.POLICY_IS_DESTROYED;
@@ -67,7 +69,6 @@ public class NetworkRanker {
         /**
          * @see MultinetworkPolicyTracker#getActivelyPreferBadWifi()
          */
-        // TODO : implement the behavior.
         public boolean activelyPreferBadWifi() {
             return mActivelyPreferBadWifi;
         }
@@ -142,11 +143,42 @@ public class NetworkRanker {
         }
     }
 
-    private <T extends Scoreable> boolean isBadWiFi(@NonNull final T candidate) {
+    /**
+     * Returns whether the wifi passed as an argument is a preferred network to yielding cell.
+     *
+     * When comparing bad wifi to cell with POLICY_YIELD_TO_BAD_WIFI, it may be necessary to
+     * know if a particular bad wifi is preferred to such a cell network. This method computes
+     * and returns this.
+     *
+     * @param candidate a bad wifi to evaluate
+     * @return whether this candidate is preferred to cell with POLICY_YIELD_TO_BAD_WIFI
+     */
+    private <T extends Scoreable> boolean isPreferredBadWiFi(@NonNull final T candidate) {
         final FullScore score = candidate.getScore();
-        return score.hasPolicy(POLICY_EVER_VALIDATED)
+        final NetworkCapabilities caps = candidate.getCapsNoCopy();
-                && !score.hasPolicy(POLICY_AVOIDED_WHEN_UNVALIDATED)
+
-                && candidate.getCapsNoCopy().hasTransport(TRANSPORT_WIFI);
+        // Whatever the policy, only WiFis can be preferred bad WiFis.
+        if (!caps.hasTransport(TRANSPORT_WIFI)) return false;
+        // Validated networks aren't bad networks, so a fortiori can't be preferred bad WiFis.
+        if (score.hasPolicy(POLICY_IS_VALIDATED)) return false;
+        // A WiFi that the user explicitly wanted to avoid in UI is never a preferred bad WiFi.
+        if (score.hasPolicy(POLICY_AVOIDED_WHEN_UNVALIDATED)) return false;
+
+        if (mConf.activelyPreferBadWifi()) {
+            // If a network is still evaluating, don't prefer it.
+            if (!score.hasPolicy(POLICY_EVER_EVALUATED)) return false;
+
+            // If a network is not a captive portal, then prefer it.
+            if (!caps.hasCapability(NET_CAPABILITY_CAPTIVE_PORTAL)) return true;
+
+            // If it's a captive portal, prefer it if it previously validated but is no longer
+            // validated (i.e., the user logged in in the past, but later the portal closed).
+            return score.hasPolicy(POLICY_EVER_VALIDATED);
+        } else {
+            // Under the original "prefer bad WiFi" policy, only networks that have ever validated
+            // are preferred.
+            return score.hasPolicy(POLICY_EVER_VALIDATED);
+        }
     }
 
     /**
@@ -169,7 +201,7 @@ public class NetworkRanker {
             // No network with the policy : do nothing.
             return;
         }
-        if (!CollectionUtils.any(rejected, n -> isBadWiFi(n))) {
+        if (!CollectionUtils.any(rejected, n -> isPreferredBadWiFi(n))) {
             // No bad WiFi : do nothing.
             return;
         }
@@ -179,7 +211,7 @@ public class NetworkRanker {
             // wifis by the following policies (e.g. exiting).
             final ArrayList<T> acceptedYielders = new ArrayList<>(accepted);
             final ArrayList<T> rejectedWithBadWiFis = new ArrayList<>(rejected);
-            partitionInto(rejectedWithBadWiFis, n -> isBadWiFi(n), accepted, rejected);
+            partitionInto(rejectedWithBadWiFis, n -> isPreferredBadWiFi(n), accepted, rejected);
             accepted.addAll(acceptedYielders);
             return;
         }

tests/unit/java/com/android/server/connectivity/NetworkRankerTest.kt

@@ -17,6 +17,7 @@
 package com.android.server.connectivity
 
 import android.net.NetworkCapabilities
+import android.net.NetworkCapabilities.NET_CAPABILITY_CAPTIVE_PORTAL
 import android.net.NetworkCapabilities.TRANSPORT_CELLULAR
 import android.net.NetworkCapabilities.TRANSPORT_WIFI
 import android.net.NetworkScore.KEEP_CONNECTED_NONE
@@ -25,25 +26,29 @@ import android.net.NetworkScore.POLICY_TRANSPORT_PRIMARY
 import android.net.NetworkScore.POLICY_YIELD_TO_BAD_WIFI
 import android.os.Build
 import androidx.test.filters.SmallTest
+import com.android.connectivity.resources.R
 import com.android.server.connectivity.FullScore.POLICY_AVOIDED_WHEN_UNVALIDATED
+import com.android.server.connectivity.FullScore.POLICY_EVER_EVALUATED
 import com.android.server.connectivity.FullScore.POLICY_EVER_VALIDATED
 import com.android.server.connectivity.FullScore.POLICY_IS_VALIDATED
 import com.android.testutils.DevSdkIgnoreRule
-import com.android.testutils.DevSdkIgnoreRunner
+import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
+import org.junit.runners.Parameterized
 import kotlin.test.assertEquals
 
 private fun score(vararg policies: Int) = FullScore(
         policies.fold(0L) { acc, e -> acc or (1L shl e) }, KEEP_CONNECTED_NONE)
-private fun caps(transport: Int) = NetworkCapabilities.Builder().addTransportType(transport).build()
+private fun caps(transport: Int, vararg capabilities: Int) =
+        NetworkCapabilities.Builder().addTransportType(transport).apply {
+            capabilities.forEach { addCapability(it) }
+        }.build()
 
 @SmallTest
-@RunWith(DevSdkIgnoreRunner::class)
+@RunWith(Parameterized::class)
-@DevSdkIgnoreRule.IgnoreUpTo(Build.VERSION_CODES.R)
+class NetworkRankerTest(private val activelyPreferBadWifi: Boolean) {
-class NetworkRankerTest {
+    private val mRanker = NetworkRanker(NetworkRanker.Configuration(activelyPreferBadWifi))
-    private val mRanker = NetworkRanker(NetworkRanker.Configuration(
-            false /* activelyPreferBadWifi */))
 
     private class TestScore(private val sc: FullScore, private val nc: NetworkCapabilities)
             : NetworkRanker.Scoreable {
@@ -51,35 +56,91 @@ class NetworkRankerTest {
         override fun getCapsNoCopy(): NetworkCapabilities = nc
     }
 
+    @get:Rule
+    val mIgnoreRule: DevSdkIgnoreRule = DevSdkIgnoreRule(ignoreClassUpTo = Build.VERSION_CODES.R)
+
+    companion object {
+        @JvmStatic
+        @Parameterized.Parameters
+        fun ranker() = listOf(true, false)
+    }
+
     @Test
     fun testYieldToBadWiFiOneCell() {
         // Only cell, it wins
-        val winner = TestScore(score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
+        val winner = TestScore(
+                score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED, POLICY_EVER_EVALUATED),
                 caps(TRANSPORT_CELLULAR))
         val scores = listOf(winner)
         assertEquals(winner, mRanker.getBestNetworkByPolicy(scores, null))
     }
 
+    @Test
+    fun testPreferBadWifiOneCellOneEvaluatingWifi() {
+        // TODO : refactor the tests to name each network like this test
+        val wifi = TestScore(score(), caps(TRANSPORT_WIFI))
+        val cell = TestScore(
+                score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED, POLICY_EVER_EVALUATED),
+                caps(TRANSPORT_CELLULAR))
+        assertEquals(cell, mRanker.getBestNetworkByPolicy(listOf(wifi, cell), null))
+    }
+
     @Test
     fun testYieldToBadWiFiOneCellOneBadWiFi() {
         // Bad wifi wins against yielding validated cell
-        val winner = TestScore(score(POLICY_EVER_VALIDATED), caps(TRANSPORT_WIFI))
+        val winner = TestScore(score(POLICY_EVER_VALIDATED, POLICY_EVER_EVALUATED),
+                caps(TRANSPORT_WIFI))
         val scores = listOf(
                 winner,
-                TestScore(score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
+                TestScore(
+                        score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED, POLICY_EVER_EVALUATED),
                         caps(TRANSPORT_CELLULAR))
         )
         assertEquals(winner, mRanker.getBestNetworkByPolicy(scores, null))
     }
 
+    @Test
+    fun testPreferBadWifiOneCellOneBadWifi() {
+        val wifi = TestScore(score(POLICY_EVER_EVALUATED), caps(TRANSPORT_WIFI))
+        val cell = TestScore(
+                score(POLICY_EVER_EVALUATED, POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
+                caps(TRANSPORT_CELLULAR))
+        val scores = listOf(wifi, cell)
+        val winner = if (activelyPreferBadWifi) wifi else cell
+        assertEquals(winner, mRanker.getBestNetworkByPolicy(scores, null))
+    }
+
+    @Test
+    fun testPreferBadWifiOneCellOneCaptivePortalWifi() {
+        val wifi = TestScore(score(POLICY_EVER_EVALUATED),
+                caps(TRANSPORT_WIFI, NET_CAPABILITY_CAPTIVE_PORTAL))
+        val cell = TestScore(
+                score(POLICY_EVER_EVALUATED, POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
+                caps(TRANSPORT_CELLULAR))
+        assertEquals(cell, mRanker.getBestNetworkByPolicy(listOf(wifi, cell), null))
+    }
+
+    @Test
+    fun testYieldToBadWifiOneCellOneCaptivePortalWifiThatClosed() {
+        val wifi = TestScore(score(POLICY_EVER_EVALUATED, POLICY_EVER_VALIDATED),
+                caps(TRANSPORT_WIFI, NET_CAPABILITY_CAPTIVE_PORTAL))
+        val cell = TestScore(
+                score(POLICY_EVER_EVALUATED, POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
+                caps(TRANSPORT_CELLULAR))
+        assertEquals(wifi, mRanker.getBestNetworkByPolicy(listOf(wifi, cell), null))
+    }
+
     @Test
     fun testYieldToBadWifiAvoidUnvalidated() {
         // Bad wifi avoided when unvalidated loses against yielding validated cell
-        val winner = TestScore(score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
+        val winner = TestScore(
+                score(POLICY_EVER_EVALUATED, POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
                 caps(TRANSPORT_CELLULAR))
         val scores = listOf(
                 winner,
-                TestScore(score(POLICY_EVER_VALIDATED, POLICY_AVOIDED_WHEN_UNVALIDATED),
+                TestScore(
+                        score(POLICY_EVER_EVALUATED, POLICY_EVER_VALIDATED,
+                                POLICY_AVOIDED_WHEN_UNVALIDATED),
                         caps(TRANSPORT_WIFI))
         )
         assertEquals(winner, mRanker.getBestNetworkByPolicy(scores, null))
@@ -88,12 +149,16 @@ class NetworkRankerTest {
     @Test
     fun testYieldToBadWiFiOneCellTwoBadWiFi() {
         // Bad wifi wins against yielding validated cell. Prefer the one that's primary.
-        val winner = TestScore(score(POLICY_EVER_VALIDATED,
+        val winner = TestScore(
-                POLICY_TRANSPORT_PRIMARY), caps(TRANSPORT_WIFI))
+                score(POLICY_EVER_EVALUATED, POLICY_EVER_VALIDATED, POLICY_TRANSPORT_PRIMARY),
+                caps(TRANSPORT_WIFI))
         val scores = listOf(
                 winner,
-                TestScore(score(POLICY_EVER_VALIDATED), caps(TRANSPORT_WIFI)),
+                TestScore(
-                TestScore(score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
+                        score(POLICY_EVER_EVALUATED, POLICY_EVER_VALIDATED),
+                        caps(TRANSPORT_WIFI)),
+                TestScore(
+                        score(POLICY_EVER_EVALUATED, POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
                         caps(TRANSPORT_CELLULAR))
         )
         assertEquals(winner, mRanker.getBestNetworkByPolicy(scores, null))
@@ -103,11 +168,13 @@ class NetworkRankerTest {
     fun testYieldToBadWiFiOneCellTwoBadWiFiOneNotAvoided() {
         // Bad wifi ever validated wins against bad wifi that never was validated (or was
         // avoided when bad).
-        val winner = TestScore(score(POLICY_EVER_VALIDATED), caps(TRANSPORT_WIFI))
+        val winner = TestScore(score(POLICY_EVER_EVALUATED, POLICY_EVER_VALIDATED),
+                caps(TRANSPORT_WIFI))
         val scores = listOf(
                 winner,
-                TestScore(score(), caps(TRANSPORT_WIFI)),
+                TestScore(score(POLICY_EVER_EVALUATED), caps(TRANSPORT_WIFI)),
-                TestScore(score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
+                TestScore(
+                        score(POLICY_EVER_EVALUATED, POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
                         caps(TRANSPORT_CELLULAR))
         )
         assertEquals(winner, mRanker.getBestNetworkByPolicy(scores, null))
@@ -116,27 +183,49 @@ class NetworkRankerTest {
     @Test
     fun testYieldToBadWiFiOneCellOneBadWiFiOneGoodWiFi() {
         // Good wifi wins
-        val winner = TestScore(score(POLICY_EVER_VALIDATED, POLICY_IS_VALIDATED),
+        val winner = TestScore(
+                score(POLICY_EVER_EVALUATED, POLICY_EVER_VALIDATED, POLICY_IS_VALIDATED),
                 caps(TRANSPORT_WIFI))
         val scores = listOf(
                 winner,
-                TestScore(score(POLICY_EVER_VALIDATED, POLICY_TRANSPORT_PRIMARY),
+                TestScore(
+                        score(POLICY_EVER_EVALUATED, POLICY_EVER_VALIDATED,
+                                POLICY_TRANSPORT_PRIMARY),
                         caps(TRANSPORT_WIFI)),
-                TestScore(score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
+                TestScore(
+                        score(POLICY_EVER_EVALUATED, POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
                         caps(TRANSPORT_CELLULAR))
         )
         assertEquals(winner, mRanker.getBestNetworkByPolicy(scores, null))
     }
 
+    @Test
+    fun testPreferBadWifiOneCellOneBadWifiOneEvaluatingWifi() {
+        val cell = TestScore(
+                score(POLICY_EVER_EVALUATED, POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
+                caps(TRANSPORT_CELLULAR))
+        val badWifi = TestScore(score(POLICY_EVER_EVALUATED), caps(TRANSPORT_WIFI))
+        val evaluatingWifi = TestScore(score(), caps(TRANSPORT_WIFI))
+        val winner = if (activelyPreferBadWifi) badWifi else cell
+        assertEquals(winner,
+                mRanker.getBestNetworkByPolicy(listOf(cell, badWifi, evaluatingWifi), null))
+    }
+
     @Test
     fun testYieldToBadWiFiTwoCellsOneBadWiFi() {
         // Cell that doesn't yield wins over cell that yields and bad wifi
-        val winner = TestScore(score(POLICY_IS_VALIDATED), caps(TRANSPORT_CELLULAR))
+        val winner = TestScore(
+                score(POLICY_EVER_EVALUATED, POLICY_IS_VALIDATED),
+                caps(TRANSPORT_CELLULAR))
         val scores = listOf(
                 winner,
-                TestScore(score(POLICY_EVER_VALIDATED, POLICY_TRANSPORT_PRIMARY),
+                TestScore(
+                        score(POLICY_EVER_EVALUATED, POLICY_EVER_VALIDATED,
+                                POLICY_TRANSPORT_PRIMARY),
                         caps(TRANSPORT_WIFI)),
-                TestScore(score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
+                TestScore(
+                        score(POLICY_EVER_EVALUATED, POLICY_YIELD_TO_BAD_WIFI,
+                                POLICY_EVER_VALIDATED, POLICY_IS_VALIDATED),
                         caps(TRANSPORT_CELLULAR))
         )
         assertEquals(winner, mRanker.getBestNetworkByPolicy(scores, null))
@@ -145,13 +234,20 @@ class NetworkRankerTest {
     @Test
     fun testYieldToBadWiFiTwoCellsOneBadWiFiOneGoodWiFi() {
         // Good wifi wins over cell that doesn't yield and cell that yields
-        val winner = TestScore(score(POLICY_IS_VALIDATED), caps(TRANSPORT_WIFI))
+        val winner = TestScore(
+                score(POLICY_EVER_EVALUATED, POLICY_IS_VALIDATED),
+                caps(TRANSPORT_WIFI))
         val scores = listOf(
                 winner,
-                TestScore(score(POLICY_EVER_VALIDATED, POLICY_TRANSPORT_PRIMARY),
+                TestScore(
+                        score(POLICY_EVER_EVALUATED, POLICY_EVER_VALIDATED,
+                                POLICY_TRANSPORT_PRIMARY),
                         caps(TRANSPORT_WIFI)),
-                TestScore(score(POLICY_IS_VALIDATED), caps(TRANSPORT_CELLULAR)),
+                TestScore(
-                TestScore(score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
+                        score(POLICY_EVER_EVALUATED, POLICY_IS_VALIDATED),
+                        caps(TRANSPORT_CELLULAR)),
+                TestScore(
+                        score(POLICY_EVER_EVALUATED, POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
                         caps(TRANSPORT_CELLULAR))
         )
         assertEquals(winner, mRanker.getBestNetworkByPolicy(scores, null))
@@ -160,11 +256,14 @@ class NetworkRankerTest {
     @Test
     fun testYieldToBadWiFiOneExitingGoodWiFi() {
         // Yielding cell wins over good exiting wifi
-        val winner = TestScore(score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
+        val winner = TestScore(
+                score(POLICY_EVER_EVALUATED, POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
                 caps(TRANSPORT_CELLULAR))
         val scores = listOf(
                 winner,
-                TestScore(score(POLICY_IS_VALIDATED, POLICY_EXITING), caps(TRANSPORT_WIFI))
+                TestScore(
+                        score(POLICY_EVER_EVALUATED, POLICY_IS_VALIDATED, POLICY_EXITING),
+                        caps(TRANSPORT_WIFI))
         )
         assertEquals(winner, mRanker.getBestNetworkByPolicy(scores, null))
     }
@@ -172,11 +271,14 @@ class NetworkRankerTest {
     @Test
     fun testYieldToBadWiFiOneExitingBadWiFi() {
         // Yielding cell wins over bad exiting wifi
-        val winner = TestScore(score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
+        val winner = TestScore(
+                score(POLICY_EVER_EVALUATED, POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
                 caps(TRANSPORT_CELLULAR))
         val scores = listOf(
                 winner,
-                TestScore(score(POLICY_EVER_VALIDATED, POLICY_EXITING), caps(TRANSPORT_WIFI))
+                TestScore(
+                        score(POLICY_EVER_EVALUATED, POLICY_EVER_VALIDATED, POLICY_EXITING),
+                        caps(TRANSPORT_WIFI))
         )
         assertEquals(winner, mRanker.getBestNetworkByPolicy(scores, null))
     }