Move NetworkAgent#register to a new method.

Calling IPC in a constructor is unusual and confusing, and can be
considered bad form. There are multiple reasons for this :
• Users can't obtain an instance of the class without calling the
  constructor, but they can't always afford an IPC where they need
  this, forcing them to know about the implementation detail and
  sometimes design around it.
• On a related but generalized note, constructors should usually
  be fast for the same range of reasons.
• Having a separate method to register the agent simply gives more
  flexibility to the app.
• It's also a lot easier to test.
But also we can't fix it without an update to the API, so here it is.

Another reason for doing this is consistency with the NetworkProvider
API.

Bug: 138306002
Bug: 139268426
Test: atest FrameworksNetTests FrameworksWifiTests FrameworksTelephonyTests
Change-Id: I1ee5c7b1353d581e487c8a8a159009bebd781643

core/java/android/net/NetworkAgent.java

@@ -51,8 +51,8 @@ public abstract class NetworkAgent {
     /**
      * The {@link Network} corresponding to this object.
      */
-    @NonNull
+    @Nullable
-    public final Network network;
+    private volatile Network mNetwork;
 
     // Whether this NetworkAgent is using the legacy (never unhidden) API. The difference is
     // that the legacy API uses NetworkInfo to convey the state, while the current API is
@@ -65,7 +65,6 @@ public abstract class NetworkAgent {
     private final String LOG_TAG;
     private static final boolean DBG = true;
     private static final boolean VDBG = false;
-    private final Context mContext;
     private final ArrayList<Message>mPreConnectedQueue = new ArrayList<Message>();
     private volatile long mLastBwRefreshTime = 0;
     private static final long BW_REFRESH_MIN_WIN_MS = 500;
@@ -277,18 +276,21 @@ public abstract class NetworkAgent {
     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, NetworkProvider.ID_NONE);
+        // Register done by the constructor called in the previous line
     }
 
     /** @hide TODO: remove and replace usage with the public constructor. */
     public NetworkAgent(Looper looper, Context context, String logTag, NetworkInfo ni,
             NetworkCapabilities nc, LinkProperties lp, int score, NetworkAgentConfig config) {
         this(looper, context, logTag, ni, nc, lp, score, config, NetworkProvider.ID_NONE);
+        // Register done by the constructor called in the previous line
     }
 
     /** @hide TODO: remove and replace usage with the public constructor. */
     public NetworkAgent(Looper looper, Context context, String logTag, NetworkInfo ni,
             NetworkCapabilities nc, LinkProperties lp, int score, int providerId) {
         this(looper, context, logTag, ni, nc, lp, score, null, providerId);
+        // Register done by the constructor called in the previous line
     }
 
     /** @hide TODO: remove and replace usage with the public constructor. */
@@ -296,6 +298,7 @@ public abstract class NetworkAgent {
             NetworkCapabilities nc, LinkProperties lp, int score, NetworkAgentConfig config,
             int providerId) {
         this(looper, context, logTag, nc, lp, score, config, providerId, ni, true /* legacy */);
+        register();
     }
 
     private static NetworkInfo getLegacyNetworkInfo(final NetworkAgentConfig config) {
@@ -324,12 +327,32 @@ public abstract class NetworkAgent {
                 getLegacyNetworkInfo(config), false /* legacy */);
     }
 
-    private NetworkAgent(Looper looper, Context context, String logTag, NetworkCapabilities nc,
+    private static class InitialConfiguration {
-            LinkProperties lp, int score, NetworkAgentConfig config, int providerId,
+        public final Context context;
-            NetworkInfo ni, boolean legacy) {
+        public final NetworkCapabilities capabilities;
+        public final LinkProperties properties;
+        public final int score;
+        public final NetworkAgentConfig config;
+        public final NetworkInfo info;
+        InitialConfiguration(@NonNull Context context, @NonNull NetworkCapabilities capabilities,
+                @NonNull LinkProperties properties, int score, @NonNull NetworkAgentConfig config,
+                @NonNull NetworkInfo info) {
+            this.context = context;
+            this.capabilities = capabilities;
+            this.properties = properties;
+            this.score = score;
+            this.config = config;
+            this.info = info;
+        }
+    }
+    private volatile InitialConfiguration mInitialConfiguration;
+
+    private NetworkAgent(@NonNull Looper looper, @NonNull Context context, @NonNull String logTag,
+            @NonNull NetworkCapabilities nc, @NonNull LinkProperties lp, int score,
+            @NonNull NetworkAgentConfig config, int providerId, @NonNull NetworkInfo ni,
+            boolean legacy) {
         mHandler = new NetworkAgentHandler(looper);
         LOG_TAG = logTag;
-        mContext = context;
         mIsLegacy = legacy;
         mNetworkInfo = new NetworkInfo(ni);
         this.providerId = providerId;
@@ -337,12 +360,8 @@ public abstract class NetworkAgent {
             throw new IllegalArgumentException();
         }
 
-        if (VDBG) log("Registering NetworkAgent");
+        mInitialConfiguration = new InitialConfiguration(context, new NetworkCapabilities(nc),
-        ConnectivityManager cm = (ConnectivityManager)mContext.getSystemService(
+                new LinkProperties(lp), score, config, ni);
-                Context.CONNECTIVITY_SERVICE);
-        network = cm.registerNetworkAgent(new Messenger(mHandler), new NetworkInfo(ni),
-                new LinkProperties(lp), new NetworkCapabilities(nc), score, config,
-                providerId);
     }
 
     private class NetworkAgentHandler extends Handler {
@@ -464,6 +483,32 @@ public abstract class NetworkAgent {
         }
     }
 
+    /**
+     * Register this network agent with ConnectivityService.
+     * @return the Network associated with this network agent (which can also be obtained later
+     *         by calling getNetwork() on this agent).
+     */
+    @NonNull
+    public Network register() {
+        if (VDBG) log("Registering NetworkAgent");
+        final ConnectivityManager cm = (ConnectivityManager) mInitialConfiguration.context
+                .getSystemService(Context.CONNECTIVITY_SERVICE);
+        mNetwork = cm.registerNetworkAgent(new Messenger(mHandler),
+                new NetworkInfo(mInitialConfiguration.info),
+                mInitialConfiguration.properties, mInitialConfiguration.capabilities,
+                mInitialConfiguration.score, mInitialConfiguration.config, providerId);
+        mInitialConfiguration = null; // All this memory can now be GC'd
+        return mNetwork;
+    }
+
+    /**
+     * @return The Network associated with this agent, or null if it's not registered yet.
+     */
+    @Nullable
+    public Network getNetwork() {
+        return mNetwork;
+    }
+
     private void queueOrSendMessage(int what, Object obj) {
         queueOrSendMessage(what, 0, 0, obj);
     }

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

@@ -218,7 +218,7 @@ class TestNetworkService extends ITestNetworkManager.Stub {
             // Has to be in TestNetworkAgent to ensure all teardown codepaths properly clean up
             // resources, even for binder death or unwanted calls.
             synchronized (mTestNetworkTracker) {
-                mTestNetworkTracker.remove(network.netId);
+                mTestNetworkTracker.remove(getNetwork().netId);
             }
         }
     }
@@ -337,7 +337,7 @@ class TestNetworkService extends ITestNetworkManager.Stub {
                                             callingUid,
                                             binder);
 
-                            mTestNetworkTracker.put(agent.network.netId, agent);
+                            mTestNetworkTracker.put(agent.getNetwork().netId, agent);
                         }
                     } catch (SocketException e) {
                         throw new UncheckedIOException(e);

tests/net/integration/util/com/android/server/NetworkAgentWrapper.java

@@ -222,7 +222,7 @@ public class NetworkAgentWrapper implements TestableNetworkCallback.HasNetwork {
 
     @Override
     public Network getNetwork() {
-        return mNetworkAgent.network;
+        return mNetworkAgent.getNetwork();
     }
 
     public void expectPreventReconnectReceived(long timeoutMs) {

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

@@ -575,7 +575,7 @@ public class ConnectivityServiceTest {
                 }
             };
 
-            assertEquals(na.network.netId, nmNetworkCaptor.getValue().netId);
+            assertEquals(na.getNetwork().netId, nmNetworkCaptor.getValue().netId);
             mNmCallbacks = nmCbCaptor.getValue();
 
             mNmCallbacks.onNetworkMonitorCreated(mNetworkMonitor);