From d43d92fcab505dce69f2be5d23fbcc416c706f41 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Fri, 9 Oct 2020 16:52:53 +0900 Subject: [PATCH 1/4] Cleanup TestNetworkService Very small cleanup where arguments to TestNetworkAgent should have the same order as the callee. Also remove an unused member. Test: FrameworksNetTests NetworkStackTests Change-Id: I9da16bc81be8524e227a7f7e83760882bc4d77e5 --- .../java/com/android/server/TestNetworkService.java | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/services/core/java/com/android/server/TestNetworkService.java b/services/core/java/com/android/server/TestNetworkService.java index a45466d985..4703117fb8 100644 --- a/services/core/java/com/android/server/TestNetworkService.java +++ b/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, - new NetworkAgentConfig.Builder().build(), nc, lp, callingUid, binder, + final TestNetworkAgent agent = new TestNetworkAgent(context, looper, nc, lp, + new NetworkAgentConfig.Builder().build(), callingUid, binder, mNetworkProvider); agent.register(); agent.markConnected(); From b274feac5ca3f2073a1a910bde89fe67e0a1b53f Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Fri, 9 Oct 2020 18:05:06 +0900 Subject: [PATCH 2/4] Migrate NetworkAgentWrapper to the new NA API Test: FrameworksNetTests NetworkStackTests Bug: 167544279 Change-Id: I5d53a938572682dea827ea681596226b1e271aa6 --- .../android/server/NetworkAgentWrapper.java | 41 +++++++++++-------- .../server/ConnectivityServiceTest.java | 7 ++-- 2 files changed, 28 insertions(+), 20 deletions(-) diff --git a/tests/net/integration/util/com/android/server/NetworkAgentWrapper.java b/tests/net/integration/util/com/android/server/NetworkAgentWrapper.java index 2a24d1ac22..3d4dc4d67d 100644 --- a/tests/net/integration/util/com/android/server/NetworkAgentWrapper.java +++ b/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) { - super(wrapper.mHandlerThread.getLooper(), wrapper.mContext, wrapper.mLogTag, - wrapper.mNetworkInfo, wrapper.mNetworkCapabilities, lp, wrapper.mScore, - new NetworkAgentConfig(), NetworkProvider.ID_NONE); + public InstrumentedNetworkAgent(NetworkAgentWrapper wrapper, LinkProperties lp, + final int type, final String typeName) { + super(wrapper.mContext, wrapper.mHandlerThread.getLooper(), wrapper.mLogTag, + 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", - mNetworkInfo.getDetailedState(), NetworkInfo.DetailedState.CONNECTED); - mNetworkInfo.setDetailedState(NetworkInfo.DetailedState.CONNECTED, null, null); - mNetworkAgent.sendNetworkInfo(mNetworkInfo); + if (!mConnected.compareAndSet(false /* expect */, true /* update */)) { + // compareAndSet returns false when the value couldn't be updated because it did not + // match the expected value. + 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.sendNetworkInfo(mNetworkInfo); + mNetworkAgent.unregister(); } @Override diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 7a1cb25b2d..9aeafcd7a9 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -646,8 +646,8 @@ public class ConnectivityServiceTest { } @Override - protected InstrumentedNetworkAgent makeNetworkAgent(LinkProperties linkProperties) - throws Exception { + protected InstrumentedNetworkAgent makeNetworkAgent(LinkProperties linkProperties, + 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; From 855c9c8f88317af15b14aea04bb2460f2a695fde Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Fri, 9 Oct 2020 18:20:27 +0900 Subject: [PATCH 3/4] Remove deprecated constructors for NetworkAgent Finally. Now that mLegacy is always false, removing the support for legacy agents, a follow-up change will remove the member and all the associated code. Test: FrameworksNetTests NetworkStackTests Bug: 167544279 Change-Id: I6e2c27facdd3ecc232a0aa32bf57c33cb06f118e --- core/java/android/net/NetworkAgent.java | 29 ------------------------- 1 file changed, 29 deletions(-) diff --git a/core/java/android/net/NetworkAgent.java b/core/java/android/net/NetworkAgent.java index 0676ad4e23..8354126359 100644 --- a/core/java/android/net/NetworkAgent.java +++ b/core/java/android/net/NetworkAgent.java @@ -337,35 +337,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. From 74c32e10abb1658166cec9ce1e1f86aa12c24a65 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Fri, 9 Oct 2020 18:50:08 +0900 Subject: [PATCH 4/4] Remove support for legacy network agents Test: FrameworksNetTests NetworkStackTests Bug: 167544279 Change-Id: Ia950e16d991cd08d4b609d71aad61a4a4f7fda39 --- core/java/android/net/NetworkAgent.java | 33 +++---------------------- 1 file changed, 3 insertions(+), 30 deletions(-) diff --git a/core/java/android/net/NetworkAgent.java b/core/java/android/net/NetworkAgent.java index 8354126359..6780167fa6 100644 --- a/core/java/android/net/NetworkAgent.java +++ b/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 @@ -364,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 { @@ -389,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, - boolean legacy) { + @NonNull NetworkAgentConfig config, int providerId, @NonNull NetworkInfo ni) { mHandler = new NetworkAgentHandler(looper); LOG_TAG = logTag; - mIsLegacy = legacy; mNetworkInfo = new NetworkInfo(ni); this.providerId = providerId; if (ni == null || nc == null || lp == null) { @@ -667,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); @@ -684,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 */); @@ -706,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); } @@ -731,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); } @@ -744,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)); }