[NS13] Remove the last usage of the legacy int

Note that this requires removing part of a test, because
that part is testing that the a 50 bonus of the legacy
int is stronger than the validation penalty, which is
not a mechanic we want to have. When WiFi is unvalidated
and cell is unvalidated, cell should be kept in case it
validates, like is described in comments in
isNetworkPotentialSatisfier ; however this test is
checking that it *IS* reaped off if the wifi score is
strong enough. This should be incorrect, and should not
be tested, so this patch removes the check.

Test: ConnectivityServiceTest
Bug: 184834350
Merged-In: I3c2563d4ae4e3715d0c6270344ba8f7ef067872f
Merged-In: I8966abee59fea2d9f10f082aba87df6588b72762
Change-Id: I8966abee59fea2d9f10f082aba87df6588b72762
  (cherry-picked from ag/14127306)

service/src/com/android/server/ConnectivityService.java

@@ -4038,7 +4038,8 @@ public class ConnectivityService extends IConnectivityManager.Stub
                 // multilayer requests, returning as soon as a NetworkAgentInfo satisfies a request
                 // is important so as to not evaluate lower priority requests further in
                 // nri.mRequests.
-                final boolean isNetworkNeeded = candidate.isSatisfyingRequest(req.requestId)
+                final NetworkAgentInfo champion = req.equals(nri.getActiveRequest())
+                        ? nri.getSatisfier() : null;
                 // Note that this catches two important cases:
                 // 1. Unvalidated cellular will not be reaped when unvalidated WiFi
                 //    is currently satisfying the request.  This is desirable when
@@ -4046,9 +4047,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
                 // 2. Unvalidated WiFi will not be reaped when validated cellular
                 //    is currently satisfying the request.  This is desirable when
                 //    WiFi ends up validating and out scoring cellular.
-                        || nri.getSatisfier().getCurrentScore()
+                return mNetworkRanker.mightBeat(req, champion, candidate.getValidatedScoreable());
-                        < candidate.getCurrentScoreAsValidated();
-                return isNetworkNeeded;
             }
         }
 
@@ -7887,7 +7886,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
             @NonNull final Collection<NetworkRequestInfo> networkRequests) {
         final NetworkReassignment changes = new NetworkReassignment();
 
-        // Gather the list of all relevant agents and sort them by score.
+        // Gather the list of all relevant agents.
         final ArrayList<NetworkAgentInfo> nais = new ArrayList<>();
         for (final NetworkAgentInfo nai : mNetworkAgentInfos) {
             if (!nai.everConnected) {

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

@@ -258,6 +258,14 @@ public class FullScore {
                 keepConnectedReason);
     }
 
+    /**
+     * Returns this score but validated.
+     */
+    public FullScore asValidated() {
+        return new FullScore(mLegacyInt, mPolicies | (1L << POLICY_IS_VALIDATED),
+                mKeepConnectedReason);
+    }
+
     /**
      * For backward compatibility, get the legacy int.
      * This will be removed before S is published.

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

@@ -937,6 +937,26 @@ public class NetworkAgentInfo implements Comparable<NetworkAgentInfo>, NetworkRa
         return everValidated && !avoidUnvalidated;
     }
 
+    /**
+     * Returns a Scoreable identical to this NAI, but validated.
+     *
+     * This is useful to probe what scoring would be if this network validated, to know
+     * whether to provisionally keep a network that may or may not validate.
+     *
+     * @return a Scoreable identical to this NAI, but validated.
+     */
+    public NetworkRanker.Scoreable getValidatedScoreable() {
+        return new NetworkRanker.Scoreable() {
+            @Override public FullScore getScore() {
+                return mScore.asValidated();
+            }
+
+            @Override public NetworkCapabilities getCapsNoCopy() {
+                return networkCapabilities;
+            }
+        };
+    }
+
     /**
      * Return a {@link NetworkStateSnapshot} for this network.
      */

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

@@ -234,16 +234,17 @@ public class NetworkRanker {
         NetworkAgentInfo bestNetwork = null;
         int bestScore = Integer.MIN_VALUE;
         for (final NetworkAgentInfo nai : nais) {
-            if (nai.getCurrentScore() > bestScore) {
+            final int naiScore = nai.getCurrentScore();
+            if (naiScore > bestScore) {
                 bestNetwork = nai;
-                bestScore = nai.getCurrentScore();
+                bestScore = naiScore;
             }
         }
         return bestNetwork;
     }
 
     /**
-     * Returns whether an offer has a chance to beat a champion network for a request.
+     * Returns whether a {@link Scoreable} has a chance to beat a champion network for a request.
      *
      * Offers are sent by network providers when they think they might be able to make a network
      * with the characteristics contained in the offer. If the offer has no chance to beat
@@ -257,15 +258,15 @@ public class NetworkRanker {
      *
      * @param request The request to evaluate against.
      * @param champion The currently best network for this request.
-     * @param offer The offer.
+     * @param contestant The offer.
      * @return Whether the offer stands a chance to beat the champion.
      */
     public boolean mightBeat(@NonNull final NetworkRequest request,
             @Nullable final NetworkAgentInfo champion,
-            @NonNull final NetworkOffer offer) {
+            @NonNull final Scoreable contestant) {
         // If this network can't even satisfy the request then it can't beat anything, not
         // even an absence of network. It can't satisfy it anyway.
-        if (!request.canBeSatisfiedBy(offer.caps)) return false;
+        if (!request.canBeSatisfiedBy(contestant.getCapsNoCopy())) return false;
         // If there is no satisfying network, then this network can beat, because some network
         // is always better than no network.
         if (null == champion) return true;
@@ -274,25 +275,24 @@ public class NetworkRanker {
             // Otherwise rank them.
             final ArrayList<Scoreable> candidates = new ArrayList<>();
             candidates.add(champion);
-            candidates.add(offer);
+            candidates.add(contestant);
-            return offer == getBestNetworkByPolicy(candidates, champion);
+            return contestant == getBestNetworkByPolicy(candidates, champion);
         } else {
-            return mightBeatByLegacyInt(request, champion.getScore(), offer);
+            return mightBeatByLegacyInt(champion.getScore(), contestant);
         }
     }
 
     /**
-     * Returns whether an offer might beat a champion according to the legacy int.
+     * Returns whether a contestant might beat a champion according to the legacy int.
      */
-    public boolean mightBeatByLegacyInt(@NonNull final NetworkRequest request,
+    private boolean mightBeatByLegacyInt(@Nullable final FullScore championScore,
-            @Nullable final FullScore championScore,
+            @NonNull final Scoreable contestant) {
-            @NonNull final NetworkOffer offer) {
         final int offerIntScore;
-        if (offer.caps.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)) {
+        if (contestant.getCapsNoCopy().hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)) {
             // If the offer might have Internet access, then it might validate.
-            offerIntScore = offer.score.getLegacyIntAsValidated();
+            offerIntScore = contestant.getScore().getLegacyIntAsValidated();
         } else {
-            offerIntScore = offer.score.getLegacyInt();
+            offerIntScore = contestant.getScore().getLegacyInt();
         }
         return championScore.getLegacyInt() < offerIntScore;
     }

tests/unit/java/com/android/server/ConnectivityServiceTest.java

@@ -2683,25 +2683,6 @@ public class ConnectivityServiceTest {
         assertEquals(mCellNetworkAgent.getNetwork(), mCm.getActiveNetwork());
         assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork());
 
-        // Bring up wifi with a score of 70.
-        // Cell is lingered because it would not satisfy any request, even if it validated.
-        mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI);
-        mWiFiNetworkAgent.adjustScore(50);
-        mWiFiNetworkAgent.connect(false);   // Score: 70
-        callback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent);
-        callback.expectCallback(CallbackEntry.LOSING, mCellNetworkAgent);
-        defaultCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent);
-        assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetwork());
-        assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork());
-
-        // Tear down wifi.
-        mWiFiNetworkAgent.disconnect();
-        callback.expectCallback(CallbackEntry.LOST, mWiFiNetworkAgent);
-        defaultCallback.expectCallback(CallbackEntry.LOST, mWiFiNetworkAgent);
-        defaultCallback.expectAvailableCallbacksUnvalidated(mCellNetworkAgent);
-        assertEquals(mCellNetworkAgent.getNetwork(), mCm.getActiveNetwork());
-        assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork());
-
         // Bring up wifi, then validate it. Previous versions would immediately tear down cell, but
         // it's arguably correct to linger it, since it was the default network before it validated.
         mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI);
@@ -3017,11 +2998,17 @@ public class ConnectivityServiceTest {
         callback.expectCapabilitiesWith(NET_CAPABILITY_VALIDATED, mWiFiNetworkAgent);
         assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetwork());
 
-        // BUG: the network will no longer linger, even though it's validated and outscored.
-        // TODO: fix this.
         mEthernetNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_ETHERNET);
         mEthernetNetworkAgent.connect(true);
+        // BUG: with the legacy int-based scoring code, the network will no longer linger, even
+        // though it's validated and outscored.
+        // The new policy-based scoring code fixes this.
+        // TODO: remove the line below and replace with the three commented lines when
+        // the policy-based scoring code is turned on.
         callback.expectAvailableThenValidatedCallbacks(mEthernetNetworkAgent);
+        // callback.expectAvailableCallbacksUnvalidated(mEthernetNetworkAgent);
+        // callback.expectCallback(CallbackEntry.LOSING, mWiFiNetworkAgent);
+        // callback.expectCapabilitiesWith(NET_CAPABILITY_VALIDATED, mEthernetNetworkAgent);
         assertEquals(mEthernetNetworkAgent.getNetwork(), mCm.getActiveNetwork());
         callback.assertNoCallback();