Tell the factory it is already serving a request.

This is a cherry-pick of ag/607226 that has been rebased on
top of four years of changes and with comments addressed.

Gives each factory a serial number and propogates it to every
NetworkAgent so when a score comes back indicating a request is
being handled the factory can account for it properly.

Without this, a new request that's already handled by a network
offered by a factory will not cause an increment of the factorys
ref count. Concretely this results in issues like the RAT icon
not being displayed in spite of the network actually being up
and usable.

This will be ported to AOSP as soon as possible, but immediately
some master-only WiFi tests need to be adjusted with this change
which would not let me submit to AOSP.

Bug: 18637384
Bug: 29030667
Test: manual
Test: atest frameworks/opt/telephony/tests/telephonytests
Test: atest frameworks-net
Test: atest CtsNetTestCases CtsHostsideNetworkTests
Change-Id: I597ac588f76dd507512ff02868fd1310b7e63f7e

9 files changed, 301 insertions, 104 deletions

diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java
index 3bae12e93745..ef8ca9e3cb67 100644
--- a/core/java/android/net/ConnectivityManager.java
+++ b/core/java/android/net/ConnectivityManager.java
@@ -2918,11 +2918,11 @@ public class ConnectivityManager {
         }
     }
 
-    /** {@hide} */
+    /** {@hide} - returns the factory serial number */
     @UnsupportedAppUsage
-    public void registerNetworkFactory(Messenger messenger, String name) {
+    public int registerNetworkFactory(Messenger messenger, String name) {
         try {
-            mService.registerNetworkFactory(messenger, name);
+            return mService.registerNetworkFactory(messenger, name);
         } catch (RemoteException e) {
             throw e.rethrowFromSystemServer();
         }
@@ -2938,6 +2938,10 @@ public class ConnectivityManager {
         }
     }
 
+    // TODO : remove this method. It is a stopgap measure to help sheperding a number
+    // of dependent changes that would conflict throughout the automerger graph. Having this
+    // temporarily helps with the process of going through with all these dependent changes across
+    // the entire tree.
     /**
      * @hide
      * Register a NetworkAgent with ConnectivityService.
@@ -2945,8 +2949,20 @@ public class ConnectivityManager {
      */
     public int registerNetworkAgent(Messenger messenger, NetworkInfo ni, LinkProperties lp,
             NetworkCapabilities nc, int score, NetworkMisc misc) {
+        return registerNetworkAgent(messenger, ni, lp, nc, score, misc,
+                NetworkFactory.SerialNumber.NONE);
+    }
+
+    /**
+     * @hide
+     * Register a NetworkAgent with ConnectivityService.
+     * @return NetID corresponding to NetworkAgent.
+     */
+    public int registerNetworkAgent(Messenger messenger, NetworkInfo ni, LinkProperties lp,
+            NetworkCapabilities nc, int score, NetworkMisc misc, int factorySerialNumber) {
         try {
-            return mService.registerNetworkAgent(messenger, ni, lp, nc, score, misc);
+            return mService.registerNetworkAgent(messenger, ni, lp, nc, score, misc,
+                    factorySerialNumber);
         } catch (RemoteException e) {
             throw e.rethrowFromSystemServer();
         }
diff --git a/core/java/android/net/IConnectivityManager.aidl b/core/java/android/net/IConnectivityManager.aidl
index fd7360fd4c17..f88adc2c64b7 100644
--- a/core/java/android/net/IConnectivityManager.aidl
+++ b/core/java/android/net/IConnectivityManager.aidl
@@ -139,14 +139,14 @@ interface IConnectivityManager
 
     void setAirplaneMode(boolean enable);
 
-    void registerNetworkFactory(in Messenger messenger, in String name);
+    int registerNetworkFactory(in Messenger messenger, in String name);
 
     boolean requestBandwidthUpdate(in Network network);
 
     void unregisterNetworkFactory(in Messenger messenger);
 
     int registerNetworkAgent(in Messenger messenger, in NetworkInfo ni, in LinkProperties lp,
-            in NetworkCapabilities nc, int score, in NetworkMisc misc);
+            in NetworkCapabilities nc, int score, in NetworkMisc misc, in int factorySerialNumber);
 
     NetworkRequest requestNetwork(in NetworkCapabilities networkCapabilities,
             in Messenger messenger, int timeoutSec, in IBinder binder, int legacy);
diff --git a/core/java/android/net/NetworkAgent.java b/core/java/android/net/NetworkAgent.java
index 99bfc140f140..204c25f01d01 100644
--- a/core/java/android/net/NetworkAgent.java
+++ b/core/java/android/net/NetworkAgent.java
@@ -58,6 +58,7 @@ public abstract class NetworkAgent extends Handler {
     private static final long BW_REFRESH_MIN_WIN_MS = 500;
     private boolean mPollLceScheduled = false;
     private AtomicBoolean mPollLcePending = new AtomicBoolean(false);
+    public final int mFactorySerialNumber;
 
     private static final int BASE = Protocol.BASE_NETWORK_AGENT;
 
@@ -193,16 +194,31 @@ public abstract class NetworkAgent extends Handler {
      */
     public static final int CMD_PREVENT_AUTOMATIC_RECONNECT = BASE + 15;
 
+    // TODO : remove these two constructors. They are a stopgap measure to help sheperding a number
+    // of dependent changes that would conflict throughout the automerger graph. Having these
+    // temporarily helps with the process of going through with all these dependent changes across
+    // the entire tree.
     public NetworkAgent(Looper looper, Context context, String logTag, NetworkInfo ni,
             NetworkCapabilities nc, LinkProperties lp, int score) {
-        this(looper, context, logTag, ni, nc, lp, score, null);
+        this(looper, context, logTag, ni, nc, lp, score, null, NetworkFactory.SerialNumber.NONE);
     }
-
     public NetworkAgent(Looper looper, Context context, String logTag, NetworkInfo ni,
             NetworkCapabilities nc, LinkProperties lp, int score, NetworkMisc misc) {
+        this(looper, context, logTag, ni, nc, lp, score, misc, NetworkFactory.SerialNumber.NONE);
+    }
+
+    public NetworkAgent(Looper looper, Context context, String logTag, NetworkInfo ni,
+            NetworkCapabilities nc, LinkProperties lp, int score, int factorySerialNumber) {
+        this(looper, context, logTag, ni, nc, lp, score, null, factorySerialNumber);
+    }
+
+    public NetworkAgent(Looper looper, Context context, String logTag, NetworkInfo ni,
+            NetworkCapabilities nc, LinkProperties lp, int score, NetworkMisc misc,
+            int factorySerialNumber) {
         super(looper);
         LOG_TAG = logTag;
         mContext = context;
+        mFactorySerialNumber = factorySerialNumber;
         if (ni == null || nc == null || lp == null) {
             throw new IllegalArgumentException();
         }
@@ -211,7 +227,8 @@ public abstract class NetworkAgent extends Handler {
         ConnectivityManager cm = (ConnectivityManager)mContext.getSystemService(
                 Context.CONNECTIVITY_SERVICE);
         netId = cm.registerNetworkAgent(new Messenger(this), new NetworkInfo(ni),
-                new LinkProperties(lp), new NetworkCapabilities(nc), score, misc);
+                new LinkProperties(lp), new NetworkCapabilities(nc), score, misc,
+                factorySerialNumber);
     }
 
     @Override
diff --git a/core/java/android/net/NetworkFactory.java b/core/java/android/net/NetworkFactory.java
index 181cab41e928..0dfe7a495738 100644
--- a/core/java/android/net/NetworkFactory.java
+++ b/core/java/android/net/NetworkFactory.java
@@ -32,6 +32,7 @@ import com.android.internal.util.Protocol;
 
 import java.io.FileDescriptor;
 import java.io.PrintWriter;
+import java.util.concurrent.atomic.AtomicInteger;
 
 /**
  * A NetworkFactory is an entity that creates NetworkAgent objects.
@@ -49,6 +50,20 @@ import java.io.PrintWriter;
  * @hide
  **/
 public class NetworkFactory extends Handler {
+    /** @hide */
+    public static class SerialNumber {
+        // Guard used by no network factory.
+        public static final int NONE = -1;
+        // A hardcoded serial number for NetworkAgents representing VPNs. These agents are
+        // not created by any factory, so they use this constant for clarity instead of NONE.
+        public static final int VPN = -2;
+        private static final AtomicInteger sNetworkFactorySerialNumber = new AtomicInteger(1);
+        /** Returns a unique serial number for a factory. */
+        public static final int nextSerialNumber() {
+            return sNetworkFactorySerialNumber.getAndIncrement();
+        }
+    }
+
     private static final boolean DBG = true;
     private static final boolean VDBG = false;
 
@@ -64,7 +79,7 @@ public class NetworkFactory extends Handler {
      * disregard any that it will never be able to service, for example
      * those requiring a different bearer.
      * msg.obj = NetworkRequest
-     * msg.arg1 = score - the score of the any network currently satisfying this
+     * msg.arg1 = score - the score of the network currently satisfying this
      *            request.  If this bearer knows in advance it cannot
      *            exceed this score it should not try to connect, holding the request
      *            for the future.
@@ -74,6 +89,8 @@ public class NetworkFactory extends Handler {
      *            with the same NetworkRequest but an updated score.
      *            Also, network conditions may change for this bearer
      *            allowing for a better score in the future.
+     * msg.arg2 = the serial number of the factory currently responsible for the
+     *            NetworkAgent handling this request, or SerialNumber.NONE if none.
      */
     public static final int CMD_REQUEST_NETWORK = BASE;
 
@@ -107,6 +124,7 @@ public class NetworkFactory extends Handler {
 
     private int mRefCount = 0;
     private Messenger mMessenger = null;
+    private int mSerialNumber;
 
     @UnsupportedAppUsage
     public NetworkFactory(Looper looper, Context context, String logTag,
@@ -121,7 +139,8 @@ public class NetworkFactory extends Handler {
         if (DBG) log("Registering NetworkFactory");
         if (mMessenger == null) {
             mMessenger = new Messenger(this);
-            ConnectivityManager.from(mContext).registerNetworkFactory(mMessenger, LOG_TAG);
+            mSerialNumber = ConnectivityManager.from(mContext).registerNetworkFactory(mMessenger,
+                    LOG_TAG);
         }
     }
 
@@ -137,7 +156,7 @@ public class NetworkFactory extends Handler {
     public void handleMessage(Message msg) {
         switch (msg.what) {
             case CMD_REQUEST_NETWORK: {
-                handleAddRequest((NetworkRequest)msg.obj, msg.arg1);
+                handleAddRequest((NetworkRequest) msg.obj, msg.arg1, msg.arg2);
                 break;
             }
             case CMD_CANCEL_REQUEST: {
@@ -159,11 +178,13 @@ public class NetworkFactory extends Handler {
         public final NetworkRequest request;
         public int score;
         public boolean requested; // do we have a request outstanding, limited by score
+        public int factorySerialNumber;
 
-        public NetworkRequestInfo(NetworkRequest request, int score) {
+        NetworkRequestInfo(NetworkRequest request, int score, int factorySerialNumber) {
             this.request = request;
             this.score = score;
             this.requested = false;
+            this.factorySerialNumber = factorySerialNumber;
         }
 
         @Override
@@ -172,16 +193,51 @@ public class NetworkFactory extends Handler {
         }
     }
 
+    /**
+     * Add a NetworkRequest that the bearer may want to attempt to satisfy.
+     * @see #CMD_REQUEST_NETWORK
+     *
+     * @param request the request to handle.
+     * @param score the score of the NetworkAgent currently satisfying this request.
+     * @param servingFactorySerialNumber the serial number of the NetworkFactory that
+     *         created the NetworkAgent currently satisfying this request.
+     */
+    // TODO : remove this method. It is a stopgap measure to help sheperding a number
+    // of dependent changes that would conflict throughout the automerger graph. Having this
+    // temporarily helps with the process of going through with all these dependent changes across
+    // the entire tree.
     @VisibleForTesting
     protected void handleAddRequest(NetworkRequest request, int score) {
+        handleAddRequest(request, score, SerialNumber.NONE);
+    }
+
+    /**
+     * Add a NetworkRequest that the bearer may want to attempt to satisfy.
+     * @see #CMD_REQUEST_NETWORK
+     *
+     * @param request the request to handle.
+     * @param score the score of the NetworkAgent currently satisfying this request.
+     * @param servingFactorySerialNumber the serial number of the NetworkFactory that
+     *         created the NetworkAgent currently satisfying this request.
+     */
+    @VisibleForTesting
+    protected void handleAddRequest(NetworkRequest request, int score,
+            int servingFactorySerialNumber) {
         NetworkRequestInfo n = mNetworkRequests.get(request.requestId);
         if (n == null) {
-            if (DBG) log("got request " + request + " with score " + score);
-            n = new NetworkRequestInfo(request, score);
+            if (DBG) {
+                log("got request " + request + " with score " + score
+                        + " and serial " + servingFactorySerialNumber);
+            }
+            n = new NetworkRequestInfo(request, score, servingFactorySerialNumber);
             mNetworkRequests.put(n.request.requestId, n);
         } else {
-            if (VDBG) log("new score " + score + " for exisiting request " + request);
+            if (VDBG) {
+                log("new score " + score + " for exisiting request " + request
+                        + " with serial " + servingFactorySerialNumber);
+            }
             n.score = score;
+            n.factorySerialNumber = servingFactorySerialNumber;
         }
         if (VDBG) log("  my score=" + mScore + ", my filter=" + mCapabilityFilter);
 
@@ -231,16 +287,19 @@ public class NetworkFactory extends Handler {
     }
 
     private void evalRequest(NetworkRequestInfo n) {
-        if (VDBG) log("evalRequest");
-        if (n.requested == false && n.score < mScore &&
-                n.request.networkCapabilities.satisfiedByNetworkCapabilities(
-                mCapabilityFilter) && acceptRequest(n.request, n.score)) {
+        if (VDBG) {
+            log("evalRequest");
+            log(" n.requests = " + n.requested);
+            log(" n.score = " + n.score);
+            log(" mScore = " + mScore);
+            log(" n.factorySerialNumber = " + n.factorySerialNumber);
+            log(" mSerialNumber = " + mSerialNumber);
+        }
+        if (shouldNeedNetworkFor(n)) {
             if (VDBG) log("  needNetworkFor");
             needNetworkFor(n.request, n.score);
             n.requested = true;
-        } else if (n.requested == true &&
-                (n.score > mScore || n.request.networkCapabilities.satisfiedByNetworkCapabilities(
-                mCapabilityFilter) == false || acceptRequest(n.request, n.score) == false)) {
+        } else if (shouldReleaseNetworkFor(n)) {
             if (VDBG) log("  releaseNetworkFor");
             releaseNetworkFor(n.request);
             n.requested = false;
@@ -249,10 +308,39 @@ public class NetworkFactory extends Handler {
         }
     }
 
+    private boolean shouldNeedNetworkFor(NetworkRequestInfo n) {
+        // If this request is already tracked, it doesn't qualify for need
+        return !n.requested
+            // If the score of this request is higher or equal to that of this factory and some
+            // other factory is responsible for it, then this factory should not track the request
+            // because it has no hope of satisfying it.
+            && (n.score < mScore || n.factorySerialNumber == mSerialNumber)
+            // If this factory can't satisfy the capability needs of this request, then it
+            // should not be tracked.
+            && n.request.networkCapabilities.satisfiedByNetworkCapabilities(mCapabilityFilter)
+            // Finally if the concrete implementation of the factory rejects the request, then
+            // don't track it.
+            && acceptRequest(n.request, n.score);
+    }
+
+    private boolean shouldReleaseNetworkFor(NetworkRequestInfo n) {
+        // Don't release a request that's not tracked.
+        return n.requested
+            // The request should be released if it can't be satisfied by this factory. That
+            // means either of the following conditions are met :
+            // - Its score is too high to be satisfied by this factory and it's not already
+            //   assigned to the factory
+            // - This factory can't satisfy the capability needs of the request
+            // - The concrete implementation of the factory rejects the request
+            && ((n.score > mScore && n.factorySerialNumber != mSerialNumber)
+                    || !n.request.networkCapabilities.satisfiedByNetworkCapabilities(
+                            mCapabilityFilter)
+                    || !acceptRequest(n.request, n.score));
+    }
+
     private void evalRequests() {
         for (int i = 0; i < mNetworkRequests.size(); i++) {
             NetworkRequestInfo n = mNetworkRequests.valueAt(i);
-
             evalRequest(n);
         }
     }
@@ -280,16 +368,6 @@ public class NetworkFactory extends Handler {
         if (--mRefCount == 0) stopNetwork();
     }
 
-
-    public void addNetworkRequest(NetworkRequest networkRequest, int score) {
-        sendMessage(obtainMessage(CMD_REQUEST_NETWORK,
-                new NetworkRequestInfo(networkRequest, score)));
-    }
-
-    public void removeNetworkRequest(NetworkRequest networkRequest) {
-        sendMessage(obtainMessage(CMD_CANCEL_REQUEST, networkRequest));
-    }
-
     @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P, trackingBug = 115609023)
     public void setScoreFilter(int score) {
         sendMessage(obtainMessage(CMD_SET_SCORE, score, 0));
@@ -304,6 +382,10 @@ public class NetworkFactory extends Handler {
         return mNetworkRequests.size();
     }
 
+    public int getSerialNumber() {
+        return mSerialNumber;
+    }
+
     protected void log(String s) {
         Log.d(LOG_TAG, s);
     }
@@ -321,10 +403,11 @@ public class NetworkFactory extends Handler {
 
     @Override
     public String toString() {
-        StringBuilder sb = new StringBuilder("{").append(LOG_TAG).append(" - ScoreFilter=").
-                append(mScore).append(", Filter=").append(mCapabilityFilter).append(", requests=").
-                append(mNetworkRequests.size()).append(", refCount=").append(mRefCount).
-                append("}");
+        StringBuilder sb = new StringBuilder("{").append(LOG_TAG).append(" - mSerialNumber=")
+                .append(mSerialNumber).append(", ScoreFilter=")
+                .append(mScore).append(", Filter=").append(mCapabilityFilter).append(", requests=")
+                .append(mNetworkRequests.size()).append(", refCount=").append(mRefCount)
+                .append("}");
         return sb.toString();
     }
 }
diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index c8e5d2bdb769..de7c8cc7a6e7 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -79,6 +79,7 @@ import android.net.Network;
 import android.net.NetworkAgent;
 import android.net.NetworkCapabilities;
 import android.net.NetworkConfig;
+import android.net.NetworkFactory;
 import android.net.NetworkInfo;
 import android.net.NetworkInfo.DetailedState;
 import android.net.NetworkMisc;
@@ -2773,8 +2774,17 @@ public class ConnectivityService extends IConnectivityManager.Stub
                 for (NetworkRequestInfo nri : mNetworkRequests.values()) {
                     if (nri.request.isListen()) continue;
                     NetworkAgentInfo nai = getNetworkForRequest(nri.request.requestId);
-                    ac.sendMessage(android.net.NetworkFactory.CMD_REQUEST_NETWORK,
-                            (nai != null ? nai.getCurrentScore() : 0), 0, nri.request);
+                    final int score;
+                    final int serial;
+                    if (nai != null) {
+                        score = nai.getCurrentScore();
+                        serial = nai.factorySerialNumber;
+                    } else {
+                        score = 0;
+                        serial = NetworkFactory.SerialNumber.NONE;
+                    }
+                    ac.sendMessage(android.net.NetworkFactory.CMD_REQUEST_NETWORK, score, serial,
+                            nri.request);
                 }
             } else {
                 loge("Error connecting NetworkFactory");
@@ -2871,7 +2881,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
             NetworkAgentInfo currentNetwork = getNetworkForRequest(request.requestId);
             if (currentNetwork != null && currentNetwork.network.netId == nai.network.netId) {
                 clearNetworkForRequest(request.requestId);
-                sendUpdatedScoreToFactories(request, 0);
+                sendUpdatedScoreToFactories(request, null);
             }
         }
         nai.clearLingerState();
@@ -2947,7 +2957,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
         }
         rematchAllNetworksAndRequests(null, 0);
         if (nri.request.isRequest() && getNetworkForRequest(nri.request.requestId) == null) {
-            sendUpdatedScoreToFactories(nri.request, 0);
+            sendUpdatedScoreToFactories(nri.request, null);
         }
     }
 
@@ -4532,11 +4542,14 @@ public class ConnectivityService extends IConnectivityManager.Stub
         public final String name;
         public final Messenger messenger;
         public final AsyncChannel asyncChannel;
+        public final int factorySerialNumber;
 
-        public NetworkFactoryInfo(String name, Messenger messenger, AsyncChannel asyncChannel) {
+        NetworkFactoryInfo(String name, Messenger messenger, AsyncChannel asyncChannel,
+                int factorySerialNumber) {
             this.name = name;
             this.messenger = messenger;
             this.asyncChannel = asyncChannel;
+            this.factorySerialNumber = factorySerialNumber;
         }
     }
 
@@ -4897,10 +4910,12 @@ public class ConnectivityService extends IConnectivityManager.Stub
     }
 
     @Override
-    public void registerNetworkFactory(Messenger messenger, String name) {
+    public int registerNetworkFactory(Messenger messenger, String name) {
         enforceConnectivityInternalPermission();
-        NetworkFactoryInfo nfi = new NetworkFactoryInfo(name, messenger, new AsyncChannel());
+        NetworkFactoryInfo nfi = new NetworkFactoryInfo(name, messenger, new AsyncChannel(),
+                NetworkFactory.SerialNumber.nextSerialNumber());
         mHandler.sendMessage(mHandler.obtainMessage(EVENT_REGISTER_NETWORK_FACTORY, nfi));
+        return nfi.factorySerialNumber;
     }
 
     private void handleRegisterNetworkFactory(NetworkFactoryInfo nfi) {
@@ -4992,9 +5007,35 @@ public class ConnectivityService extends IConnectivityManager.Stub
         return nri.request.requestId == mDefaultRequest.requestId;
     }
 
+    // TODO : remove this method. It's a stopgap measure to help sheperding a number of dependent
+    // changes that would conflict throughout the automerger graph. Having this method temporarily
+    // helps with the process of going through with all these dependent changes across the entire
+    // tree.
     public int registerNetworkAgent(Messenger messenger, NetworkInfo networkInfo,
             LinkProperties linkProperties, NetworkCapabilities networkCapabilities,
             int currentScore, NetworkMisc networkMisc) {
+        return registerNetworkAgent(messenger, networkInfo, linkProperties, networkCapabilities,
+                currentScore, networkMisc, NetworkFactory.SerialNumber.NONE);
+    }
+
+    /**
+     * Register a new agent with ConnectivityService to handle a network.
+     *
+     * @param messenger a messenger for ConnectivityService to contact the agent asynchronously.
+     * @param networkInfo the initial info associated with this network. It can be updated later :
+     *         see {@link #updateNetworkInfo}.
+     * @param linkProperties the initial link properties of this network. They can be updated
+     *         later : see {@link #updateLinkProperties}.
+     * @param networkCapabilities the initial capabilites of this network. They can be updated
+     *         later : see {@link #updateNetworkCapabilities}.
+     * @param currentScore the initial score of the network. See
+     *         {@link NetworkAgentInfo#getCurrentScore}.
+     * @param networkMisc metadata about the network. This is never updated.
+     * @param factorySerialNumber the serial number of the factory owning this NetworkAgent.
+     */
+    public int registerNetworkAgent(Messenger messenger, NetworkInfo networkInfo,
+            LinkProperties linkProperties, NetworkCapabilities networkCapabilities,
+            int currentScore, NetworkMisc networkMisc, int factorySerialNumber) {
         enforceConnectivityInternalPermission();
 
         LinkProperties lp = new LinkProperties(linkProperties);
@@ -5004,7 +5045,8 @@ public class ConnectivityService extends IConnectivityManager.Stub
         final NetworkCapabilities nc = new NetworkCapabilities(networkCapabilities);
         final NetworkAgentInfo nai = new NetworkAgentInfo(messenger, new AsyncChannel(),
                 new Network(reserveNetId()), new NetworkInfo(networkInfo), lp, nc, currentScore,
-                mContext, mTrackerHandler, new NetworkMisc(networkMisc), this, mNetd, mNMS);
+                mContext, mTrackerHandler, new NetworkMisc(networkMisc), this, mNetd, mNMS,
+                factorySerialNumber);
         // Make sure the network capabilities reflect what the agent info says.
         nai.networkCapabilities = mixInCapabilities(nai, nc);
         final String extraInfo = networkInfo.getExtraInfo();
@@ -5420,17 +5462,23 @@ public class ConnectivityService extends IConnectivityManager.Stub
             NetworkRequest nr = nai.requestAt(i);
             // Don't send listening requests to factories. b/17393458
             if (nr.isListen()) continue;
-            sendUpdatedScoreToFactories(nr, nai.getCurrentScore());
+            sendUpdatedScoreToFactories(nr, nai);
         }
     }
 
-    private void sendUpdatedScoreToFactories(NetworkRequest networkRequest, int score) {
+    private void sendUpdatedScoreToFactories(NetworkRequest networkRequest, NetworkAgentInfo nai) {
+        int score = 0;
+        int serial = 0;
+        if (nai != null) {
+            score = nai.getCurrentScore();
+            serial = nai.factorySerialNumber;
+        }
         if (VDBG || DDBG){
             log("sending new Min Network Score(" + score + "): " + networkRequest.toString());
         }
         for (NetworkFactoryInfo nfi : mNetworkFactoryInfos.values()) {
-            nfi.asyncChannel.sendMessage(android.net.NetworkFactory.CMD_REQUEST_NETWORK, score, 0,
-                    networkRequest);
+            nfi.asyncChannel.sendMessage(android.net.NetworkFactory.CMD_REQUEST_NETWORK, score,
+                    serial, networkRequest);
         }
     }
 
@@ -5706,7 +5754,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
                     // TODO - this could get expensive if we have a lot of requests for this
                     // network.  Think about if there is a way to reduce this.  Push
                     // netid->request mapping to each factory?
-                    sendUpdatedScoreToFactories(nri.request, score);
+                    sendUpdatedScoreToFactories(nri.request, newNetwork);
                     if (isDefaultRequest(nri)) {
                         isNewDefault = true;
                         oldDefaultNetwork = currentNetwork;
@@ -5730,7 +5778,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
                 newNetwork.removeRequest(nri.request.requestId);
                 if (currentNetwork == newNetwork) {
                     clearNetworkForRequest(nri.request.requestId);
-                    sendUpdatedScoreToFactories(nri.request, 0);
+                    sendUpdatedScoreToFactories(nri.request, null);
                 } else {
                     Slog.wtf(TAG, "BUG: Removing request " + nri.request.requestId + " from " +
                             newNetwork.name() +
diff --git a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java
index d0cff25dbf05..cd4ce2d142bb 100644
--- a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java
+++ b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java
@@ -235,6 +235,8 @@ public class NetworkAgentInfo implements Comparable<NetworkAgentInfo> {
     public final Messenger messenger;
     public final AsyncChannel asyncChannel;
 
+    public final int factorySerialNumber;
+
     // Used by ConnectivityService to keep track of 464xlat.
     public Nat464Xlat clatd;
 
@@ -252,7 +254,7 @@ public class NetworkAgentInfo implements Comparable<NetworkAgentInfo> {
     public NetworkAgentInfo(Messenger messenger, AsyncChannel ac, Network net, NetworkInfo info,
             LinkProperties lp, NetworkCapabilities nc, int score, Context context, Handler handler,
             NetworkMisc misc, ConnectivityService connService, INetd netd,
-            INetworkManagementService nms) {
+            INetworkManagementService nms, int factorySerialNumber) {
         this.messenger = messenger;
         asyncChannel = ac;
         network = net;
@@ -266,6 +268,7 @@ public class NetworkAgentInfo implements Comparable<NetworkAgentInfo> {
         mContext = context;
         mHandler = handler;
         networkMisc = misc;
+        this.factorySerialNumber = factorySerialNumber;
     }
 
     /**
diff --git a/services/core/java/com/android/server/connectivity/Vpn.java b/services/core/java/com/android/server/connectivity/Vpn.java
index 250884431440..7e95f10752bd 100644
--- a/services/core/java/com/android/server/connectivity/Vpn.java
+++ b/services/core/java/com/android/server/connectivity/Vpn.java
@@ -54,6 +54,7 @@ import android.net.LocalSocketAddress;
 import android.net.Network;
 import android.net.NetworkAgent;
 import android.net.NetworkCapabilities;
+import android.net.NetworkFactory;
 import android.net.NetworkInfo;
 import android.net.NetworkInfo.DetailedState;
 import android.net.NetworkMisc;
@@ -942,7 +943,8 @@ public class Vpn {
         try {
             mNetworkAgent = new NetworkAgent(mLooper, mContext, NETWORKTYPE /* logtag */,
                     mNetworkInfo, mNetworkCapabilities, lp,
-                    ConnectivityConstants.VPN_DEFAULT_SCORE, networkMisc) {
+                    ConnectivityConstants.VPN_DEFAULT_SCORE, networkMisc,
+                    NetworkFactory.SerialNumber.VPN) {
                             @Override
                             public void unwanted() {
                                 // We are user controlled, not driven by NetworkRequest.
diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java
index 036c5dcc8926..3127d745b55d 100644
--- a/tests/net/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java
@@ -495,7 +495,7 @@ public class ConnectivityServiceTest {
 
             mNetworkAgent = new NetworkAgent(mHandlerThread.getLooper(), mServiceContext,
                     "Mock-" + typeName, mNetworkInfo, mNetworkCapabilities,
-                    linkProperties, mScore, new NetworkMisc()) {
+                    linkProperties, mScore, new NetworkMisc(), NetworkFactory.SerialNumber.NONE) {
                 @Override
                 public void unwanted() { mDisconnected.open(); }
 
@@ -725,7 +725,7 @@ public class ConnectivityServiceTest {
     /**
      * A NetworkFactory that allows tests to wait until any in-flight NetworkRequest add or remove
      * operations have been processed. Before ConnectivityService can add or remove any requests,
-     * the factory must be told to expect those operations by calling expectAddRequests or
+     * the factory must be told to expect those operations by calling expectAddRequestsWithScores or
      * expectRemoveRequests.
      */
     private static class MockNetworkFactory extends NetworkFactory {
@@ -734,19 +734,22 @@ public class ConnectivityServiceTest {
         private final AtomicBoolean mNetworkStarted = new AtomicBoolean(false);
 
         // Used to expect that requests be removed or added on a separate thread, without sleeping.
-        // Callers can call either expectAddRequests() or expectRemoveRequests() exactly once, then
-        // cause some other thread to add or remove requests, then call waitForRequests(). We can
-        // either expect requests to be added or removed, but not both, because CountDownLatch can
-        // only count in one direction.
-        private CountDownLatch mExpectations;
+        // Callers can call either expectAddRequestsWithScores() or expectRemoveRequests() exactly
+        // once, then cause some other thread to add or remove requests, then call
+        // waitForRequests().
+        // It is not possible to wait for both add and remove requests. When adding, the queue
+        // contains the expected score. When removing, the value is unused, all matters is the
+        // number of objects in the queue.
+        private final LinkedBlockingQueue<Integer> mExpectations;
 
         // Whether we are currently expecting requests to be added or removed. Valid only if
-        // mExpectations is non-null.
+        // mExpectations is non-empty.
         private boolean mExpectingAdditions;
 
         public MockNetworkFactory(Looper looper, Context context, String logTag,
                 NetworkCapabilities filter) {
             super(looper, context, logTag, filter);
+            mExpectations = new LinkedBlockingQueue<>();
         }
 
         public int getMyRequestCount() {
@@ -778,69 +781,82 @@ public class ConnectivityServiceTest {
         }
 
         @Override
-        protected void handleAddRequest(NetworkRequest request, int score) {
-            // If we're expecting anything, we must be expecting additions.
-            if (mExpectations != null && !mExpectingAdditions) {
-                fail("Can't add requests while expecting requests to be removed");
-            }
-
-            // Add the request.
-            super.handleAddRequest(request, score);
+        protected void handleAddRequest(NetworkRequest request, int score,
+                int factorySerialNumber) {
+            synchronized (mExpectations) {
+                final Integer expectedScore = mExpectations.poll(); // null if the queue is empty
+
+                assertNotNull("Added more requests than expected (" + request + " score : "
+                        + score + ")", expectedScore);
+                // If we're expecting anything, we must be expecting additions.
+                if (!mExpectingAdditions) {
+                    fail("Can't add requests while expecting requests to be removed");
+                }
+                if (expectedScore != score) {
+                    fail("Expected score was " + expectedScore + " but actual was " + score
+                            + " in added request");
+                }
 
-            // Reduce the number of request additions we're waiting for.
-            if (mExpectingAdditions) {
-                assertTrue("Added more requests than expected", mExpectations.getCount() > 0);
-                mExpectations.countDown();
+                // Add the request.
+                super.handleAddRequest(request, score, factorySerialNumber);
+                mExpectations.notify();
             }
         }
 
         @Override
         protected void handleRemoveRequest(NetworkRequest request) {
-            // If we're expecting anything, we must be expecting removals.
-            if (mExpectations != null && mExpectingAdditions) {
-                fail("Can't remove requests while expecting requests to be added");
-            }
+            synchronized (mExpectations) {
+                final Integer expectedScore = mExpectations.poll(); // null if the queue is empty
 
-            // Remove the request.
-            super.handleRemoveRequest(request);
+                assertTrue("Removed more requests than expected", expectedScore != null);
+                // If we're expecting anything, we must be expecting removals.
+                if (mExpectingAdditions) {
+                    fail("Can't remove requests while expecting requests to be added");
+                }
 
-            // Reduce the number of request removals we're waiting for.
-            if (!mExpectingAdditions) {
-                assertTrue("Removed more requests than expected", mExpectations.getCount() > 0);
-                mExpectations.countDown();
+                // Remove the request.
+                super.handleRemoveRequest(request);
+                mExpectations.notify();
             }
         }
 
         private void assertNoExpectations() {
-            if (mExpectations != null) {
-                fail("Can't add expectation, " + mExpectations.getCount() + " already pending");
+            if (mExpectations.size() != 0) {
+                fail("Can't add expectation, " + mExpectations.size() + " already pending");
             }
         }
 
-        // Expects that count requests will be added.
-        public void expectAddRequests(final int count) {
+        // Expects that requests with the specified scores will be added.
+        public void expectAddRequestsWithScores(final int... scores) {
             assertNoExpectations();
             mExpectingAdditions = true;
-            mExpectations = new CountDownLatch(count);
+            for (int score : scores) {
+                mExpectations.add(score);
+            }
         }
 
         // Expects that count requests will be removed.
         public void expectRemoveRequests(final int count) {
             assertNoExpectations();
             mExpectingAdditions = false;
-            mExpectations = new CountDownLatch(count);
+            for (int i = 0; i < count; ++i) {
+                mExpectations.add(0); // For removals the score is ignored so any value will do.
+            }
         }
 
         // Waits for the expected request additions or removals to happen within a timeout.
         public void waitForRequests() throws InterruptedException {
-            assertNotNull("Nothing to wait for", mExpectations);
-            mExpectations.await(TIMEOUT_MS, TimeUnit.MILLISECONDS);
-            final long count = mExpectations.getCount();
+            final long deadline = SystemClock.elapsedRealtime() + TIMEOUT_MS;
+            synchronized (mExpectations) {
+                while (mExpectations.size() > 0 && SystemClock.elapsedRealtime() < deadline) {
+                    mExpectations.wait(deadline - SystemClock.elapsedRealtime());
+                }
+            }
+            final long count = mExpectations.size();
             final String msg = count + " requests still not " +
                     (mExpectingAdditions ? "added" : "removed") +
                     " after " + TIMEOUT_MS + " ms";
             assertEquals(msg, 0, count);
-            mExpectations = null;
         }
 
         public void waitForNetworkRequests(final int count) throws InterruptedException {
@@ -2271,6 +2287,12 @@ public class ConnectivityServiceTest {
         callback.expectCallback(CallbackState.LOST, mEthernetNetworkAgent);
     }
 
+    private int[] makeIntArray(final int size, final int value) {
+        final int[] array = new int[size];
+        Arrays.fill(array, value);
+        return array;
+    }
+
     private void tryNetworkFactoryRequests(int capability) throws Exception {
         // Verify NOT_RESTRICTED is set appropriately
         final NetworkCapabilities nc = new NetworkRequest.Builder().addCapability(capability)
@@ -2292,7 +2314,7 @@ public class ConnectivityServiceTest {
                 mServiceContext, "testFactory", filter);
         testFactory.setScoreFilter(40);
         ConditionVariable cv = testFactory.getNetworkStartedCV();
-        testFactory.expectAddRequests(1);
+        testFactory.expectAddRequestsWithScores(0);
         testFactory.register();
         testFactory.waitForNetworkRequests(1);
         int expectedRequestCount = 1;
@@ -2303,7 +2325,7 @@ public class ConnectivityServiceTest {
             assertFalse(testFactory.getMyStartRequested());
             NetworkRequest request = new NetworkRequest.Builder().addCapability(capability).build();
             networkCallback = new NetworkCallback();
-            testFactory.expectAddRequests(1);
+            testFactory.expectAddRequestsWithScores(0);  // New request
             mCm.requestNetwork(request, networkCallback);
             expectedRequestCount++;
             testFactory.waitForNetworkRequests(expectedRequestCount);
@@ -2323,7 +2345,7 @@ public class ConnectivityServiceTest {
         // When testAgent connects, ConnectivityService will re-send us all current requests with
         // the new score. There are expectedRequestCount such requests, and we must wait for all of
         // them.
-        testFactory.expectAddRequests(expectedRequestCount);
+        testFactory.expectAddRequestsWithScores(makeIntArray(expectedRequestCount, 50));
         testAgent.connect(false);
         testAgent.addCapability(capability);
         waitFor(cv);
@@ -2331,7 +2353,7 @@ public class ConnectivityServiceTest {
         assertFalse(testFactory.getMyStartRequested());
 
         // Bring in a bunch of requests.
-        testFactory.expectAddRequests(10);
+        testFactory.expectAddRequestsWithScores(makeIntArray(10, 50));
         assertEquals(expectedRequestCount, testFactory.getMyRequestCount());
         ConnectivityManager.NetworkCallback[] networkCallbacks =
                 new ConnectivityManager.NetworkCallback[10];
@@ -2354,8 +2376,11 @@ public class ConnectivityServiceTest {
 
         // Drop the higher scored network.
         cv = testFactory.getNetworkStartedCV();
+        // With the default network disconnecting, the requests are sent with score 0 to factories.
+        testFactory.expectAddRequestsWithScores(makeIntArray(expectedRequestCount, 0));
         testAgent.disconnect();
         waitFor(cv);
+        testFactory.waitForNetworkRequests(expectedRequestCount);
         assertEquals(expectedRequestCount, testFactory.getMyRequestCount());
         assertTrue(testFactory.getMyStartRequested());
 
@@ -3177,22 +3202,23 @@ public class ConnectivityServiceTest {
         testFactory.setScoreFilter(40);
 
         // Register the factory and expect it to start looking for a network.
-        testFactory.expectAddRequests(1);
+        testFactory.expectAddRequestsWithScores(0);  // Score 0 as the request is not served yet.
         testFactory.register();
         testFactory.waitForNetworkRequests(1);
         assertTrue(testFactory.getMyStartRequested());
 
         // Bring up wifi. The factory stops looking for a network.
         mWiFiNetworkAgent = new MockNetworkAgent(TRANSPORT_WIFI);
-        testFactory.expectAddRequests(2);  // Because the default request changes score twice.
+        // Score 60 - 40 penalty for not validated yet, then 60 when it validates
+        testFactory.expectAddRequestsWithScores(20, 60);
         mWiFiNetworkAgent.connect(true);
-        testFactory.waitForNetworkRequests(1);
+        testFactory.waitForRequests();
         assertFalse(testFactory.getMyStartRequested());
 
         ContentResolver cr = mServiceContext.getContentResolver();
 
         // Turn on mobile data always on. The factory starts looking again.
-        testFactory.expectAddRequests(1);
+        testFactory.expectAddRequestsWithScores(0);  // Always on requests comes up with score 0
         setAlwaysOnNetworks(true);
         testFactory.waitForNetworkRequests(2);
         assertTrue(testFactory.getMyStartRequested());
@@ -3200,7 +3226,7 @@ public class ConnectivityServiceTest {
         // Bring up cell data and check that the factory stops looking.
         assertLength(1, mCm.getAllNetworks());
         mCellNetworkAgent = new MockNetworkAgent(TRANSPORT_CELLULAR);
-        testFactory.expectAddRequests(2);  // Because the cell request changes score twice.
+        testFactory.expectAddRequestsWithScores(10, 50);  // Unvalidated, then validated
         mCellNetworkAgent.connect(true);
         cellNetworkCallback.expectAvailableThenValidatedCallbacks(mCellNetworkAgent);
         testFactory.waitForNetworkRequests(2);
diff --git a/tests/net/java/com/android/server/connectivity/LingerMonitorTest.java b/tests/net/java/com/android/server/connectivity/LingerMonitorTest.java
index 9578ded1a089..e877a8f7e6c1 100644
--- a/tests/net/java/com/android/server/connectivity/LingerMonitorTest.java
+++ b/tests/net/java/com/android/server/connectivity/LingerMonitorTest.java
@@ -35,6 +35,7 @@ import android.net.ConnectivityManager;
 import android.net.INetd;
 import android.net.Network;
 import android.net.NetworkCapabilities;
+import android.net.NetworkFactory;
 import android.net.NetworkInfo;
 import android.net.NetworkMisc;
 import android.net.NetworkStack;
@@ -356,7 +357,8 @@ public class LingerMonitorTest {
         caps.addCapability(0);
         caps.addTransportType(transport);
         NetworkAgentInfo nai = new NetworkAgentInfo(null, null, new Network(netId), info, null,
-                caps, 50, mCtx, null, mMisc, mConnService, mNetd, mNMS);
+                caps, 50, mCtx, null, mMisc, mConnService, mNetd, mNMS,
+                NetworkFactory.SerialNumber.NONE);
         nai.everValidated = true;
         return nai;
     }