Make yield-to-bad-wifi behavior backward compatible with R

Yielding cell wins to exiting wifi (whether good or bad).
It loses to bad wifi that's not exiting.

In R, yielding to bad wifi only affects wifis that are
unvalidated, but a wifi that is exiting should still be
dropped in favor of a cell that yields to bad wifi.

I had misunderstood the policy and implemented it wrong.
Now it's implemented right, and has careful tests.

Test: new tests for this
Bug: 186458024
Merged-In: I3c2563d4ae4e3715d0c6270344ba8f7ef067872f
Merged-In: Ib8637100d491e72a2edb837584ce55b7dda58524
Change-Id: Ib8637100d491e72a2edb837584ce55b7dda58524
  (cherry-picked from ag/14486203)

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

@@ -108,7 +108,58 @@ public class NetworkRanker {
         }
     }
 
-    @Nullable private <T extends Scoreable> T getBestNetworkByPolicy(
+    private <T extends Scoreable> boolean isBadWiFi(@NonNull final T candidate) {
+        return candidate.getScore().hasPolicy(POLICY_EVER_VALIDATED_NOT_AVOIDED_WHEN_BAD)
+                && candidate.getCapsNoCopy().hasTransport(TRANSPORT_WIFI);
+    }
+
+    /**
+     * Apply the "yield to bad WiFi" policy.
+     *
+     * This function must run immediately after the validation policy.
+     *
+     * If any of the accepted networks has the "yield to bad WiFi" policy AND there are some
+     * bad WiFis in the rejected list, then move the networks with the policy to the rejected
+     * list. If this leaves no accepted network, then move the bad WiFis back to the accepted list.
+     *
+     * This function returns nothing, but will have updated accepted and rejected in-place.
+     *
+     * @param accepted networks accepted by the validation policy
+     * @param rejected networks rejected by the validation policy
+     */
+    private <T extends Scoreable> void applyYieldToBadWifiPolicy(@NonNull ArrayList<T> accepted,
+            @NonNull ArrayList<T> rejected) {
+        if (!CollectionUtils.any(accepted, n -> n.getScore().hasPolicy(POLICY_YIELD_TO_BAD_WIFI))) {
+            // No network with the policy : do nothing.
+            return;
+        }
+        if (!CollectionUtils.any(rejected, n -> isBadWiFi(n))) {
+            // No bad WiFi : do nothing.
+            return;
+        }
+        if (CollectionUtils.all(accepted, n -> n.getScore().hasPolicy(POLICY_YIELD_TO_BAD_WIFI))) {
+            // All validated networks yield to bad WiFis : keep bad WiFis alongside with the
+            // yielders. This is important because the yielders need to be compared to the bad
+            // 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);
+            accepted.addAll(acceptedYielders);
+            return;
+        }
+        // Only some of the validated networks yield to bad WiFi : keep only the ones who don't.
+        final ArrayList<T> acceptedWithYielders = new ArrayList<>(accepted);
+        partitionInto(acceptedWithYielders, n -> !n.getScore().hasPolicy(POLICY_YIELD_TO_BAD_WIFI),
+                accepted, rejected);
+    }
+
+    /**
+     * Get the best network among a list of candidates according to policy.
+     * @param candidates the candidates
+     * @param currentSatisfier the current satisfier, or null if none
+     * @return the best network
+     */
+    @Nullable public <T extends Scoreable> T getBestNetworkByPolicy(
             @NonNull List<T> candidates,
             @Nullable final T currentSatisfier) {
         // Used as working areas.
@@ -148,24 +199,15 @@ public class NetworkRanker {
         if (accepted.size() == 1) return accepted.get(0);
         if (accepted.size() > 0 && rejected.size() > 0) candidates = new ArrayList<>(accepted);
 
-        // Yield to bad wifi policy : if any wifi has ever been validated (even if it's now
-        // unvalidated), and unless it's been explicitly avoided when bad in UI, then keep only
-        // networks that don't yield to such a wifi network.
-        final boolean anyWiFiEverValidated = CollectionUtils.any(candidates,
-                nai -> nai.getScore().hasPolicy(POLICY_EVER_VALIDATED_NOT_AVOIDED_WHEN_BAD)
-                        && nai.getCapsNoCopy().hasTransport(TRANSPORT_WIFI));
-        if (anyWiFiEverValidated) {
-            partitionInto(candidates, nai -> !nai.getScore().hasPolicy(POLICY_YIELD_TO_BAD_WIFI),
-                    accepted, rejected);
-            if (accepted.size() == 1) return accepted.get(0);
-            if (accepted.size() > 0 && rejected.size() > 0) candidates = new ArrayList<>(accepted);
-        }
-
         // If any network is validated (or should be accepted even if it's not validated), then
         // don't choose one that isn't.
         partitionInto(candidates, nai -> nai.getScore().hasPolicy(POLICY_IS_VALIDATED)
                         || nai.getScore().hasPolicy(POLICY_ACCEPT_UNVALIDATED),
                 accepted, rejected);
+        // Yield to bad wifi policy : if any network has the "yield to bad WiFi" policy and
+        // there are bad WiFis connected, then accept the bad WiFis and reject the networks with
+        // the policy.
+        applyYieldToBadWifiPolicy(accepted, rejected);
         if (accepted.size() == 1) return accepted.get(0);
         if (accepted.size() > 0 && rejected.size() > 0) candidates = new ArrayList<>(accepted);
 
@@ -194,16 +236,24 @@ public class NetworkRanker {
         // subscription with the same transport.
         partitionInto(candidates, nai -> nai.getScore().hasPolicy(POLICY_TRANSPORT_PRIMARY),
                 accepted, rejected);
-        for (final Scoreable defaultSubNai : accepted) {
+        if (accepted.size() > 0) {
-            // Remove all networks without the DEFAULT_SUBSCRIPTION policy and the same transports
+            // Some networks are primary. For each transport, keep only the primary, but also
-            // as a network that has it.
+            // keep all networks for which there isn't a primary (which are now in the |rejected|
+            // array).
+            // So for each primary network, remove from |rejected| all networks with the same
+            // transports as one of the primary networks. The remaining networks should be accepted.
+            for (final T defaultSubNai : accepted) {
                 final int[] transports = defaultSubNai.getCapsNoCopy().getTransportTypes();
-            candidates.removeIf(nai -> !nai.getScore().hasPolicy(POLICY_TRANSPORT_PRIMARY)
+                rejected.removeIf(
-                    && Arrays.equals(transports, nai.getCapsNoCopy().getTransportTypes()));
+                        nai -> Arrays.equals(transports, nai.getCapsNoCopy().getTransportTypes()));
+            }
+            accepted.addAll(rejected);
+            candidates = new ArrayList<>(accepted);
         }
         if (1 == candidates.size()) return candidates.get(0);
-        // It's guaranteed candidates.size() > 0 because there is at least one with the
+        // If there were no primary network, then candidates.size() > 0 because it didn't
-        // TRANSPORT_PRIMARY policy and only those without it were removed.
+        // change from the previous result. If there were, it's guaranteed candidates.size() > 0
+        // because accepted.size() > 0 above.
 
         // If some of the networks have a better transport than others, keep only the ones with
         // the best transports.

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2020 The Android Open Source Project
+ * Copyright (C) 2021 The Android Open Source Project
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -17,74 +17,156 @@
 package com.android.server.connectivity
 
 import android.net.NetworkCapabilities
-import android.net.NetworkRequest
+import android.net.NetworkCapabilities.TRANSPORT_CELLULAR
+import android.net.NetworkCapabilities.TRANSPORT_WIFI
 import android.net.NetworkScore.KEEP_CONNECTED_NONE
+import android.net.NetworkScore.POLICY_EXITING
+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 androidx.test.runner.AndroidJUnit4
+import com.android.server.connectivity.FullScore.POLICY_EVER_VALIDATED_NOT_AVOIDED_WHEN_BAD
+import com.android.server.connectivity.FullScore.POLICY_IS_VALIDATED
+import com.android.testutils.DevSdkIgnoreRule
+import com.android.testutils.DevSdkIgnoreRunner
 import org.junit.Test
 import org.junit.runner.RunWith
-import org.mockito.ArgumentMatchers.any
-import org.mockito.Mockito.doReturn
-import org.mockito.Mockito.mock
 import kotlin.test.assertEquals
-import kotlin.test.assertNull
 
-@RunWith(AndroidJUnit4::class)
+private fun score(vararg policies: Int) = FullScore(0,
+        policies.fold(0L) { acc, e -> acc or (1L shl e) }, KEEP_CONNECTED_NONE)
+private fun caps(transport: Int) = NetworkCapabilities.Builder().addTransportType(transport).build()
+
 @SmallTest
+@RunWith(DevSdkIgnoreRunner::class)
+@DevSdkIgnoreRule.IgnoreUpTo(Build.VERSION_CODES.R)
 class NetworkRankerTest {
-    private val ranker = NetworkRanker()
+    private val mRanker = NetworkRanker()
 
-    private fun makeNai(satisfy: Boolean, legacyScore: Int) =
+    private class TestScore(private val sc: FullScore, private val nc: NetworkCapabilities)
-            mock(NetworkAgentInfo::class.java).also {
+            : NetworkRanker.Scoreable {
-                doReturn(satisfy).`when`(it).satisfies(any())
+        override fun getScore() = sc
-                val fs = FullScore(legacyScore, 0 /* policies */, KEEP_CONNECTED_NONE)
+        override fun getCapsNoCopy(): NetworkCapabilities = nc
-                doReturn(fs).`when`(it).getScore()
-                val nc = NetworkCapabilities.Builder().build()
-                doReturn(nc).`when`(it).getCapsNoCopy()
     }
 
     @Test
-    fun testGetBestNetwork() {
+    fun testYieldToBadWiFiOneCell() {
-        val scores = listOf(20, 50, 90, 60, 23, 68)
+        // Only cell, it wins
-        val nais = scores.map { makeNai(true, it) }
+        val winner = TestScore(score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
-        val bestNetwork = nais[2] // The one with the top score
+                caps(TRANSPORT_CELLULAR))
-        val someRequest = mock(NetworkRequest::class.java)
+        val scores = listOf(winner)
-        assertEquals(bestNetwork, ranker.getBestNetwork(someRequest, nais, bestNetwork))
+        assertEquals(winner, mRanker.getBestNetworkByPolicy(scores, null))
     }
 
     @Test
-    fun testIgnoreNonSatisfying() {
+    fun testYieldToBadWiFiOneCellOneBadWiFi() {
-        val nais = listOf(makeNai(true, 20), makeNai(true, 50), makeNai(false, 90),
+        // Bad wifi wins against yielding validated cell
-                makeNai(false, 60), makeNai(true, 23), makeNai(false, 68))
+        val winner = TestScore(score(POLICY_EVER_VALIDATED_NOT_AVOIDED_WHEN_BAD),
-        val bestNetwork = nais[1] // Top score that's satisfying
+                caps(TRANSPORT_WIFI))
-        val someRequest = mock(NetworkRequest::class.java)
+        val scores = listOf(
-        assertEquals(bestNetwork, ranker.getBestNetwork(someRequest, nais, nais[1]))
+                winner,
+                TestScore(score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
+                        caps(TRANSPORT_CELLULAR))
+        )
+        assertEquals(winner, mRanker.getBestNetworkByPolicy(scores, null))
     }
 
     @Test
-    fun testNoMatch() {
+    fun testYieldToBadWiFiOneCellTwoBadWiFi() {
-        val nais = listOf(makeNai(false, 20), makeNai(false, 50), makeNai(false, 90))
+        // Bad wifi wins against yielding validated cell. Prefer the one that's primary.
-        val someRequest = mock(NetworkRequest::class.java)
+        val winner = TestScore(score(POLICY_EVER_VALIDATED_NOT_AVOIDED_WHEN_BAD,
-        assertNull(ranker.getBestNetwork(someRequest, nais, null))
+                POLICY_TRANSPORT_PRIMARY), caps(TRANSPORT_WIFI))
+        val scores = listOf(
+                winner,
+                TestScore(score(POLICY_EVER_VALIDATED_NOT_AVOIDED_WHEN_BAD),
+                        caps(TRANSPORT_WIFI)),
+                TestScore(score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
+                        caps(TRANSPORT_CELLULAR))
+        )
+        assertEquals(winner, mRanker.getBestNetworkByPolicy(scores, null))
     }
 
     @Test
-    fun testEmpty() {
+    fun testYieldToBadWiFiOneCellTwoBadWiFiOneNotAvoided() {
-        val someRequest = mock(NetworkRequest::class.java)
+        // Bad wifi ever validated wins against bad wifi that never was validated (or was
-        assertNull(ranker.getBestNetwork(someRequest, emptyList(), null))
+        // avoided when bad).
+        val winner = TestScore(score(POLICY_EVER_VALIDATED_NOT_AVOIDED_WHEN_BAD),
+                caps(TRANSPORT_WIFI))
+        val scores = listOf(
+                winner,
+                TestScore(score(), caps(TRANSPORT_WIFI)),
+                TestScore(score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
+                        caps(TRANSPORT_CELLULAR))
+        )
+        assertEquals(winner, mRanker.getBestNetworkByPolicy(scores, null))
     }
 
-    // Make sure the ranker is "stable" (as in stable sort), that is, it always returns the FIRST
-    // network satisfying the request if multiple of them have the same score.
     @Test
-    fun testStable() {
+    fun testYieldToBadWiFiOneCellOneBadWiFiOneGoodWiFi() {
-        val nais1 = listOf(makeNai(true, 30), makeNai(true, 30), makeNai(true, 30),
+        // Good wifi wins
-                makeNai(true, 30), makeNai(true, 30), makeNai(true, 30))
+        val winner = TestScore(score(POLICY_EVER_VALIDATED_NOT_AVOIDED_WHEN_BAD,
-        val someRequest = mock(NetworkRequest::class.java)
+                POLICY_IS_VALIDATED), caps(TRANSPORT_WIFI))
-        assertEquals(nais1[0], ranker.getBestNetwork(someRequest, nais1, nais1[0]))
+        val scores = listOf(
+                winner,
+                TestScore(score(POLICY_EVER_VALIDATED_NOT_AVOIDED_WHEN_BAD,
+                        POLICY_TRANSPORT_PRIMARY), caps(TRANSPORT_WIFI)),
+                TestScore(score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
+                        caps(TRANSPORT_CELLULAR))
+        )
+        assertEquals(winner, mRanker.getBestNetworkByPolicy(scores, null))
+    }
 
-        val nais2 = listOf(makeNai(true, 30), makeNai(true, 50), makeNai(true, 20),
+    @Test
-                makeNai(true, 50), makeNai(true, 50), makeNai(true, 40))
+    fun testYieldToBadWiFiTwoCellsOneBadWiFi() {
-        assertEquals(nais2[1], ranker.getBestNetwork(someRequest, nais2, nais2[1]))
+        // Cell that doesn't yield wins over cell that yields and bad wifi
+        val winner = TestScore(score(POLICY_IS_VALIDATED), caps(TRANSPORT_CELLULAR))
+        val scores = listOf(
+                winner,
+                TestScore(score(POLICY_EVER_VALIDATED_NOT_AVOIDED_WHEN_BAD,
+                        POLICY_TRANSPORT_PRIMARY), caps(TRANSPORT_WIFI)),
+                TestScore(score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
+                        caps(TRANSPORT_CELLULAR))
+        )
+        assertEquals(winner, mRanker.getBestNetworkByPolicy(scores, null))
+    }
+
+    @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 scores = listOf(
+                winner,
+                TestScore(score(POLICY_EVER_VALIDATED_NOT_AVOIDED_WHEN_BAD,
+                        POLICY_TRANSPORT_PRIMARY), caps(TRANSPORT_WIFI)),
+                TestScore(score(POLICY_IS_VALIDATED), caps(TRANSPORT_CELLULAR)),
+                TestScore(score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
+                        caps(TRANSPORT_CELLULAR))
+        )
+        assertEquals(winner, mRanker.getBestNetworkByPolicy(scores, null))
+    }
+
+    @Test
+    fun testYieldToBadWiFiOneExitingGoodWiFi() {
+        // Yielding cell wins over good exiting wifi
+        val winner = TestScore(score(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))
+        )
+        assertEquals(winner, mRanker.getBestNetworkByPolicy(scores, null))
+    }
+
+    @Test
+    fun testYieldToBadWiFiOneExitingBadWiFi() {
+        // Yielding cell wins over bad exiting wifi
+        val winner = TestScore(score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
+                caps(TRANSPORT_CELLULAR))
+        val scores = listOf(
+                winner,
+                TestScore(score(POLICY_EVER_VALIDATED_NOT_AVOIDED_WHEN_BAD,
+                        POLICY_EXITING), caps(TRANSPORT_WIFI))
+        )
+        assertEquals(winner, mRanker.getBestNetworkByPolicy(scores, null))
     }
 }