diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index b9b590b9a0..bfb51dadad 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -113,7 +113,6 @@ import static android.net.NetworkCapabilities.RES_ID_MATCH_ALL_RESERVATIONS; import static android.net.NetworkCapabilities.RES_ID_UNSET; import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR; import static android.net.NetworkCapabilities.TRANSPORT_TEST; -import static android.net.NetworkCapabilities.TRANSPORT_THREAD; import static android.net.NetworkCapabilities.TRANSPORT_VPN; import static android.net.NetworkCapabilities.TRANSPORT_WIFI; import static android.net.NetworkRequest.Type.LISTEN_FOR_BEST; @@ -430,7 +429,6 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import java.util.function.BiConsumer; import java.util.function.Consumer; -import java.util.function.Predicate; /** * @hide @@ -4895,11 +4893,7 @@ public class ConnectivityService extends IConnectivityManager.Stub { // the destroyed flag is only just above the "current satisfier wins" // tie-breaker. But technically anything that affects scoring should rematch. rematchAllNetworksAndRequests(); - if (mQueueNetworkAgentEventsInSystemServer) { - mHandler.postDelayed(() -> disconnectAndDestroyNetwork(nai), timeoutMs); - } else { - mHandler.postDelayed(() -> nai.disconnect(), timeoutMs); - } + mHandler.postDelayed(() -> nai.disconnect(), timeoutMs); break; } } @@ -5329,12 +5323,12 @@ public class ConnectivityService extends IConnectivityManager.Stub { private void handlePrivateDnsSettingsChanged() { final PrivateDnsConfig cfg = mDnsManager.getPrivateDnsConfig(); - forEachNetworkAgentInfo(nai -> { + for (NetworkAgentInfo nai : mNetworkAgentInfos) { handlePerNetworkPrivateDnsConfig(nai, cfg); if (networkRequiresPrivateDnsValidation(nai)) { handleUpdateLinkProperties(nai, new LinkProperties(nai.linkProperties)); } - }); + } } private void handlePerNetworkPrivateDnsConfig(NetworkAgentInfo nai, PrivateDnsConfig cfg) { @@ -5449,18 +5443,15 @@ public class ConnectivityService extends IConnectivityManager.Stub { } @VisibleForTesting - protected boolean shouldCreateNetworksImmediately(@NonNull NetworkCapabilities caps) { + protected static boolean shouldCreateNetworksImmediately(@NonNull NetworkCapabilities caps) { // The feature of creating the networks immediately was slated for U, but race conditions // detected late required this was flagged off. - // TODO : remove when it's determined that the code is stable - return mQueueNetworkAgentEventsInSystemServer - // Local network agents for Thread used to not create networks immediately, - // but other local agents (tethering, P2P) require this to function. - || (caps.hasCapability(NET_CAPABILITY_LOCAL_NETWORK) - && !caps.hasTransport(TRANSPORT_THREAD)); + // TODO : enable this in a Mainline update or in V, and re-enable the test for this + // in NetworkAgentTest. + return caps.hasCapability(NET_CAPABILITY_LOCAL_NETWORK); } - private boolean shouldCreateNativeNetwork(@NonNull NetworkAgentInfo nai, + private static boolean shouldCreateNativeNetwork(@NonNull NetworkAgentInfo nai, @NonNull NetworkInfo.State state) { if (nai.isCreated()) return false; if (state == NetworkInfo.State.CONNECTED) return true; @@ -5517,11 +5508,6 @@ public class ConnectivityService extends IConnectivityManager.Stub { if (DBG) { log(nai.toShortString() + " disconnected, was satisfying " + nai.numNetworkRequests()); } - - if (mQueueNetworkAgentEventsInSystemServer) { - nai.disconnect(); - } - // Clear all notifications of this network. mNotifier.clearNotification(nai.network.getNetId()); // A network agent has disconnected. @@ -5665,16 +5651,16 @@ public class ConnectivityService extends IConnectivityManager.Stub { private void maybeDisableForwardRulesForDisconnectingNai( @NonNull final NetworkAgentInfo disconnecting, final boolean sendCallbacks) { // Step 1 : maybe this network was the upstream for one or more local networks. - forEachNetworkAgentInfo(local -> { - if (!local.isLocalNetwork()) return; // return@forEach + for (final NetworkAgentInfo local : mNetworkAgentInfos) { + if (!local.isLocalNetwork()) continue; final NetworkRequest selector = local.localNetworkConfig.getUpstreamSelector(); - if (null == selector) return; // return@forEach + if (null == selector) continue; final NetworkRequestInfo nri = mNetworkRequests.get(selector); // null == nri can happen while disconnecting a network, because destroyNetwork() is // called after removing all associated NRIs from mNetworkRequests. - if (null == nri) return; // return@forEach + if (null == nri) continue; final NetworkAgentInfo satisfier = nri.getSatisfier(); - if (disconnecting != satisfier) return; // return@forEach + if (disconnecting != satisfier) continue; removeLocalNetworkUpstream(local, disconnecting); // Set the satisfier to null immediately so that the LOCAL_NETWORK_CHANGED callback // correctly contains null as an upstream. @@ -5682,7 +5668,7 @@ public class ConnectivityService extends IConnectivityManager.Stub { nri.setSatisfier(null, null); notifyNetworkCallbacks(local, CALLBACK_LOCAL_NETWORK_INFO_CHANGED); } - }); + } // Step 2 : maybe this is a local network that had an upstream. if (!disconnecting.isLocalNetwork()) return; @@ -5855,12 +5841,12 @@ public class ConnectivityService extends IConnectivityManager.Stub { mNetworkRequests.put(req, nri); // TODO: Consider update signal strength for other types. if (req.isListen()) { - forEachNetworkAgentInfo(network -> { + for (final NetworkAgentInfo network : mNetworkAgentInfos) { if (req.networkCapabilities.hasSignalStrength() && network.satisfiesImmutableCapabilitiesOf(req)) { updateSignalStrengthThresholds(network, "REGISTER", req); } - }); + } } else if (req.isRequest() && mNetworkRequestStateStatsMetrics != null) { mNetworkRequestStateStatsMetrics.onNetworkRequestReceived(req); } @@ -6155,13 +6141,13 @@ public class ConnectivityService extends IConnectivityManager.Stub { private void removeListenRequestFromNetworks(@NonNull final NetworkRequest req) { // listens don't have a singular affected Network. Check all networks to see // if this listen request applies and remove it. - forEachNetworkAgentInfo(nai -> { + for (final NetworkAgentInfo nai : mNetworkAgentInfos) { nai.removeRequest(req.requestId); if (req.networkCapabilities.hasSignalStrength() && nai.satisfiesImmutableCapabilitiesOf(req)) { updateSignalStrengthThresholds(nai, "RELEASE", req); } - }); + } } /** @@ -6224,43 +6210,6 @@ public class ConnectivityService extends IConnectivityManager.Stub { } } - /** - * Perform the specified operation on all networks. - * - * This method will run |op| exactly once for each network currently registered at the - * time it is called, even if |op| adds or removes networks. - * - * @param op the operation to perform. The operation is allowed to disconnect any number of - * networks. - */ - private void forEachNetworkAgentInfo(final Consumer op) { - // Create a copy instead of iterating over the set so |op| is allowed to disconnect any - // number of networks (which removes it from mNetworkAgentInfos). The copy is cheap - // because there are at most a handful of NetworkAgents connected at any given time. - final NetworkAgentInfo[] nais = new NetworkAgentInfo[mNetworkAgentInfos.size()]; - mNetworkAgentInfos.toArray(nais); - for (NetworkAgentInfo nai : nais) { - op.accept(nai); - } - } - - /** - * Check whether the specified condition is true for any network. - * - * This method will stop evaluating as soon as the condition returns true for any network. - * The order of iteration is not contractual. - * - * @param condition the condition to verify. This method must not modify the set of networks in - * any way. - * @return whether {@code condition} returned true for any network - */ - private boolean anyNetworkAgentInfo(final Predicate condition) { - for (int i = mNetworkAgentInfos.size() - 1; i >= 0; i--) { - if (condition.test(mNetworkAgentInfos.valueAt(i))) return true; - } - return false; - } - private RequestInfoPerUidCounter getRequestCounter(NetworkRequestInfo nri) { return hasAnyPermissionOf(mContext, nri.mPid, nri.mUid, NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK) @@ -6602,14 +6551,14 @@ public class ConnectivityService extends IConnectivityManager.Stub { ensureRunningOnConnectivityServiceThread(); // Agent info scores and offer scores depend on whether cells yields to bad wifi. final boolean avoidBadWifi = avoidBadWifi(); - forEachNetworkAgentInfo(nai -> { + for (final NetworkAgentInfo nai : mNetworkAgentInfos) { nai.updateScoreForNetworkAgentUpdate(); if (avoidBadWifi) { // If the device is now avoiding bad wifi, remove notifications that might have // been put up when the device didn't. mNotifier.clearNotification(nai.network.getNetId(), NotificationType.LOST_INTERNET); } - }); + } // UpdateOfferScore will update mNetworkOffers inline, so make a copy first. final ArrayList offersToUpdate = new ArrayList<>(mNetworkOffers); for (final NetworkOfferInfo noi : offersToUpdate) { @@ -6947,15 +6896,19 @@ public class ConnectivityService extends IConnectivityManager.Stub { final Network underpinnedNetwork = ki.getUnderpinnedNetwork(); final Network network = ki.getNetwork(); - final boolean networkFound = - anyNetworkAgentInfo(n -> n.network.equals(network)); + boolean networkFound = false; + boolean underpinnedNetworkFound = false; + for (NetworkAgentInfo n : mNetworkAgentInfos) { + if (n.network.equals(network)) networkFound = true; + if (n.everConnected() && n.network.equals(underpinnedNetwork)) { + underpinnedNetworkFound = true; + } + } // If the network no longer exists, then the keepalive should have been // cleaned up already. There is no point trying to resume keepalives. if (!networkFound) return; - final boolean underpinnedNetworkFound = anyNetworkAgentInfo( - n -> n.everConnected() && n.network.equals(underpinnedNetwork)); if (underpinnedNetworkFound) { mKeepaliveTracker.handleMonitorAutomaticKeepalive(ki, underpinnedNetwork.netId); @@ -7025,11 +6978,7 @@ public class ConnectivityService extends IConnectivityManager.Stub { final NetworkAgentInfo nai = getNetworkAgentInfoForNetwork((Network) msg.obj); if (nai == null) break; nai.onPreventAutomaticReconnect(); - if (mQueueNetworkAgentEventsInSystemServer) { - disconnectAndDestroyNetwork(nai); - } else { - nai.disconnect(); - } + nai.disconnect(); break; case EVENT_SET_VPN_NETWORK_PREFERENCE: handleSetVpnNetworkPreference((VpnNetworkPreferenceInfo) msg.obj); @@ -7419,12 +7368,12 @@ public class ConnectivityService extends IConnectivityManager.Stub { return new UnderlyingNetworkInfo[0]; } List infoList = new ArrayList<>(); - forEachNetworkAgentInfo(nai -> { + for (NetworkAgentInfo nai : mNetworkAgentInfos) { UnderlyingNetworkInfo info = createVpnInfo(nai); if (info != null) { infoList.add(info); } - }); + } return infoList.toArray(new UnderlyingNetworkInfo[infoList.size()]); } @@ -7502,11 +7451,11 @@ public class ConnectivityService extends IConnectivityManager.Stub { */ private void propagateUnderlyingNetworkCapabilities(Network updatedNetwork) { ensureRunningOnConnectivityServiceThread(); - forEachNetworkAgentInfo(nai -> { + for (NetworkAgentInfo nai : mNetworkAgentInfos) { if (updatedNetwork == null || hasUnderlyingNetwork(nai, updatedNetwork)) { updateCapabilitiesForNetwork(nai); } - }); + } } private boolean isUidBlockedByVpn(int uid, List blockedUidRanges) { @@ -7554,11 +7503,11 @@ public class ConnectivityService extends IConnectivityManager.Stub { mPermissionMonitor.updateVpnLockdownUidRanges(requireVpn, ranges); } - forEachNetworkAgentInfo(nai -> { + for (final NetworkAgentInfo nai : mNetworkAgentInfos) { final boolean curMetered = nai.networkCapabilities.isMetered(); maybeNotifyNetworkBlocked(nai, curMetered, curMetered, mVpnBlockedUidRanges, newVpnBlockedUidRanges); - }); + } mVpnBlockedUidRanges = newVpnBlockedUidRanges; } @@ -9122,9 +9071,6 @@ public class ConnectivityService extends IConnectivityManager.Stub { // Tracks all NetworkAgents that are currently registered. // NOTE: Only should be accessed on ConnectivityServiceThread, except dump(). - // Code iterating over this set is recommended to use forAllNetworkAgentInfos(), which allows - // code within the loop to disconnect networks during iteration without causing null pointer or - // OOB exceptions. private final ArraySet mNetworkAgentInfos = new ArraySet<>(); // UID ranges for users that are currently blocked by VPNs. @@ -10493,7 +10439,7 @@ public class ConnectivityService extends IConnectivityManager.Stub { // A NetworkAgent's allowedUids may need to be updated if the app has lost // carrier config - forEachNetworkAgentInfo(nai -> { + for (final NetworkAgentInfo nai : mNetworkAgentInfos) { if (nai.networkCapabilities.getAllowedUidsNoCopy().contains(uid) && getSubscriptionIdFromNetworkCaps(nai.networkCapabilities) == subId) { final NetworkCapabilities nc = new NetworkCapabilities(nai.networkCapabilities); @@ -10505,7 +10451,7 @@ public class ConnectivityService extends IConnectivityManager.Stub { mCarrierPrivilegeAuthenticator); updateCapabilities(nai.getScore(), nai, nc); } - }); + } } /** @@ -11262,11 +11208,7 @@ public class ConnectivityService extends IConnectivityManager.Stub { break; } } - if (mQueueNetworkAgentEventsInSystemServer) { - disconnectAndDestroyNetwork(nai); - } else { - nai.disconnect(); - } + nai.disconnect(); } private void handleLingerComplete(NetworkAgentInfo oldNetwork) { @@ -11426,7 +11368,7 @@ public class ConnectivityService extends IConnectivityManager.Stub { throw new IllegalStateException("No user is available"); } - forEachNetworkAgentInfo(nai -> { + for (final NetworkAgentInfo nai : mNetworkAgentInfos) { ArraySet allowedUidRanges = new ArraySet<>(); for (final UserHandle user : users) { final ArraySet restrictedUidRanges = @@ -11438,7 +11380,7 @@ public class ConnectivityService extends IConnectivityManager.Stub { final UidRangeParcel[] rangesParcel = toUidRangeStableParcels(allowedUidRanges); configs.add(new NativeUidRangeConfig( nai.network.netId, rangesParcel, 0 /* subPriority */)); - }); + } // The netd API replaces the previous configs with the current configs. // Thus, for network disconnection or preference removal, no need to @@ -11660,7 +11602,9 @@ public class ConnectivityService extends IConnectivityManager.Stub { // Gather the list of all relevant agents. final ArrayList nais = new ArrayList<>(); - forEachNetworkAgentInfo(nai -> nais.add(nai)); + for (final NetworkAgentInfo nai : mNetworkAgentInfos) { + nais.add(nai); + } for (final NetworkRequestInfo nri : networkRequests) { // Non-multilayer listen requests can be ignored. @@ -11766,14 +11710,14 @@ public class ConnectivityService extends IConnectivityManager.Stub { // Don't send CALLBACK_LOCAL_NETWORK_INFO_CHANGED yet though : they should be sent after // onAvailable so clients know what network the change is about. Store such changes in // an array that's only allocated if necessary (because it's almost never necessary). - final ArrayList localInfoChangedAgents = new ArrayList<>(); - forEachNetworkAgentInfo(nai -> { - if (!nai.isLocalNetwork()) return; // return@forEach + ArrayList localInfoChangedAgents = null; + for (final NetworkAgentInfo nai : mNetworkAgentInfos) { + if (!nai.isLocalNetwork()) continue; final NetworkRequest nr = nai.localNetworkConfig.getUpstreamSelector(); - if (null == nr) return; // return@forEach, no upstream for this local network + if (null == nr) continue; // No upstream for this local network final NetworkRequestInfo nri = mNetworkRequests.get(nr); final NetworkReassignment.RequestReassignment change = changes.getReassignment(nri); - if (null == change) return; // return@forEach, no change in upstreams for this network + if (null == change) continue; // No change in upstreams for this network final String fromIface = nai.linkProperties.getInterfaceName(); if (!hasSameInterfaceName(change.mOldNetwork, change.mNewNetwork) || change.mOldNetwork.isDestroyed()) { @@ -11801,8 +11745,9 @@ public class ConnectivityService extends IConnectivityManager.Stub { loge("Can't update forwarding rules", e); } } + if (null == localInfoChangedAgents) localInfoChangedAgents = new ArrayList<>(); localInfoChangedAgents.add(nai); - }); + } // Notify requested networks are available after the default net is switched, but // before LegacyTypeTracker sends legacy broadcasts @@ -11853,14 +11798,17 @@ public class ConnectivityService extends IConnectivityManager.Stub { } // Send LOCAL_NETWORK_INFO_CHANGED callbacks now that onAvailable and onLost have been sent. - for (final NetworkAgentInfo nai : localInfoChangedAgents) { - notifyNetworkCallbacks(nai, CALLBACK_LOCAL_NETWORK_INFO_CHANGED); + if (null != localInfoChangedAgents) { + for (final NetworkAgentInfo nai : localInfoChangedAgents) { + notifyNetworkCallbacks(nai, + CALLBACK_LOCAL_NETWORK_INFO_CHANGED); + } } updateLegacyTypeTrackerAndVpnLockdownForRematch(changes, nais); // Tear down all unneeded networks. - forEachNetworkAgentInfo(nai -> { + for (NetworkAgentInfo nai : mNetworkAgentInfos) { if (unneeded(nai, UnneededFor.TEARDOWN)) { if (nai.getInactivityExpiry() > 0) { // This network has active linger timers and no requests, but is not @@ -11878,7 +11826,7 @@ public class ConnectivityService extends IConnectivityManager.Stub { teardownUnneededNetwork(nai); } } - }); + } } /** @@ -12267,9 +12215,7 @@ public class ConnectivityService extends IConnectivityManager.Stub { // This has to happen after matching the requests, because callbacks are just requests. notifyNetworkCallbacks(networkAgent, CALLBACK_PRECHECK); } else if (state == NetworkInfo.State.DISCONNECTED) { - if (!mQueueNetworkAgentEventsInSystemServer) { - networkAgent.disconnect(); - } + networkAgent.disconnect(); if (networkAgent.isVPN()) { updateVpnUids(networkAgent, networkAgent.networkCapabilities, null); } @@ -12393,7 +12339,7 @@ public class ConnectivityService extends IConnectivityManager.Stub { * @param blockedReasons The reasons for why an uid is blocked. */ private void maybeNotifyNetworkBlockedForNewState(int uid, @BlockedReason int blockedReasons) { - forEachNetworkAgentInfo(nai -> { + for (final NetworkAgentInfo nai : mNetworkAgentInfos) { final boolean metered = nai.networkCapabilities.isMetered(); final boolean vpnBlocked = isUidBlockedByVpn(uid, mVpnBlockedUidRanges); @@ -12401,7 +12347,9 @@ public class ConnectivityService extends IConnectivityManager.Stub { uid, mUidBlockedReasons.get(uid, BLOCKED_REASON_NONE), metered, vpnBlocked); final int newBlockedState = getBlockedState(uid, blockedReasons, metered, vpnBlocked); - if (oldBlockedState == newBlockedState) return; // return@forEach + if (oldBlockedState == newBlockedState) { + continue; + } for (int i = 0; i < nai.numNetworkRequests(); i++) { NetworkRequest nr = nai.requestAt(i); NetworkRequestInfo nri = mNetworkRequests.get(nr); @@ -12410,7 +12358,7 @@ public class ConnectivityService extends IConnectivityManager.Stub { newBlockedState); } } - }); + } } @VisibleForTesting @@ -12499,11 +12447,11 @@ public class ConnectivityService extends IConnectivityManager.Stub { activeNetIds.add(nri.getSatisfier().network().netId); } } - forEachNetworkAgentInfo(nai -> { + for (NetworkAgentInfo nai : mNetworkAgentInfos) { if (activeNetIds.contains(nai.network().netId) || nai.isVPN()) { defaultNetworks.add(nai.network); } - }); + } return defaultNetworks; } @@ -13394,10 +13342,15 @@ public class ConnectivityService extends IConnectivityManager.Stub { } private boolean ownsVpnRunningOverNetwork(int uid, Network network) { - return anyNetworkAgentInfo(virtual -> - virtual.propagateUnderlyingCapabilities() - && virtual.networkCapabilities.getOwnerUid() == uid - && CollectionUtils.contains(virtual.declaredUnderlyingNetworks, network)); + for (NetworkAgentInfo virtual : mNetworkAgentInfos) { + if (virtual.propagateUnderlyingCapabilities() + && virtual.networkCapabilities.getOwnerUid() == uid + && CollectionUtils.contains(virtual.declaredUnderlyingNetworks, network)) { + return true; + } + } + + return false; } @CheckResult @@ -13568,16 +13521,18 @@ public class ConnectivityService extends IConnectivityManager.Stub { @Override public void onInterfaceLinkStateChanged(@NonNull String iface, boolean up) { mHandler.post(() -> { - forEachNetworkAgentInfo(nai -> - nai.clatd.handleInterfaceLinkStateChanged(iface, up)); + for (NetworkAgentInfo nai : mNetworkAgentInfos) { + nai.clatd.handleInterfaceLinkStateChanged(iface, up); + } }); } @Override public void onInterfaceRemoved(@NonNull String iface) { mHandler.post(() -> { - forEachNetworkAgentInfo(nai -> - nai.clatd.handleInterfaceRemoved(iface)); + for (NetworkAgentInfo nai : mNetworkAgentInfos) { + nai.clatd.handleInterfaceRemoved(iface); + } }); } } @@ -14358,7 +14313,7 @@ public class ConnectivityService extends IConnectivityManager.Stub { final long oldIngressRateLimit = mIngressRateLimit; mIngressRateLimit = ConnectivitySettingsManager.getIngressRateLimitInBytesPerSecond( mContext); - forEachNetworkAgentInfo(networkAgent -> { + for (final NetworkAgentInfo networkAgent : mNetworkAgentInfos) { if (canNetworkBeRateLimited(networkAgent)) { // If rate limit has previously been enabled, remove the old limit first. if (oldIngressRateLimit >= 0) { @@ -14369,7 +14324,7 @@ public class ConnectivityService extends IConnectivityManager.Stub { mIngressRateLimit); } } - }); + } } private boolean canNetworkBeRateLimited(@NonNull final NetworkAgentInfo networkAgent) { diff --git a/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt b/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt index 4c3bce0baf..bd9bd2a5f9 100644 --- a/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt +++ b/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt @@ -179,10 +179,6 @@ private const val TAG = "NetworkAgentTest" // without affecting the run time of successful runs. Thus, set a very high timeout. private const val DEFAULT_TIMEOUT_MS = 5000L -private const val QUEUE_NETWORK_AGENT_EVENTS_IN_SYSTEM_SERVER = - "queue_network_agent_events_in_system_server" - - // When waiting for a NetworkCallback to determine there was no timeout, waiting is the // only possible thing (the relevant handler is the one in the real ConnectivityService, // and then there is the Binder call), so have a short timeout for this as it will be @@ -207,6 +203,12 @@ private val REMOTE_ADDRESS = InetAddresses.parseNumericAddress("2001:db8::123") private val PREFIX = IpPrefix("2001:db8::/64") private val NEXTHOP = InetAddresses.parseNumericAddress("fe80::abcd") +// On T and below, the native network is only created when the agent connects. +// Starting in U, the native network was to be created as soon as the agent is registered, +// but this has been flagged off for now pending resolution of race conditions. +// TODO : enable this in a Mainline update or in V. +private const val SHOULD_CREATE_NETWORKS_IMMEDIATELY = false + @AppModeFull(reason = "Instant apps can't use NetworkAgent because it needs NETWORK_FACTORY'.") // NetworkAgent is updated as part of the connectivity module, and running NetworkAgent tests in MTS // for modules other than Connectivity does not provide much value. Only run them in connectivity @@ -232,18 +234,6 @@ class NetworkAgentTest { private var qosTestSocket: Closeable? = null // either Socket or DatagramSocket private val ifacesToCleanUp = mutableListOf() - // Unless the queuing in system server feature is chickened out, native networks are created - // immediately. Historically they would only created as they'd connect, which would force - // the code to apply link properties multiple times and suffer errors early on. Creating - // them early required that ordering between the client and the system server is guaranteed - // (at least to some extent), which has been done by moving the event queue from the client - // to the system server. When that feature is not chickened out, create networks immediately. - private val SHOULD_CREATE_NETWORKS_IMMEDIATELY - get() = mCM.isConnectivityServiceFeatureEnabledForTesting( - QUEUE_NETWORK_AGENT_EVENTS_IN_SYSTEM_SERVER - ) - - @Before fun setUp() { instrumentation.getUiAutomation().adoptShellPermissionIdentity() @@ -1670,17 +1660,16 @@ class NetworkAgentTest { // Connect a third network. Because network1 is awaiting replacement, network3 is preferred // as soon as it validates (until then, it is outscored by network1). - // The fact that the first event seen by matchAllCallback is the connection of network3 + // The fact that the first events seen by matchAllCallback is the connection of network3 // implicitly ensures that no callbacks are sent since network1 was lost. val (agent3, network3) = connectNetwork(lp = lp) + matchAllCallback.expectAvailableThenValidatedCallbacks(network3) + testCallback.expectAvailableDoubleValidatedCallbacks(network3) + sendAndExpectUdpPacket(network3, reader, iface) // As soon as the replacement arrives, network1 is disconnected. // Check that this happens before the replacement timeout (5 seconds) fires. - matchAllCallback.expectAvailableCallbacks(network3, validated = false) matchAllCallback.expect(network1, 2_000 /* timeoutMs */) - matchAllCallback.expectCaps(network3) { it.hasCapability(NET_CAPABILITY_VALIDATED) } - sendAndExpectUdpPacket(network3, reader, iface) - testCallback.expectAvailableDoubleValidatedCallbacks(network3) agent1.expectCallback() // Test lingering: @@ -1728,7 +1717,7 @@ class NetworkAgentTest { val callback = TestableNetworkCallback() requestNetwork(makeTestNetworkRequest(specifier = specifier6), callback) val agent6 = createNetworkAgent(specifier = specifier6) - agent6.register() + val network6 = agent6.register() if (SHOULD_CREATE_NETWORKS_IMMEDIATELY) { agent6.expectCallback() } else { @@ -1798,9 +1787,8 @@ class NetworkAgentTest { val (newWifiAgent, newWifiNetwork) = connectNetwork(TRANSPORT_WIFI) testCallback.expectAvailableCallbacks(newWifiNetwork, validated = true) - matchAllCallback.expectAvailableCallbacks(newWifiNetwork, validated = false) + matchAllCallback.expectAvailableThenValidatedCallbacks(newWifiNetwork) matchAllCallback.expect(wifiNetwork) - matchAllCallback.expectCaps(newWifiNetwork) { it.hasCapability(NET_CAPABILITY_VALIDATED) } wifiAgent.expectCallback() testCallback.expect(newWifiNetwork) diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index 35a7fbd9c8..3eefa0f227 100755 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -3092,24 +3092,23 @@ public class ConnectivityServiceTest { generalCb.expectAvailableCallbacksUnvalidated(net2); if (expectLingering) { generalCb.expectLosing(net1); + } + generalCb.expectCaps(net2, c -> c.hasCapability(NET_CAPABILITY_VALIDATED)); + defaultCb.expectAvailableDoubleValidatedCallbacks(net2); - // Make sure cell 1 is unwanted immediately if the radio can't time share, but only - // after some delay if it can. - generalCb.expectCaps(net2, c -> c.hasCapability(NET_CAPABILITY_VALIDATED)); - defaultCb.expectAvailableDoubleValidatedCallbacks(net2); + // Make sure cell 1 is unwanted immediately if the radio can't time share, but only + // after some delay if it can. + if (expectLingering) { net1.assertNotDisconnected(TEST_CALLBACK_TIMEOUT_MS); // always incurs the timeout generalCb.assertNoCallback(); // assertNotDisconnected waited for TEST_CALLBACK_TIMEOUT_MS, so waiting for the // linger period gives TEST_CALLBACK_TIMEOUT_MS time for the event to process. net1.expectDisconnected(UNREASONABLY_LONG_ALARM_WAIT_MS); - generalCb.expect(LOST, net1); } else { net1.expectDisconnected(TEST_CALLBACK_TIMEOUT_MS); - net1.disconnect(); - generalCb.expect(LOST, net1); - generalCb.expectCaps(net2, c -> c.hasCapability(NET_CAPABILITY_VALIDATED)); - defaultCb.expectAvailableDoubleValidatedCallbacks(net2); } + net1.disconnect(); + generalCb.expect(LOST, net1); // Remove primary from net 2 net2.setScore(new NetworkScore.Builder().build());