From 19dcc447c47b732bc2a3f0802097031accaa8ecb Mon Sep 17 00:00:00 2001 From: Satish Yalla Date: Wed, 19 Mar 2025 20:18:37 -0700 Subject: [PATCH] Revert "Always disconnect agents immediately." Revert submission 3548747 Reason for revert: Droidmonitor created revert due to b/404956859. Will be verified through ABTD for standard investigation. Reverted changes: /q/submissionid:3548747 Change-Id: I44affb742ca066406e93fc75056b4d3d1298eadb --- .../android/server/ConnectivityService.java | 27 +++---------------- .../src/android/net/cts/NetworkAgentTest.kt | 14 +++++----- .../server/ConnectivityServiceTest.java | 17 ++++++------ 3 files changed, 18 insertions(+), 40 deletions(-) diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 48a825aa25..9c70e7c4e6 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -4894,11 +4894,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; } } @@ -5513,11 +5509,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. @@ -7021,11 +7012,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); @@ -11264,11 +11251,7 @@ public class ConnectivityService extends IConnectivityManager.Stub { break; } } - if (mQueueNetworkAgentEventsInSystemServer) { - disconnectAndDestroyNetwork(nai); - } else { - nai.disconnect(); - } + nai.disconnect(); } private void handleLingerComplete(NetworkAgentInfo oldNetwork) { @@ -12269,9 +12252,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); } diff --git a/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt b/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt index 92155c3287..bd9bd2a5f9 100644 --- a/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt +++ b/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt @@ -1660,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: @@ -1718,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 { @@ -1788,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());