Revert "[NS D05] Rework how to tear down networks"

Revert submission 10343065

Reason for revert: This is part of a feature that was punted out of R.
Reverted Changes:
Ic9a3d3363:[NS D05] Rework how to tear down networks
I7d815f873:[NS D06] Implement more policies
I561098476:[NS D07] Use the unmodified legacy score

Change-Id: I8d2696d15999265d79abfc9163e7e5ccad873cfe

3 files changed, 39 insertions, 57 deletions

diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index f7eabac3b21f..7083281eaa7e 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -3430,16 +3430,16 @@ public class ConnectivityService extends IConnectivityManager.Stub
             // there is hope for it to become one if it validated, then it is needed.
             ensureRunningOnConnectivityServiceThread();
             if (nri.request.isRequest() && nai.satisfies(nri.request) &&
-                    (nai.isSatisfyingRequest(nri.request.requestId)
-                    // Note that canPossiblyBeat catches two important cases:
-                    // 1. Unvalidated slow networks will not be reaped when an unvalidated fast
-                    // network is currently satisfying the request. This is desirable for example
-                    // when cellular ends up validating but WiFi/Ethernet does not.
-                    // 2. Fast networks will not be reaped when a validated slow network is
-                    // currently satisfying the request. This is desirable for example when
-                    // Ethernet ends up validating and out scoring WiFi, or WiFi/Ethernet ends
-                    // up validating and out scoring cellular.
-                            || nai.canPossiblyBeat(nri.mSatisfier))) {
+                    (nai.isSatisfyingRequest(nri.request.requestId) ||
+                    // 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
+                    //    cellular ends up validating but WiFi does not.
+                    // 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.mSatisfier.getCurrentScore()
+                            < nai.getCurrentScoreAsValidated())) {
                 return false;
             }
         }
diff --git a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java
index 3860904a3fed..4612cfd0f7cb 100644
--- a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java
+++ b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java
@@ -16,8 +16,6 @@
 
 package com.android.server.connectivity;
 
-import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_METERED;
-import static android.net.NetworkCapabilities.NET_CAPABILITY_VALIDATED;
 import static android.net.NetworkCapabilities.transportNamesOf;
 
 import android.annotation.NonNull;
@@ -477,16 +475,24 @@ public class NetworkAgentInfo implements Comparable<NetworkAgentInfo> {
         return networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_VPN);
     }
 
-    /** Gets the current score */
-    public int getCurrentScore() {
+    private int getCurrentScore(boolean pretendValidated) {
+        // TODO: We may want to refactor this into a NetworkScore class that takes a base score from
+        // the NetworkAgent and signals from the NetworkAgent and uses those signals to modify the
+        // score.  The NetworkScore class would provide a nice place to centralize score constants
+        // so they are not scattered about the transports.
+
         // If this network is explicitly selected and the user has decided to use it even if it's
-        // unvalidated, give it the maximum score.
-        if (networkAgentConfig.explicitlySelected && networkAgentConfig.acceptUnvalidated) {
+        // unvalidated, give it the maximum score. Also give it the maximum score if it's explicitly
+        // selected and we're trying to see what its score could be. This ensures that we don't tear
+        // down an explicitly selected network before the user gets a chance to prefer it when
+        // a higher-scoring network (e.g., Ethernet) is available.
+        if (networkAgentConfig.explicitlySelected
+                && (networkAgentConfig.acceptUnvalidated || pretendValidated)) {
             return ConnectivityConstants.EXPLICITLY_SELECTED_NETWORK_SCORE;
         }
 
         int score = mNetworkScore.getLegacyScore();
-        if (!lastValidated && !ignoreWifiUnvalidationPenalty() && !isVPN()) {
+        if (!lastValidated && !pretendValidated && !ignoreWifiUnvalidationPenalty() && !isVPN()) {
             score -= ConnectivityConstants.UNVALIDATED_SCORE_PENALTY;
         }
         if (score < 0) score = 0;
@@ -502,6 +508,18 @@ public class NetworkAgentInfo implements Comparable<NetworkAgentInfo> {
         return isWifi && !avoidBadWifi && everValidated;
     }
 
+    // Get the current score for this Network.  This may be modified from what the
+    // NetworkAgent sent, as it has modifiers applied to it.
+    public int getCurrentScore() {
+        return getCurrentScore(false);
+    }
+
+    // Get the current score for this Network as if it was validated.  This may be modified from
+    // what the NetworkAgent sent, as it has modifiers applied to it.
+    public int getCurrentScoreAsValidated() {
+        return getCurrentScore(true);
+    }
+
     public void setNetworkScore(@NonNull NetworkScore ns) {
         mNetworkScore = ns;
     }
@@ -611,41 +629,6 @@ public class NetworkAgentInfo implements Comparable<NetworkAgentInfo> {
         mLingering = false;
     }
 
-    /**
-     * Returns whether this NAI has any chance of ever beating this other agent.
-     *
-     * The chief use case of this is the decision to tear down this network. ConnectivityService
-     * tears down networks that don't satisfy any request, unless they have a chance to beat any
-     * existing satisfier.
-     *
-     * @param other the agent to beat
-     * @return whether this should be given more time to try and beat the other agent
-     * TODO : remove this and migrate to a ranker-based approach
-     */
-    public boolean canPossiblyBeat(@NonNull final NetworkAgentInfo other) {
-        // Any explicitly selected network should be held on.
-        if (networkAgentConfig.explicitlySelected) return true;
-        // An outscored exiting network should be torn down.
-        if (mNetworkScore.isExiting()) return false;
-        // If this network is validated it can be torn down as it can't hope to be better than
-        // it already is.
-        if (networkCapabilities.hasCapability(NET_CAPABILITY_VALIDATED)) return false;
-        // If neither network is validated, keep both until at least one does.
-        if (!other.networkCapabilities.hasCapability(NET_CAPABILITY_VALIDATED)) return true;
-        // If this network is not metered but the other is, it should be preferable if it validates.
-        if (networkCapabilities.hasCapability(NET_CAPABILITY_NOT_METERED)
-                && !other.networkCapabilities.hasCapability(NET_CAPABILITY_NOT_METERED)) {
-            return true;
-        }
-
-        // If the control comes here :
-        // • This network is neither exiting or explicitly selected
-        // • This network is not validated, but the other is
-        // • This network is metered, or both networks are unmetered
-        // Keep it if it's expected to be faster than the other., should it validate.
-        return mNetworkScore.probablyFasterThan(other.mNetworkScore);
-    }
-
     public void dumpLingerTimers(PrintWriter pw) {
         for (LingerTimer timer : mLingerTimers) { pw.println(timer); }
     }
diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java
index 5aabc60b229f..08869751b93e 100644
--- a/tests/net/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java
@@ -2054,15 +2054,14 @@ public class ConnectivityServiceTest {
         assertEquals(mCellNetworkAgent.getNetwork(), mCm.getActiveNetwork());
         assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork());
 
-        // Bring up validated wifi.
+        // 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.connect(true);   // Score: 60
+        mWiFiNetworkAgent.adjustScore(50);
+        mWiFiNetworkAgent.connect(false);   // Score: 70
         callback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent);
-        // TODO: Investigate sending validated before losing.
         callback.expectCallback(CallbackEntry.LOSING, mCellNetworkAgent);
-        callback.expectCapabilitiesWith(NET_CAPABILITY_VALIDATED, mWiFiNetworkAgent);
-        defaultCallback.expectAvailableThenValidatedCallbacks(mWiFiNetworkAgent);
+        defaultCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent);
         assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetwork());
         assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork());