Split POLICY_EVER_VALIDATED_NOT_AVOIDED_WHEN_BAD into two

This flag conflates these two things, but it's a lot clearer
if they are separate and evaluated at the end.

Moreover, a new policy will make use only of one of them,
so having them separate is also useful going forward.

Test: FrameworksNetTests

Change-Id: Ia47b3974277cf76153a53d7e8a0e969c90ba78f4

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

@@ -23,7 +23,6 @@ import static android.net.NetworkCapabilities.TRANSPORT_VPN;
 import static android.net.NetworkScore.KEEP_CONNECTED_NONE;
 import static android.net.NetworkScore.POLICY_YIELD_TO_BAD_WIFI;
 
-import android.annotation.IntDef;
 import android.annotation.NonNull;
 import android.net.NetworkAgentConfig;
 import android.net.NetworkCapabilities;
@@ -35,8 +34,6 @@ import android.util.SparseArray;
 import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.util.MessageUtils;
 
-import java.lang.annotation.Retention;
-import java.lang.annotation.RetentionPolicy;
 import java.util.StringJoiner;
 
 /**
@@ -49,53 +46,44 @@ import java.util.StringJoiner;
 public class FullScore {
     private static final String TAG = FullScore.class.getSimpleName();
 
-    /** @hide */
-    @Retention(RetentionPolicy.SOURCE)
-    @IntDef(prefix = {"POLICY_"}, value = {
-            POLICY_IS_VALIDATED,
-            POLICY_IS_VPN,
-            POLICY_EVER_USER_SELECTED,
-            POLICY_ACCEPT_UNVALIDATED,
-            POLICY_IS_UNMETERED
-    })
-    public @interface Policy {
-    }
-
     // Agent-managed policies are in NetworkScore. They start from 1.
     // CS-managed policies, counting from 63 downward
     // This network is validated. CS-managed because the source of truth is in NetworkCapabilities.
     /** @hide */
     public static final int POLICY_IS_VALIDATED = 63;
 
+    // This network has been validated at least once since it was connected.
+    /** @hide */
+    public static final int POLICY_EVER_VALIDATED = 62;
+
     // This is a VPN and behaves as one for scoring purposes.
     /** @hide */
-    public static final int POLICY_IS_VPN = 62;
+    public static final int POLICY_IS_VPN = 61;
 
     // This network has been selected by the user manually from settings or a 3rd party app
     // at least once. @see NetworkAgentConfig#explicitlySelected.
     /** @hide */
-    public static final int POLICY_EVER_USER_SELECTED = 61;
+    public static final int POLICY_EVER_USER_SELECTED = 60;
 
     // The user has indicated in UI that this network should be used even if it doesn't
     // validate. @see NetworkAgentConfig#acceptUnvalidated.
     /** @hide */
-    public static final int POLICY_ACCEPT_UNVALIDATED = 60;
+    public static final int POLICY_ACCEPT_UNVALIDATED = 59;
+
+    // The user explicitly said in UI to avoid this network when unvalidated.
+    // TODO : remove setAvoidUnvalidated and instead disconnect the network when the user
+    // chooses to move away from this network, and remove this flag.
+    /** @hide */
+    public static final int POLICY_AVOIDED_WHEN_UNVALIDATED = 58;
 
     // This network is unmetered. @see NetworkCapabilities.NET_CAPABILITY_NOT_METERED.
     /** @hide */
-    public static final int POLICY_IS_UNMETERED = 59;
+    public static final int POLICY_IS_UNMETERED = 57;
 
     // This network is invincible. This is useful for offers until there is an API to listen
     // to requests.
     /** @hide */
-    public static final int POLICY_IS_INVINCIBLE = 58;
+    public static final int POLICY_IS_INVINCIBLE = 56;
-
-    // This network has been validated at least once since it was connected, but not explicitly
-    // avoided in UI.
-    // TODO : remove setAvoidUnvalidated and instead disconnect the network when the user
-    // chooses to move away from this network, and remove this flag.
-    /** @hide */
-    public static final int POLICY_EVER_VALIDATED_NOT_AVOIDED_WHEN_BAD = 57;
 
     // The network agent has communicated that this network no longer functions, and the underlying
     // native network has been destroyed. The network will still be reported to clients as connected
@@ -103,7 +91,7 @@ public class FullScore {
     // This network should lose to an identical network that has not been destroyed, but should
     // otherwise be scored exactly the same.
     /** @hide */
-    public static final int POLICY_IS_DESTROYED = 56;
+    public static final int POLICY_IS_DESTROYED = 55;
 
     // To help iterate when printing
     @VisibleForTesting
@@ -154,6 +142,7 @@ public class FullScore {
      * @param caps the NetworkCapabilities of the network
      * @param config the NetworkAgentConfig of the network
      * @param everValidated whether this network has ever validated
+     * @param avoidUnvalidated whether the user said in UI to avoid this network when unvalidated
      * @param yieldToBadWiFi whether this network yields to a previously validated wifi gone bad
      * @param destroyed whether this network has been destroyed pending a replacement connecting
      * @return a FullScore that is appropriate to use for ranking.
@@ -163,18 +152,19 @@ public class FullScore {
     // connectivity for backward compatibility.
     public static FullScore fromNetworkScore(@NonNull final NetworkScore score,
             @NonNull final NetworkCapabilities caps, @NonNull final NetworkAgentConfig config,
-            final boolean everValidated, final boolean yieldToBadWiFi, final boolean destroyed) {
+            final boolean everValidated, final boolean avoidUnvalidated,
+            final boolean yieldToBadWiFi, final boolean destroyed) {
         return withPolicies(score.getPolicies(),
                 score.getKeepConnectedReason(),
                 caps.hasCapability(NET_CAPABILITY_VALIDATED),
-                caps.hasTransport(TRANSPORT_VPN),
+                everValidated, caps.hasTransport(TRANSPORT_VPN),
-                caps.hasCapability(NET_CAPABILITY_NOT_METERED),
-                everValidated,
                 config.explicitlySelected,
                 config.acceptUnvalidated,
+                avoidUnvalidated,
+                caps.hasCapability(NET_CAPABILITY_NOT_METERED),
                 yieldToBadWiFi,
-                destroyed,
+                false /* invincible */, // only prospective scores can be invincible
-                false /* invincible */); // only prospective scores can be invincible
+                destroyed);
     }
 
     /**
@@ -194,25 +184,29 @@ public class FullScore {
             @NonNull final NetworkCapabilities caps, final boolean yieldToBadWiFi) {
         // If the network offers Internet access, it may validate.
         final boolean mayValidate = caps.hasCapability(NET_CAPABILITY_INTERNET);
-        // VPN transports are known in advance.
-        final boolean vpn = caps.hasTransport(TRANSPORT_VPN);
-        // Prospective scores are always unmetered, because unmetered networks are stronger
-        // than metered networks, and it's not known in advance whether the network is metered.
-        final boolean unmetered = true;
         // If the offer may validate, then it should be considered to have validated at some point
         final boolean everValidated = mayValidate;
+        // VPN transports are known in advance.
+        final boolean vpn = caps.hasTransport(TRANSPORT_VPN);
         // The network hasn't been chosen by the user (yet, at least).
         final boolean everUserSelected = false;
         // Don't assume the user will accept unvalidated connectivity.
         final boolean acceptUnvalidated = false;
+        // A prospective network is never avoided when unvalidated, because the user has never
+        // had the opportunity to say so in UI.
+        final boolean avoidUnvalidated = false;
+        // Prospective scores are always unmetered, because unmetered networks are stronger
+        // than metered networks, and it's not known in advance whether the network is metered.
+        final boolean unmetered = true;
         // A network can only be destroyed once it has connected.
         final boolean destroyed = false;
         // A prospective score is invincible if the legacy int in the filter is over the maximum
         // score.
         final boolean invincible = score.getLegacyInt() > NetworkRanker.LEGACY_INT_MAX;
         return withPolicies(score.getPolicies(), KEEP_CONNECTED_NONE,
-                mayValidate, vpn, unmetered, everValidated, everUserSelected, acceptUnvalidated,
+                mayValidate, everValidated, vpn, everUserSelected, acceptUnvalidated,
-                yieldToBadWiFi, destroyed, invincible);
+                avoidUnvalidated, unmetered,
+                yieldToBadWiFi, invincible, destroyed);
     }
 
     /**
@@ -228,18 +222,19 @@ public class FullScore {
     public FullScore mixInScore(@NonNull final NetworkCapabilities caps,
             @NonNull final NetworkAgentConfig config,
             final boolean everValidated,
+            final boolean avoidUnvalidated,
             final boolean yieldToBadWifi,
             final boolean destroyed) {
         return withPolicies(mPolicies, mKeepConnectedReason,
                 caps.hasCapability(NET_CAPABILITY_VALIDATED),
-                caps.hasTransport(TRANSPORT_VPN),
+                everValidated, caps.hasTransport(TRANSPORT_VPN),
-                caps.hasCapability(NET_CAPABILITY_NOT_METERED),
-                everValidated,
                 config.explicitlySelected,
                 config.acceptUnvalidated,
+                avoidUnvalidated,
+                caps.hasCapability(NET_CAPABILITY_NOT_METERED),
                 yieldToBadWifi,
-                destroyed,
+                false /* invincible */, // only prospective scores can be invincible
-                false /* invincible */); // only prospective scores can be invincible
+                destroyed);
     }
 
     // TODO : this shouldn't manage bad wifi avoidance – instead this should be done by the
@@ -248,24 +243,26 @@ public class FullScore {
     private static FullScore withPolicies(final long externalPolicies,
             @KeepConnectedReason final int keepConnectedReason,
             final boolean isValidated,
-            final boolean isVpn,
-            final boolean isUnmetered,
             final boolean everValidated,
+            final boolean isVpn,
             final boolean everUserSelected,
             final boolean acceptUnvalidated,
+            final boolean avoidUnvalidated,
+            final boolean isUnmetered,
             final boolean yieldToBadWiFi,
-            final boolean destroyed,
+            final boolean invincible,
-            final boolean invincible) {
+            final boolean destroyed) {
         return new FullScore((externalPolicies & EXTERNAL_POLICIES_MASK)
                 | (isValidated       ? 1L << POLICY_IS_VALIDATED : 0)
+                | (everValidated     ? 1L << POLICY_EVER_VALIDATED : 0)
                 | (isVpn             ? 1L << POLICY_IS_VPN : 0)
-                | (isUnmetered       ? 1L << POLICY_IS_UNMETERED : 0)
-                | (everValidated     ? 1L << POLICY_EVER_VALIDATED_NOT_AVOIDED_WHEN_BAD : 0)
                 | (everUserSelected  ? 1L << POLICY_EVER_USER_SELECTED : 0)
                 | (acceptUnvalidated ? 1L << POLICY_ACCEPT_UNVALIDATED : 0)
+                | (avoidUnvalidated  ? 1L << POLICY_AVOIDED_WHEN_UNVALIDATED : 0)
+                | (isUnmetered       ? 1L << POLICY_IS_UNMETERED : 0)
                 | (yieldToBadWiFi    ? 1L << POLICY_YIELD_TO_BAD_WIFI : 0)
-                | (destroyed         ? 1L << POLICY_IS_DESTROYED : 0)
+                | (invincible        ? 1L << POLICY_IS_INVINCIBLE : 0)
-                | (invincible        ? 1L << POLICY_IS_INVINCIBLE : 0),
+                | (destroyed         ? 1L << POLICY_IS_DESTROYED : 0),
                 keepConnectedReason);
     }
 

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

@@ -970,8 +970,8 @@ public class NetworkAgentInfo implements NetworkRanker.Scoreable {
             @NonNull final NetworkCapabilities nc) {
         final NetworkCapabilities oldNc = networkCapabilities;
         networkCapabilities = nc;
-        mScore = mScore.mixInScore(networkCapabilities, networkAgentConfig, everValidatedForYield(),
+        mScore = mScore.mixInScore(networkCapabilities, networkAgentConfig, everValidated(),
-                yieldToBadWiFi(), isDestroyed());
+                0L != getAvoidUnvalidated(), yieldToBadWiFi(), isDestroyed());
         final NetworkMonitorManager nm = mNetworkMonitor;
         if (nm != null) {
             nm.notifyNetworkCapabilitiesChanged(nc);
@@ -1174,7 +1174,7 @@ public class NetworkAgentInfo implements NetworkRanker.Scoreable {
      */
     public void setScore(final NetworkScore score) {
         mScore = FullScore.fromNetworkScore(score, networkCapabilities, networkAgentConfig,
-                everValidatedForYield(), yieldToBadWiFi(), isDestroyed());
+                everValidated(), 0L == getAvoidUnvalidated(), yieldToBadWiFi(), isDestroyed());
     }
 
     /**
@@ -1184,11 +1184,7 @@ public class NetworkAgentInfo implements NetworkRanker.Scoreable {
      */
     public void updateScoreForNetworkAgentUpdate() {
         mScore = mScore.mixInScore(networkCapabilities, networkAgentConfig,
-                everValidatedForYield(), yieldToBadWiFi(), isDestroyed());
+                everValidated(), 0L != getAvoidUnvalidated(), yieldToBadWiFi(), isDestroyed());
-    }
-
-    private boolean everValidatedForYield() {
-        return everValidated() && 0L == mAvoidUnvalidated;
     }
 
     /**

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

@@ -26,8 +26,9 @@ 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_USER_SELECTED;
-import static com.android.server.connectivity.FullScore.POLICY_EVER_VALIDATED_NOT_AVOIDED_WHEN_BAD;
+import static com.android.server.connectivity.FullScore.POLICY_EVER_VALIDATED;
 import static com.android.server.connectivity.FullScore.POLICY_IS_DESTROYED;
 import static com.android.server.connectivity.FullScore.POLICY_IS_INVINCIBLE;
 import static com.android.server.connectivity.FullScore.POLICY_IS_VALIDATED;
@@ -104,7 +105,9 @@ public class NetworkRanker {
     }
 
     private <T extends Scoreable> boolean isBadWiFi(@NonNull final T candidate) {
-        return candidate.getScore().hasPolicy(POLICY_EVER_VALIDATED_NOT_AVOIDED_WHEN_BAD)
+        final FullScore score = candidate.getScore();
+        return score.hasPolicy(POLICY_EVER_VALIDATED)
+                && !score.hasPolicy(POLICY_AVOIDED_WHEN_UNVALIDATED)
                 && candidate.getCapsNoCopy().hasTransport(TRANSPORT_WIFI);
     }
 

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

@@ -66,7 +66,8 @@ class FullScoreTest {
             if (vpn) addTransportType(NetworkCapabilities.TRANSPORT_VPN)
             if (validated) addCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED)
         }.build()
-        return mixInScore(nc, nac, validated, false /* yieldToBadWifi */, destroyed)
+        return mixInScore(nc, nac, validated, false /* avoidUnvalidated */,
+                false /* yieldToBadWifi */, destroyed)
     }
 
     private val TAG = this::class.simpleName

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

@@ -25,7 +25,8 @@ 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.server.connectivity.FullScore.POLICY_EVER_VALIDATED_NOT_AVOIDED_WHEN_BAD
+import com.android.server.connectivity.FullScore.POLICY_AVOIDED_WHEN_UNVALIDATED
+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
@@ -61,8 +62,7 @@ class NetworkRankerTest {
     @Test
     fun testYieldToBadWiFiOneCellOneBadWiFi() {
         // Bad wifi wins against yielding validated cell
-        val winner = TestScore(score(POLICY_EVER_VALIDATED_NOT_AVOIDED_WHEN_BAD),
+        val winner = TestScore(score(POLICY_EVER_VALIDATED), caps(TRANSPORT_WIFI))
-                caps(TRANSPORT_WIFI))
         val scores = listOf(
                 winner,
                 TestScore(score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
@@ -71,15 +71,27 @@ class NetworkRankerTest {
         assertEquals(winner, mRanker.getBestNetworkByPolicy(scores, 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),
+                caps(TRANSPORT_CELLULAR))
+        val scores = listOf(
+                winner,
+                TestScore(score(POLICY_EVER_VALIDATED, POLICY_AVOIDED_WHEN_UNVALIDATED),
+                        caps(TRANSPORT_WIFI))
+        )
+        assertEquals(winner, mRanker.getBestNetworkByPolicy(scores, null))
+    }
+
     @Test
     fun testYieldToBadWiFiOneCellTwoBadWiFi() {
         // Bad wifi wins against yielding validated cell. Prefer the one that's primary.
-        val winner = TestScore(score(POLICY_EVER_VALIDATED_NOT_AVOIDED_WHEN_BAD,
+        val winner = TestScore(score(POLICY_EVER_VALIDATED,
                 POLICY_TRANSPORT_PRIMARY), caps(TRANSPORT_WIFI))
         val scores = listOf(
                 winner,
-                TestScore(score(POLICY_EVER_VALIDATED_NOT_AVOIDED_WHEN_BAD),
+                TestScore(score(POLICY_EVER_VALIDATED), caps(TRANSPORT_WIFI)),
-                        caps(TRANSPORT_WIFI)),
                 TestScore(score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
                         caps(TRANSPORT_CELLULAR))
         )
@@ -90,8 +102,7 @@ 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_NOT_AVOIDED_WHEN_BAD),
+        val winner = TestScore(score(POLICY_EVER_VALIDATED), caps(TRANSPORT_WIFI))
-                caps(TRANSPORT_WIFI))
         val scores = listOf(
                 winner,
                 TestScore(score(), caps(TRANSPORT_WIFI)),
@@ -104,12 +115,12 @@ class NetworkRankerTest {
     @Test
     fun testYieldToBadWiFiOneCellOneBadWiFiOneGoodWiFi() {
         // Good wifi wins
-        val winner = TestScore(score(POLICY_EVER_VALIDATED_NOT_AVOIDED_WHEN_BAD,
+        val winner = TestScore(score(POLICY_EVER_VALIDATED, POLICY_IS_VALIDATED),
-                POLICY_IS_VALIDATED), caps(TRANSPORT_WIFI))
+                caps(TRANSPORT_WIFI))
         val scores = listOf(
                 winner,
-                TestScore(score(POLICY_EVER_VALIDATED_NOT_AVOIDED_WHEN_BAD,
+                TestScore(score(POLICY_EVER_VALIDATED, POLICY_TRANSPORT_PRIMARY),
-                        POLICY_TRANSPORT_PRIMARY), caps(TRANSPORT_WIFI)),
+                        caps(TRANSPORT_WIFI)),
                 TestScore(score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
                         caps(TRANSPORT_CELLULAR))
         )
@@ -122,8 +133,8 @@ class NetworkRankerTest {
         val winner = TestScore(score(POLICY_IS_VALIDATED), caps(TRANSPORT_CELLULAR))
         val scores = listOf(
                 winner,
-                TestScore(score(POLICY_EVER_VALIDATED_NOT_AVOIDED_WHEN_BAD,
+                TestScore(score(POLICY_EVER_VALIDATED, POLICY_TRANSPORT_PRIMARY),
-                        POLICY_TRANSPORT_PRIMARY), caps(TRANSPORT_WIFI)),
+                        caps(TRANSPORT_WIFI)),
                 TestScore(score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
                         caps(TRANSPORT_CELLULAR))
         )
@@ -136,8 +147,8 @@ class NetworkRankerTest {
         val winner = TestScore(score(POLICY_IS_VALIDATED), caps(TRANSPORT_WIFI))
         val scores = listOf(
                 winner,
-                TestScore(score(POLICY_EVER_VALIDATED_NOT_AVOIDED_WHEN_BAD,
+                TestScore(score(POLICY_EVER_VALIDATED, POLICY_TRANSPORT_PRIMARY),
-                        POLICY_TRANSPORT_PRIMARY), caps(TRANSPORT_WIFI)),
+                        caps(TRANSPORT_WIFI)),
                 TestScore(score(POLICY_IS_VALIDATED), caps(TRANSPORT_CELLULAR)),
                 TestScore(score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
                         caps(TRANSPORT_CELLULAR))
@@ -164,8 +175,7 @@ class NetworkRankerTest {
                 caps(TRANSPORT_CELLULAR))
         val scores = listOf(
                 winner,
-                TestScore(score(POLICY_EVER_VALIDATED_NOT_AVOIDED_WHEN_BAD,
+                TestScore(score(POLICY_EVER_VALIDATED, POLICY_EXITING), caps(TRANSPORT_WIFI))
-                        POLICY_EXITING), caps(TRANSPORT_WIFI))
         )
         assertEquals(winner, mRanker.getBestNetworkByPolicy(scores, null))
     }