Merge changes from topic "remove_legacy_NA"

* changes:
  Remove support for legacy network agents
  Remove deprecated constructors for NetworkAgent
  Migrate NetworkAgentWrapper to the new NA API
  Cleanup TestNetworkService

core/java/android/net/NetworkAgent.java

@@ -51,7 +51,7 @@ import java.util.concurrent.atomic.AtomicBoolean;
  * An agent manages the life cycle of a network. A network starts its
  * life cycle when {@link register} is called on NetworkAgent. The network
  * is then connecting. When full L3 connectivity has been established,
- * the agent shoud call {@link markConnected} to inform the system that
+ * the agent should call {@link markConnected} to inform the system that
  * this network is ready to use. When the network disconnects its life
  * ends and the agent should call {@link unregister}, at which point the
  * system will clean up and free resources.
@@ -94,12 +94,6 @@ public abstract class NetworkAgent {
     @Nullable
     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
-    // exposing methods to manage it and generate it internally instead.
-    // TODO : remove this as soon as all agents have been converted.
-    private final boolean mIsLegacy;
-
     private final Handler mHandler;
     private volatile AsyncChannel mAsyncChannel;
     private final String LOG_TAG;
@@ -110,8 +104,6 @@ public abstract class NetworkAgent {
     private static final long BW_REFRESH_MIN_WIN_MS = 500;
     private boolean mBandwidthUpdateScheduled = false;
     private AtomicBoolean mBandwidthUpdatePending = new AtomicBoolean(false);
-    // Not used by legacy agents. Non-legacy agents use this to convert the NetworkAgent system API
-    // into the internal API of ConnectivityService.
     @NonNull
     private NetworkInfo mNetworkInfo;
     @NonNull
@@ -337,35 +329,6 @@ public abstract class NetworkAgent {
      */
     public static final int CMD_REMOVE_KEEPALIVE_PACKET_FILTER = BASE + 17;
 
-    /** @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) {
-        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. */
-    public NetworkAgent(Looper looper, Context context, String logTag, NetworkInfo ni,
-            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) {
         // The subtype can be changed with (TODO) setLegacySubtype, but it starts
         // with 0 (TelephonyManager.NETWORK_TYPE_UNKNOWN) and an empty description.
@@ -393,7 +356,7 @@ public abstract class NetworkAgent {
             @NonNull NetworkAgentConfig config, @Nullable NetworkProvider provider) {
         this(looper, context, logTag, nc, lp, score, config,
                 provider == null ? NetworkProvider.ID_NONE : provider.getProviderId(),
-                getLegacyNetworkInfo(config), false /* legacy */);
+                getLegacyNetworkInfo(config));
     }
 
     private static class InitialConfiguration {
@@ -418,11 +381,9 @@ public abstract class NetworkAgent {
 
     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,
+            @NonNull NetworkAgentConfig config, int providerId, @NonNull NetworkInfo ni) {
-            boolean legacy) {
         mHandler = new NetworkAgentHandler(looper);
         LOG_TAG = logTag;
-        mIsLegacy = legacy;
         mNetworkInfo = new NetworkInfo(ni);
         this.providerId = providerId;
         if (ni == null || nc == null || lp == null) {
@@ -696,11 +657,6 @@ public abstract class NetworkAgent {
      * Call {@link #unregister} to disconnect.
      */
     public void markConnected() {
-        if (mIsLegacy) {
-            throw new UnsupportedOperationException(
-                    "Legacy agents can't call markConnected.");
-        }
-        // |reason| cannot be used by the non-legacy agents
         mNetworkInfo.setDetailedState(NetworkInfo.DetailedState.CONNECTED, null /* reason */,
                 mNetworkInfo.getExtraInfo());
         queueOrSendMessage(EVENT_NETWORK_INFO_CHANGED, mNetworkInfo);
@@ -713,9 +669,6 @@ public abstract class NetworkAgent {
      * the network is torn down and this agent can no longer be used.
      */
     public void unregister() {
-        if (mIsLegacy) {
-            throw new UnsupportedOperationException("Legacy agents can't call unregister.");
-        }
         // When unregistering an agent nobody should use the extrainfo (or reason) any more.
         mNetworkInfo.setDetailedState(NetworkInfo.DetailedState.DISCONNECTED, null /* reason */,
                 null /* extraInfo */);
@@ -735,9 +688,6 @@ public abstract class NetworkAgent {
      */
     @Deprecated
     public void setLegacySubtype(final int legacySubtype, @NonNull final String legacySubtypeName) {
-        if (mIsLegacy) {
-            throw new UnsupportedOperationException("Legacy agents can't call setLegacySubtype.");
-        }
         mNetworkInfo.setSubtype(legacySubtype, legacySubtypeName);
         queueOrSendMessage(EVENT_NETWORK_INFO_CHANGED, mNetworkInfo);
     }
@@ -760,9 +710,6 @@ public abstract class NetworkAgent {
      */
     @Deprecated
     public void setLegacyExtraInfo(@Nullable final String extraInfo) {
-        if (mIsLegacy) {
-            throw new UnsupportedOperationException("Legacy agents can't call setLegacyExtraInfo.");
-        }
         mNetworkInfo.setExtraInfo(extraInfo);
         queueOrSendMessage(EVENT_NETWORK_INFO_CHANGED, mNetworkInfo);
     }
@@ -773,9 +720,6 @@ public abstract class NetworkAgent {
      */
     @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P, trackingBug = 115609023)
     public final void sendNetworkInfo(NetworkInfo networkInfo) {
-        if (!mIsLegacy) {
-            throw new UnsupportedOperationException("Only legacy agents can call sendNetworkInfo.");
-        }
         queueOrSendMessage(EVENT_NETWORK_INFO_CHANGED, new NetworkInfo(networkInfo));
     }
 

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

@@ -61,9 +61,8 @@ import java.util.concurrent.atomic.AtomicInteger;
 
 /** @hide */
 class TestNetworkService extends ITestNetworkManager.Stub {
-    @NonNull private static final String TAG = TestNetworkService.class.getSimpleName();
     @NonNull private static final String TEST_NETWORK_LOGTAG = "TestNetworkAgent";
-    @NonNull private static final String TEST_NETWORK_PROVIDER_NAME = TAG;
+    @NonNull private static final String TEST_NETWORK_PROVIDER_NAME = "TestNetworkProvider";
     @NonNull private static final AtomicInteger sTestTunIndex = new AtomicInteger();
 
     @NonNull private final Context mContext;
@@ -168,17 +167,15 @@ class TestNetworkService extends ITestNetworkManager.Stub {
         private TestNetworkAgent(
                 @NonNull Context context,
                 @NonNull Looper looper,
-                @NonNull NetworkAgentConfig config,
                 @NonNull NetworkCapabilities nc,
                 @NonNull LinkProperties lp,
+                @NonNull NetworkAgentConfig config,
                 int uid,
                 @NonNull IBinder binder,
                 @NonNull NetworkProvider np)
                 throws RemoteException {
             super(context, looper, TEST_NETWORK_LOGTAG, nc, lp, NETWORK_SCORE, config, np);
-
             mUid = uid;
-
             synchronized (mBinderLock) {
                 mBinder = binder; // Binder null-checks in create()
 
@@ -286,8 +283,8 @@ class TestNetworkService extends ITestNetworkManager.Stub {
             lp.addRoute(new RouteInfo(new IpPrefix(Inet6Address.ANY, 0), null, iface));
         }
 
-        final TestNetworkAgent agent = new TestNetworkAgent(context, looper,
+        final TestNetworkAgent agent = new TestNetworkAgent(context, looper, nc, lp,
-                new NetworkAgentConfig.Builder().build(), nc, lp, callingUid, binder,
+                new NetworkAgentConfig.Builder().build(), callingUid, binder,
                 mNetworkProvider);
         agent.register();
         agent.markConnected();

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

@@ -29,7 +29,7 @@ import static com.android.server.ConnectivityServiceTestUtils.transportToLegacyT
 import static junit.framework.Assert.assertTrue;
 
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.fail;
 
 import android.content.Context;
 import android.net.ConnectivityManager;
@@ -38,7 +38,6 @@ import android.net.Network;
 import android.net.NetworkAgent;
 import android.net.NetworkAgentConfig;
 import android.net.NetworkCapabilities;
-import android.net.NetworkInfo;
 import android.net.NetworkProvider;
 import android.net.NetworkSpecifier;
 import android.net.SocketKeepalive;
@@ -53,9 +52,9 @@ import com.android.testutils.HandlerUtils;
 import com.android.testutils.TestableNetworkCallback;
 
 import java.util.Set;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 public class NetworkAgentWrapper implements TestableNetworkCallback.HasNetwork {
-    private final NetworkInfo mNetworkInfo;
     private final NetworkCapabilities mNetworkCapabilities;
     private final HandlerThread mHandlerThread;
     private final Context mContext;
@@ -63,6 +62,7 @@ public class NetworkAgentWrapper implements TestableNetworkCallback.HasNetwork {
 
     private final ConditionVariable mDisconnected = new ConditionVariable();
     private final ConditionVariable mPreventReconnectReceived = new ConditionVariable();
+    private final AtomicBoolean mConnected = new AtomicBoolean(false);
     private int mScore;
     private NetworkAgent mNetworkAgent;
     private int mStartKeepaliveError = SocketKeepalive.ERROR_UNSUPPORTED;
@@ -76,7 +76,6 @@ public class NetworkAgentWrapper implements TestableNetworkCallback.HasNetwork {
             NetworkCapabilities ncTemplate, Context context) throws Exception {
         final int type = transportToLegacyType(transport);
         final String typeName = ConnectivityManager.getNetworkTypeName(type);
-        mNetworkInfo = new NetworkInfo(type, 0, typeName, "Mock");
         mNetworkCapabilities = (ncTemplate != null) ? ncTemplate : new NetworkCapabilities();
         mNetworkCapabilities.addCapability(NET_CAPABILITY_NOT_SUSPENDED);
         mNetworkCapabilities.addTransportType(transport);
@@ -108,22 +107,29 @@ public class NetworkAgentWrapper implements TestableNetworkCallback.HasNetwork {
         mHandlerThread = new HandlerThread(mLogTag);
         mHandlerThread.start();
 
-        mNetworkAgent = makeNetworkAgent(linkProperties);
+        mNetworkAgent = makeNetworkAgent(linkProperties, type, typeName);
     }
 
-    protected InstrumentedNetworkAgent makeNetworkAgent(LinkProperties linkProperties)
+    protected InstrumentedNetworkAgent makeNetworkAgent(LinkProperties linkProperties,
+            final int type, final String typeName)
             throws Exception {
-        return new InstrumentedNetworkAgent(this, linkProperties);
+        return new InstrumentedNetworkAgent(this, linkProperties, type, typeName);
     }
 
     public static class InstrumentedNetworkAgent extends NetworkAgent {
         private final NetworkAgentWrapper mWrapper;
+        private static final String PROVIDER_NAME = "InstrumentedNetworkAgentProvider";
 
-        public InstrumentedNetworkAgent(NetworkAgentWrapper wrapper, LinkProperties lp) {
+        public InstrumentedNetworkAgent(NetworkAgentWrapper wrapper, LinkProperties lp,
-            super(wrapper.mHandlerThread.getLooper(), wrapper.mContext, wrapper.mLogTag,
+                final int type, final String typeName) {
-                    wrapper.mNetworkInfo, wrapper.mNetworkCapabilities, lp, wrapper.mScore,
+            super(wrapper.mContext, wrapper.mHandlerThread.getLooper(), wrapper.mLogTag,
-                    new NetworkAgentConfig(), NetworkProvider.ID_NONE);
+                    wrapper.mNetworkCapabilities, lp, wrapper.mScore,
+                    new NetworkAgentConfig.Builder()
+                            .setLegacyType(type).setLegacyTypeName(typeName).build(),
+                    new NetworkProvider(wrapper.mContext, wrapper.mHandlerThread.getLooper(),
+                            PROVIDER_NAME));
             mWrapper = wrapper;
+            register();
         }
 
         @Override
@@ -212,10 +218,12 @@ public class NetworkAgentWrapper implements TestableNetworkCallback.HasNetwork {
     }
 
     public void connect() {
-        assertNotEquals("MockNetworkAgents can only be connected once",
+        if (!mConnected.compareAndSet(false /* expect */, true /* update */)) {
-                mNetworkInfo.getDetailedState(), NetworkInfo.DetailedState.CONNECTED);
+            // compareAndSet returns false when the value couldn't be updated because it did not
-        mNetworkInfo.setDetailedState(NetworkInfo.DetailedState.CONNECTED, null, null);
+            // match the expected value.
-        mNetworkAgent.sendNetworkInfo(mNetworkInfo);
+            fail("Test NetworkAgents can only be connected once");
+        }
+        mNetworkAgent.markConnected();
     }
 
     public void suspend() {
@@ -227,8 +235,7 @@ public class NetworkAgentWrapper implements TestableNetworkCallback.HasNetwork {
     }
 
     public void disconnect() {
-        mNetworkInfo.setDetailedState(NetworkInfo.DetailedState.DISCONNECTED, null, null);
+        mNetworkAgent.unregister();
-        mNetworkAgent.sendNetworkInfo(mNetworkInfo);
     }
 
     @Override

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

@@ -646,8 +646,8 @@ public class ConnectivityServiceTest {
         }
 
         @Override
-        protected InstrumentedNetworkAgent makeNetworkAgent(LinkProperties linkProperties)
+        protected InstrumentedNetworkAgent makeNetworkAgent(LinkProperties linkProperties,
-                throws Exception {
+                final int type, final String typeName) throws Exception {
             mNetworkMonitor = mock(INetworkMonitor.class);
 
             final Answer validateAnswer = inv -> {
@@ -666,7 +666,8 @@ public class ConnectivityServiceTest {
                     any() /* name */,
                     nmCbCaptor.capture());
 
-            final InstrumentedNetworkAgent na = new InstrumentedNetworkAgent(this, linkProperties) {
+            final InstrumentedNetworkAgent na = new InstrumentedNetworkAgent(this, linkProperties,
+                    type, typeName) {
                 @Override
                 public void networkStatus(int status, String redirectUrl) {
                     mRedirectUrl = redirectUrl;