Refactor NetworkFactory.

Make NetworkFactory a concrete class and divide responsibilites between it and NetworkAgent.
Factory will track requests and by default give a single connect/disconnect api for ease
of use.  Then NetworkAgent is created and destroyed as needed with very simple logic.

Change-Id: I401c14a6e5466f2fc63b04219b97ff85bb9af291

core/java/android/net/NetworkAgent.java

@@ -24,85 +24,39 @@ import android.os.Messenger;
 import android.os.Parcel;
 import android.os.Parcelable;
 import android.util.Log;
-import android.util.SparseArray;
 
 import com.android.internal.util.AsyncChannel;
 import com.android.internal.util.Protocol;
 
+import java.util.ArrayList;
 import java.util.concurrent.atomic.AtomicBoolean;
 
 /**
- * A Utility class for handling NetworkRequests.
+ * A Utility class for handling for communicating between bearer-specific
- *
+ * code and ConnectivityService.
- * Created by bearer-specific code to handle tracking requests, scores,
- * network data and handle communicating with ConnectivityService.  Two
- * abstract methods: connect and disconnect are used to act on the
- * underlying bearer code.  Connect is called when we have a NetworkRequest
- * and our score is better than the current handling network's score, while
- * disconnect is used when ConnectivityService requests a disconnect.
  *
  * A bearer may have more than one NetworkAgent if it can simultaneously
  * support separate networks (IMS / Internet / MMS Apns on cellular, or
- * perhaps connections with different SSID or P2P for Wi-Fi).  The bearer
+ * perhaps connections with different SSID or P2P for Wi-Fi).
- * code should pass its NetworkAgents the NetworkRequests each NetworkAgent
- * can handle, demultiplexing for different network types.  The bearer code
- * can also filter out requests it can never handle.
  *
- * Each NetworkAgent needs to be given a score and NetworkCapabilities for
- * their potential network.  While disconnected, the NetworkAgent will check
- * each time its score changes or a NetworkRequest changes to see if
- * the NetworkAgent can provide a higher scored network for a NetworkRequest
- * that the NetworkAgent's NetworkCapabilties can satisfy.  This condition will
- * trigger a connect request via connect().  After connection, connection data
- * should be given to the NetworkAgent by the bearer, including LinkProperties
- * NetworkCapabilties and NetworkInfo.  After that the NetworkAgent will register
- * with ConnectivityService and forward the data on.
  * @hide
  */
 public abstract class NetworkAgent extends Handler {
-    private final SparseArray<NetworkRequestAndScore> mNetworkRequests = new SparseArray<>();
+    private volatile AsyncChannel mAsyncChannel;
-    private boolean mConnectionRequested = false;
-
-    private AsyncChannel mAsyncChannel;
     private final String LOG_TAG;
     private static final boolean DBG = true;
     private static final boolean VDBG = true;
-    // TODO - this class shouldn't cache data or it runs the risk of getting out of sync
-    // Make the API require each of these when any is updated so we have the data we need,
-    // without caching.
-    private LinkProperties mLinkProperties;
-    private NetworkInfo mNetworkInfo;
-    private NetworkCapabilities mNetworkCapabilities;
-    private int mNetworkScore;
-    private boolean mRegistered = false;
     private final Context mContext;
-    private AtomicBoolean mHasRequests = new AtomicBoolean(false);
+    private final ArrayList<Message>mPreConnectedQueue = new ArrayList<Message>();
-
-    // TODO - add a name member for logging purposes.
-
-    protected final Object mLockObj = new Object();
-
 
     private static final int BASE = Protocol.BASE_NETWORK_AGENT;
 
-    /**
-     * Sent by self to queue up a new/modified request.
-     * obj = NetworkRequestAndScore
-     */
-    private static final int CMD_ADD_REQUEST = BASE + 1;
-
-    /**
-     * Sent by self to queue up the removal of a request.
-     * obj = NetworkRequest
-     */
-    private static final int CMD_REMOVE_REQUEST = BASE + 2;
-
     /**
      * Sent by ConnectivityService to the NetworkAgent to inform it of
      * suspected connectivity problems on its network.  The NetworkAgent
      * should take steps to verify and correct connectivity.
      */
-    public static final int CMD_SUSPECT_BAD = BASE + 3;
+    public static final int CMD_SUSPECT_BAD = BASE;
 
     /**
      * Sent by the NetworkAgent (note the EVENT vs CMD prefix) to
@@ -110,84 +64,63 @@ public abstract class NetworkAgent extends Handler {
      * Sent when the NetworkInfo changes, mainly due to change of state.
      * obj = NetworkInfo
      */
-    public static final int EVENT_NETWORK_INFO_CHANGED = BASE + 4;
+    public static final int EVENT_NETWORK_INFO_CHANGED = BASE + 1;
 
     /**
      * Sent by the NetworkAgent to ConnectivityService to pass the current
      * NetworkCapabilties.
      * obj = NetworkCapabilities
      */
-    public static final int EVENT_NETWORK_CAPABILITIES_CHANGED = BASE + 5;
+    public static final int EVENT_NETWORK_CAPABILITIES_CHANGED = BASE + 2;
 
     /**
      * Sent by the NetworkAgent to ConnectivityService to pass the current
      * NetworkProperties.
      * obj = NetworkProperties
      */
-    public static final int EVENT_NETWORK_PROPERTIES_CHANGED = BASE + 6;
+    public static final int EVENT_NETWORK_PROPERTIES_CHANGED = BASE + 3;
 
     /**
      * Sent by the NetworkAgent to ConnectivityService to pass the current
      * network score.
-     * arg1 = network score int
+     * obj = network score Integer
      */
-    public static final int EVENT_NETWORK_SCORE_CHANGED = BASE + 7;
+    public static final int EVENT_NETWORK_SCORE_CHANGED = BASE + 4;
 
-    public NetworkAgent(Looper looper, Context context, String logTag) {
+    public NetworkAgent(Looper looper, Context context, String logTag, NetworkInfo ni,
+            NetworkCapabilities nc, LinkProperties lp, int score) {
         super(looper);
         LOG_TAG = logTag;
         mContext = context;
-    }
+        if (ni == null || nc == null || lp == null) {
-
+            throw new IllegalArgumentException();
-    /**
-     * When conditions are right, register with ConnectivityService.
-     * Connditions include having a well defined network and a request
-     * that justifies it.  The NetworkAgent will remain registered until
-     * disconnected.
-     * TODO - this should have all data passed in rather than caching
-     */
-    private void registerSelf() {
-        synchronized(mLockObj) {
-            if (!mRegistered && mConnectionRequested &&
-                    mNetworkInfo != null && mNetworkInfo.isConnected() &&
-                    mNetworkCapabilities != null &&
-                    mLinkProperties != null &&
-                    mNetworkScore != 0) {
-                if (DBG) log("Registering NetworkAgent");
-                mRegistered = true;
-                ConnectivityManager cm = (ConnectivityManager)mContext.getSystemService(
-                        Context.CONNECTIVITY_SERVICE);
-                cm.registerNetworkAgent(new Messenger(this), new NetworkInfo(mNetworkInfo),
-                        new LinkProperties(mLinkProperties),
-                        new NetworkCapabilities(mNetworkCapabilities), mNetworkScore);
-            } else if (DBG && !mRegistered) {
-                String err = "Not registering due to ";
-                if (mConnectionRequested == false) err += "no Connect requested ";
-                if (mNetworkInfo == null) err += "null NetworkInfo ";
-                if (mNetworkInfo != null && mNetworkInfo.isConnected() == false) {
-                    err += "NetworkInfo disconnected ";
-                }
-                if (mLinkProperties == null) err += "null LinkProperties ";
-                if (mNetworkCapabilities == null) err += "null NetworkCapabilities ";
-                if (mNetworkScore == 0) err += "null NetworkScore";
-                log(err);
-            }
         }
+
+        if (DBG) log("Registering NetworkAgent");
+        ConnectivityManager cm = (ConnectivityManager)mContext.getSystemService(
+                Context.CONNECTIVITY_SERVICE);
+        cm.registerNetworkAgent(new Messenger(this), new NetworkInfo(ni),
+                new LinkProperties(lp), new NetworkCapabilities(nc), score);
     }
 
     @Override
     public void handleMessage(Message msg) {
         switch (msg.what) {
             case AsyncChannel.CMD_CHANNEL_FULL_CONNECTION: {
-                synchronized (mLockObj) {
+                if (mAsyncChannel != null) {
-                    if (mAsyncChannel != null) {
+                    log("Received new connection while already connected!");
-                        log("Received new connection while already connected!");
+                } else {
-                    } else {
+                    if (DBG) log("NetworkAgent fully connected");
-                        if (DBG) log("NetworkAgent fully connected");
+                    AsyncChannel ac = new AsyncChannel();
-                        mAsyncChannel = new AsyncChannel();
+                    ac.connected(null, this, msg.replyTo);
-                        mAsyncChannel.connected(null, this, msg.replyTo);
+                    ac.replyToMessage(msg, AsyncChannel.CMD_CHANNEL_FULLY_CONNECTED,
-                        mAsyncChannel.replyToMessage(msg, AsyncChannel.CMD_CHANNEL_FULLY_CONNECTED,
+                            AsyncChannel.STATUS_SUCCESSFUL);
-                                AsyncChannel.STATUS_SUCCESSFUL);
+                    synchronized (mPreConnectedQueue) {
+                        mAsyncChannel = ac;
+                        for (Message m : mPreConnectedQueue) {
+                            ac.sendMessage(m);
+                        }
+                        mPreConnectedQueue.clear();
                     }
                 }
                 break;
@@ -199,213 +132,69 @@ public abstract class NetworkAgent extends Handler {
             }
             case AsyncChannel.CMD_CHANNEL_DISCONNECTED: {
                 if (DBG) log("NetworkAgent channel lost");
-                disconnect();
+                // let the client know CS is done with us.
-                clear();
+                unwanted();
+                synchronized (mPreConnectedQueue) {
+                    mAsyncChannel = null;
+                }
                 break;
             }
             case CMD_SUSPECT_BAD: {
                 log("Unhandled Message " + msg);
                 break;
             }
-            case CMD_ADD_REQUEST: {
-                handleAddRequest(msg);
-                break;
-            }
-            case CMD_REMOVE_REQUEST: {
-                handleRemoveRequest(msg);
-                break;
-            }
         }
     }
 
-    private void clear() {
+    private void queueOrSendMessage(int what, Object obj) {
-        synchronized(mLockObj) {
+        synchronized (mPreConnectedQueue) {
-            mNetworkRequests.clear();
+            if (mAsyncChannel != null) {
-            mHasRequests.set(false);
+                mAsyncChannel.sendMessage(what, obj);
-            mConnectionRequested = false;
+            } else {
-            mAsyncChannel = null;
+                Message msg = Message.obtain();
-            mRegistered = false;
+                msg.what = what;
-        }
+                msg.obj = obj;
-    }
+                mPreConnectedQueue.add(msg);
-
-    private static class NetworkRequestAndScore {
-        NetworkRequest req;
-        int score;
-
-        NetworkRequestAndScore(NetworkRequest networkRequest, int score) {
-            req = networkRequest;
-            this.score = score;
-        }
-    }
-
-    private void handleAddRequest(Message msg) {
-        NetworkRequestAndScore n = (NetworkRequestAndScore)msg.obj;
-        // replaces old request, updating score
-        mNetworkRequests.put(n.req.requestId, n);
-        mHasRequests.set(true);
-        evalScores();
-    }
-
-    private void handleRemoveRequest(Message msg) {
-        NetworkRequest networkRequest = (NetworkRequest)msg.obj;
-
-        if (mNetworkRequests.get(networkRequest.requestId) != null) {
-            mNetworkRequests.remove(networkRequest.requestId);
-            if (mNetworkRequests.size() == 0) mHasRequests.set(false);
-            evalScores();
-        }
-    }
-
-    /**
-     * Called to go through our list of requests and see if we're
-     * good enough to try connecting, or if we have gotten worse and
-     * need to disconnect.
-     *
-     * Once we are registered, does nothing: we disconnect when requested via
-     * CMD_CHANNEL_DISCONNECTED, generated by either a loss of connection
-     * between modules (bearer or ConnectivityService dies) or more commonly
-     * when the NetworkInfo reports to ConnectivityService it is disconnected.
-     */
-    private void evalScores() {
-        synchronized(mLockObj) {
-            if (mRegistered) {
-                if (VDBG) log("evalScores - already connected - size=" + mNetworkRequests.size());
-                // already trying
-                return;
-            }
-            if (VDBG) log("evalScores!");
-            for (int i=0; i < mNetworkRequests.size(); i++) {
-                int score = mNetworkRequests.valueAt(i).score;
-                if (VDBG) log(" checking request Min " + score + " vs my score " + mNetworkScore);
-                if (score < mNetworkScore) {
-                    // have a request that has a lower scored network servicing it
-                    // (or no network) than we could provide, so let's connect!
-                    mConnectionRequested = true;
-                    connect();
-                    return;
-                }
-            }
-            // Our score is not high enough to satisfy any current request.
-            // This can happen if our score goes down after a connection is
-            // requested but before we actually connect. In this case, disconnect
-            // rather than continue trying - there's no point connecting if we know
-            // we'll just be torn down as soon as we do.
-            if (mConnectionRequested) {
-                mConnectionRequested = false;
-                disconnect();
             }
         }
     }
 
-    public void addNetworkRequest(NetworkRequest networkRequest, int score) {
-        if (DBG) log("adding NetworkRequest " + networkRequest + " with score " + score);
-        sendMessage(obtainMessage(CMD_ADD_REQUEST,
-                new NetworkRequestAndScore(networkRequest, score)));
-    }
-
-    public void removeNetworkRequest(NetworkRequest networkRequest) {
-        if (DBG) log("removing NetworkRequest " + networkRequest);
-        sendMessage(obtainMessage(CMD_REMOVE_REQUEST, networkRequest));
-    }
-
     /**
      * Called by the bearer code when it has new LinkProperties data.
-     * If we're a registered NetworkAgent, this new data will get forwarded on,
-     * otherwise we store a copy in anticipation of registering.  This call
-     * may also prompt registration if it causes the NetworkAgent to meet
-     * the conditions (fully configured, connected, satisfys a request and
-     * has sufficient score).
      */
     public void sendLinkProperties(LinkProperties linkProperties) {
-        linkProperties = new LinkProperties(linkProperties);
+        queueOrSendMessage(EVENT_NETWORK_PROPERTIES_CHANGED, new LinkProperties(linkProperties));
-        synchronized(mLockObj) {
-            mLinkProperties = linkProperties;
-            if (mAsyncChannel != null) {
-                mAsyncChannel.sendMessage(EVENT_NETWORK_PROPERTIES_CHANGED, linkProperties);
-            } else {
-                registerSelf();
-            }
-        }
     }
 
     /**
      * Called by the bearer code when it has new NetworkInfo data.
-     * If we're a registered NetworkAgent, this new data will get forwarded on,
-     * otherwise we store a copy in anticipation of registering.  This call
-     * may also prompt registration if it causes the NetworkAgent to meet
-     * the conditions (fully configured, connected, satisfys a request and
-     * has sufficient score).
      */
     public void sendNetworkInfo(NetworkInfo networkInfo) {
-        networkInfo = new NetworkInfo(networkInfo);
+        queueOrSendMessage(EVENT_NETWORK_INFO_CHANGED, new NetworkInfo(networkInfo));
-        synchronized(mLockObj) {
-            mNetworkInfo = networkInfo;
-            if (mAsyncChannel != null) {
-                mAsyncChannel.sendMessage(EVENT_NETWORK_INFO_CHANGED, networkInfo);
-            } else {
-                registerSelf();
-            }
-        }
     }
 
     /**
      * Called by the bearer code when it has new NetworkCapabilities data.
-     * If we're a registered NetworkAgent, this new data will get forwarded on,
-     * otherwise we store a copy in anticipation of registering.  This call
-     * may also prompt registration if it causes the NetworkAgent to meet
-     * the conditions (fully configured, connected, satisfys a request and
-     * has sufficient score).
-     * Note that if these capabilities make the network non-useful,
-     * ConnectivityServce will tear this network down.
      */
     public void sendNetworkCapabilities(NetworkCapabilities networkCapabilities) {
-        networkCapabilities = new NetworkCapabilities(networkCapabilities);
+        queueOrSendMessage(EVENT_NETWORK_CAPABILITIES_CHANGED,
-        synchronized(mLockObj) {
+                new NetworkCapabilities(networkCapabilities));
-            mNetworkCapabilities = networkCapabilities;
-            if (mAsyncChannel != null) {
-                mAsyncChannel.sendMessage(EVENT_NETWORK_CAPABILITIES_CHANGED, networkCapabilities);
-            } else {
-                registerSelf();
-            }
-        }
-    }
-
-    public NetworkCapabilities getNetworkCapabilities() {
-        synchronized(mLockObj) {
-            return new NetworkCapabilities(mNetworkCapabilities);
-        }
     }
 
     /**
      * Called by the bearer code when it has a new score for this network.
-     * If we're a registered NetworkAgent, this new data will get forwarded on,
-     * otherwise we store a copy.
      */
-    public synchronized void sendNetworkScore(int score) {
+    public void sendNetworkScore(int score) {
-        synchronized(mLockObj) {
+        queueOrSendMessage(EVENT_NETWORK_SCORE_CHANGED, new Integer(score));
-            mNetworkScore = score;
-            evalScores();
-            if (mAsyncChannel != null) {
-                mAsyncChannel.sendMessage(EVENT_NETWORK_SCORE_CHANGED, mNetworkScore);
-            } else {
-                registerSelf();
-            }
-        }
     }
 
-    public boolean hasRequests() {
+    /**
-        return mHasRequests.get();
+     * Called when ConnectivityService has indicated they no longer want this network.
-    }
+     * The parent factory should (previously) have received indication of the change
-
+     * as well, either canceling NetworkRequests or altering their score such that this
-    public boolean isConnectionRequested() {
+     * network won't be immediately requested again.
-        synchronized(mLockObj) {
+     */
-            return mConnectionRequested;
+    abstract protected void unwanted();
-        }
-    }
-
-
-    abstract protected void connect();
-    abstract protected void disconnect();
 
     protected void log(String s) {
         Log.d(LOG_TAG, "NetworkAgent: " + s);

core/java/android/net/NetworkInfo.java

@@ -198,7 +198,10 @@ public class NetworkInfo implements Parcelable {
         }
     }
 
-    void setSubtype(int subtype, String subtypeName) {
+    /**
+     * @hide
+     */
+    public void setSubtype(int subtype, String subtypeName) {
         synchronized (this) {
             mSubtype = subtype;
             mSubtypeName = subtypeName;

services/core/java/com/android/server/ConnectivityService.java

@@ -38,7 +38,6 @@ import static android.net.ConnectivityManager.TYPE_WIMAX;
 import static android.net.ConnectivityManager.TYPE_PROXY;
 import static android.net.ConnectivityManager.getNetworkTypeName;
 import static android.net.ConnectivityManager.isNetworkTypeValid;
-import static android.net.ConnectivityServiceProtocol.NetworkFactoryProtocol;
 import static android.net.NetworkPolicyManager.RULE_ALLOW_ALL;
 import static android.net.NetworkPolicyManager.RULE_REJECT_METERED;
 
@@ -80,6 +79,7 @@ import android.net.NetworkCapabilities;
 import android.net.NetworkConfig;
 import android.net.NetworkInfo;
 import android.net.NetworkInfo.DetailedState;
+import android.net.NetworkFactory;
 import android.net.NetworkQuotaInfo;
 import android.net.NetworkRequest;
 import android.net.NetworkState;
@@ -2995,6 +2995,16 @@ public class ConnectivityService extends IConnectivityManager.Stub {
                     updateNetworkInfo(nai, info);
                     break;
                 }
+                case NetworkAgent.EVENT_NETWORK_SCORE_CHANGED: {
+                    NetworkAgentInfo nai = mNetworkAgentInfos.get(msg.replyTo);
+                    if (nai == null) {
+                        loge("EVENT_NETWORK_SCORE_CHANGED from unknown NetworkAgent");
+                        break;
+                    }
+                    Integer score = (Integer) msg.obj;
+                    updateNetworkScore(nai, score);
+                    break;
+                }
                 case NetworkMonitor.EVENT_NETWORK_VALIDATED: {
                     NetworkAgentInfo nai = (NetworkAgentInfo)msg.obj;
                     handleConnectionValidated(nai);
@@ -3099,7 +3109,7 @@ public class ConnectivityService extends IConnectivityManager.Stub {
                 for (NetworkRequestInfo nri : mNetworkRequests.values()) {
                     if (nri.isRequest == false) continue;
                     NetworkAgentInfo nai = mNetworkForRequestId.get(nri.request.requestId);
-                    ac.sendMessage(NetworkFactoryProtocol.CMD_REQUEST_NETWORK,
+                    ac.sendMessage(android.net.NetworkFactory.CMD_REQUEST_NETWORK,
                             (nai != null ? nai.currentScore : 0), 0, nri.request);
                 }
             } else {
@@ -3220,7 +3230,7 @@ public class ConnectivityService extends IConnectivityManager.Stub {
         if (msg.what == EVENT_REGISTER_NETWORK_REQUEST) {
             if (DBG) log("sending new NetworkRequest to factories");
             for (NetworkFactoryInfo nfi : mNetworkFactoryInfos.values()) {
-                nfi.asyncChannel.sendMessage(NetworkFactoryProtocol.CMD_REQUEST_NETWORK, score,
+                nfi.asyncChannel.sendMessage(android.net.NetworkFactory.CMD_REQUEST_NETWORK, score,
                         0, nri.request);
             }
         }
@@ -3243,7 +3253,8 @@ public class ConnectivityService extends IConnectivityManager.Stub {
 
             if (nri.isRequest) {
                 for (NetworkFactoryInfo nfi : mNetworkFactoryInfos.values()) {
-                    nfi.asyncChannel.sendMessage(NetworkFactoryProtocol.CMD_CANCEL_REQUEST, nri.request);
+                    nfi.asyncChannel.sendMessage(android.net.NetworkFactory.CMD_CANCEL_REQUEST,
+                            nri.request);
                 }
 
                 if (affectedNetwork != null) {
@@ -5565,7 +5576,8 @@ public class ConnectivityService extends IConnectivityManager.Stub {
     private void sendUpdatedScoreToFactories(NetworkRequest networkRequest, int score) {
         if (VDBG) log("sending new Min Network Score(" + score + "): " + networkRequest.toString());
         for (NetworkFactoryInfo nfi : mNetworkFactoryInfos.values()) {
-            nfi.asyncChannel.sendMessage(NetworkFactoryProtocol.CMD_REQUEST_NETWORK, score, 0, networkRequest);
+            nfi.asyncChannel.sendMessage(android.net.NetworkFactory.CMD_REQUEST_NETWORK, score, 0,
+                    networkRequest);
         }
     }
 
@@ -5802,6 +5814,11 @@ public class ConnectivityService extends IConnectivityManager.Stub {
         }
     }
 
+    private void updateNetworkScore(NetworkAgentInfo nai, Integer scoreInteger) {
+        int score = scoreInteger.intValue();
+        // TODO
+    }
+
     // notify only this one new request of the current state
     protected void notifyNetworkCallback(NetworkAgentInfo nai, NetworkRequestInfo nri) {
         int notifyType = ConnectivityManager.CALLBACK_AVAILABLE;