LineageOS/android_packages_modules_Connectivity
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Do callback unregistration automatically in tearDown

Browse Source 
Ideally, unregisterCallbackQuietly() is only needed when the
callback is registered in the try block. The callbacks
registration located outside the try block should be
registered theoretically. Otherwise, it may be a bug if the
tests get the IllegalArgumentException.

Use unregisterCallbackQuietly() in all finally may cause bug
in unregisterNetworkCallback invisible.

However, it may cause different tests with different code to
do the unregistration. Thus, do a refactor to add some methods
to wrap the NetworkCallback registration. This allows the tests
to do the unregister automatically in the tearDown().

Bug: 195364778
Test: atest android.net.cts.ConnectivityManagerTest
Change-Id: If06993f643ac7aeb23638a283347c427934a2f99
This commit is contained in:
Chiachang Wang
2021-08-05 10:14:17 +08:00
parent 740cda4358
commit 7a45f107de
1 changed files with 124 additions and 128 deletions
Download Patch File Download Diff File Expand all files Collapse all files

190
tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java
View File

@@ -282,7 +282,8 @@ public class ConnectivityManagerTest {
private final ArraySet<Integer> mNetworkTypes = new ArraySet<>();
private UiAutomation mUiAutomation;
private CtsNetUtils mCtsNetUtils;
// The registered callbacks.
private List<NetworkCallback> mRegisteredCallbacks = new ArrayList<>();
// Used for cleanup purposes.
private final List<Range<Integer>> mVpnRequiredUidRanges = new ArrayList<>();
@@ -377,11 +378,12 @@ public class ConnectivityManagerTest {
// All tests in this class require a working Internet connection as they start. Make
// sure there is still one as they end that's ready to use for the next test to use.
final TestNetworkCallback callback = new TestNetworkCallback();
mCm.registerDefaultNetworkCallback(callback);
registerDefaultNetworkCallback(callback);
try {
assertNotNull("Couldn't restore Internet connectivity", callback.waitForAvailable());
} finally {
unregisterCallbackQuietly(callback);
// Unregister all registered callbacks.
unregisterRegisteredCallbacks();
}
}
@@ -716,7 +718,7 @@ public class ConnectivityManagerTest {
final String goodPrivateDnsServer = "dns.google";
mCtsNetUtils.storePrivateDnsSetting();
final TestableNetworkCallback cb = new TestableNetworkCallback();
mCm.registerNetworkCallback(makeWifiNetworkRequest(), cb);
registerNetworkCallback(makeWifiNetworkRequest(), cb);
try {
// Verifying the good private DNS sever
mCtsNetUtils.setPrivateDnsStrictMode(goodPrivateDnsServer);
@@ -748,15 +750,15 @@ public class ConnectivityManagerTest {
*/
@AppModeFull(reason = "Cannot get WifiManager in instant app mode")
@Test
public void testRegisterNetworkCallback() {
public void testRegisterNetworkCallback() throws Exception {
assumeTrue(mPackageManager.hasSystemFeature(FEATURE_WIFI));
// We will register for a WIFI network being available or lost.
final TestNetworkCallback callback = new TestNetworkCallback();
mCm.registerNetworkCallback(makeWifiNetworkRequest(), callback);
registerNetworkCallback(makeWifiNetworkRequest(), callback);
final TestNetworkCallback defaultTrackingCallback = new TestNetworkCallback();
mCm.registerDefaultNetworkCallback(defaultTrackingCallback);
registerDefaultNetworkCallback(defaultTrackingCallback);
final TestNetworkCallback systemDefaultCallback = new TestNetworkCallback();
final TestNetworkCallback perUidCallback = new TestNetworkCallback();
@@ -764,15 +766,13 @@ public class ConnectivityManagerTest {
final Handler h = new Handler(Looper.getMainLooper());
if (TestUtils.shouldTestSApis()) {
runWithShellPermissionIdentity(() -> {
mCmShim.registerSystemDefaultNetworkCallback(systemDefaultCallback, h);
mCmShim.registerDefaultNetworkCallbackForUid(Process.myUid(), perUidCallback, h);
registerSystemDefaultNetworkCallback(systemDefaultCallback, h);
registerDefaultNetworkCallbackForUid(Process.myUid(), perUidCallback, h);
}, NETWORK_SETTINGS);
mCm.registerBestMatchingNetworkCallback(makeDefaultRequest(), bestMatchingCallback, h);
registerBestMatchingNetworkCallback(makeDefaultRequest(), bestMatchingCallback, h);
}
Network wifiNetwork = null;
try {
mCtsNetUtils.ensureWifiConnected();
// Now we should expect to get a network callback about availability of the wifi
@@ -798,18 +798,6 @@ public class ConnectivityManagerTest {
bestMatchingNetwork);
assertEquals(defaultNetwork, bestMatchingNetwork);
}
} catch (InterruptedException e) {
fail("Broadcast receiver or NetworkCallback wait was interrupted.");
} finally {
unregisterCallbackQuietly(callback);
unregisterCallbackQuietly(defaultTrackingCallback);
if (TestUtils.shouldTestSApis()) {
unregisterCallbackQuietly(systemDefaultCallback);
unregisterCallbackQuietly(perUidCallback);
unregisterCallbackQuietly(bestMatchingCallback);
}
}
}
/**
@@ -979,22 +967,15 @@ public class ConnectivityManagerTest {
*/
@AppModeFull(reason = "CHANGE_NETWORK_STATE permission can't be granted to instant apps")
@Test
public void testRequestNetworkCallback() {
public void testRequestNetworkCallback() throws Exception {
final TestNetworkCallback callback = new TestNetworkCallback();
mCm.requestNetwork(new NetworkRequest.Builder()
requestNetwork(new NetworkRequest.Builder()
.addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)
.build(), callback);
try {
// Wait to get callback for availability of internet
Network internetNetwork = callback.waitForAvailable();
assertNotNull("Did not receive NetworkCallback#onAvailable for INTERNET",
internetNetwork);
} catch (InterruptedException e) {
fail("NetworkCallback wait was interrupted.");
} finally {
unregisterCallbackQuietly(callback);
}
assertNotNull("Did not receive NetworkCallback#onAvailable for INTERNET", internetNetwork);
}
/**
@@ -1010,9 +991,8 @@ public class ConnectivityManagerTest {
}
final TestNetworkCallback callback = new TestNetworkCallback();
mCm.requestNetwork(new NetworkRequest.Builder()
.addTransportType(TRANSPORT_WIFI)
.build(), callback, 100);
requestNetwork(new NetworkRequest.Builder().addTransportType(TRANSPORT_WIFI).build(),
callback, 100);
try {
// Wait to get callback for unavailability of requested network
@@ -1021,7 +1001,6 @@ public class ConnectivityManagerTest {
} catch (InterruptedException e) {
fail("NetworkCallback wait was interrupted.");
} finally {
unregisterCallbackQuietly(callback);
if (previousWifiEnabledState) {
mCtsNetUtils.connectToWifi();
}
@@ -1124,11 +1103,11 @@ public class ConnectivityManagerTest {
// this method will return right away, and if not, it'll wait for the setting to change.
if (useSystemDefault) {
runWithShellPermissionIdentity(() ->
mCmShim.registerSystemDefaultNetworkCallback(networkCallback,
registerSystemDefaultNetworkCallback(networkCallback,
new Handler(Looper.getMainLooper())),
NETWORK_SETTINGS);
} else {
mCm.registerDefaultNetworkCallback(networkCallback);
registerDefaultNetworkCallback(networkCallback);
}
// Changing meteredness on wifi involves reconnecting, which can take several seconds
@@ -1138,8 +1117,6 @@ public class ConnectivityManagerTest {
throw new AssertionError("Timed out waiting for active network metered status to "
+ "change to " + requestedMeteredness + " ; network = "
+ mCm.getActiveNetwork(), e);
} finally {
mCm.unregisterNetworkCallback(networkCallback);
}
}
@@ -1875,8 +1852,6 @@ public class ConnectivityManagerTest {
if (supportWifi) waitForAvailable(wifiCb);
if (supportTelephony) waitForAvailable(telephonyCb);
} finally {
if (supportWifi) unregisterCallbackQuietly(wifiCb);
if (supportTelephony) unregisterCallbackQuietly(telephonyCb);
// Restore the previous state of airplane mode and permissions:
runShellCommand("cmd connectivity airplane-mode "
+ (isAirplaneModeEnabled ? "enable" : "disable"));
@@ -1886,7 +1861,7 @@ public class ConnectivityManagerTest {
private void registerCallbackAndWaitForAvailable(@NonNull final NetworkRequest request,
@NonNull final TestableNetworkCallback cb) {
mCm.registerNetworkCallback(request, cb);
registerNetworkCallback(request, cb);
waitForAvailable(cb);
}
@@ -2004,8 +1979,8 @@ public class ConnectivityManagerTest {
foundNc.complete(nc);
}
};
try {
mCm.registerNetworkCallback(makeWifiNetworkRequest(), callback);
registerNetworkCallback(makeWifiNetworkRequest(), callback);
// Registering a callback here guarantees onCapabilitiesChanged is called immediately
// because WiFi network should be connected.
final NetworkCapabilities nc =
@@ -2013,9 +1988,6 @@ public class ConnectivityManagerTest {
// Verify if ssid is contained in the NetworkCapabilities received from callback.
assertNotNull("NetworkCapabilities of the network is null", nc);
assertEquals(hasSsid, Pattern.compile(ssid).matcher(nc.toString()).find());
} finally {
unregisterCallbackQuietly(callback);
}
}
/**
@@ -2053,14 +2025,14 @@ public class ConnectivityManagerTest {
final TestableNetworkCallback callback = new TestableNetworkCallback();
final Handler handler = new Handler(Looper.getMainLooper());
assertThrows(SecurityException.class,
() -> mCmShim.requestBackgroundNetwork(testRequest, callback, handler));
() -> requestBackgroundNetwork(testRequest, callback, handler));
Network testNetwork = null;
try {
// Request background test network via Shell identity which has NETWORK_SETTINGS
// permission granted.
runWithShellPermissionIdentity(
() -> mCmShim.requestBackgroundNetwork(testRequest, callback, handler),
() -> requestBackgroundNetwork(testRequest, callback, handler),
new String[] { android.Manifest.permission.NETWORK_SETTINGS });
// Register the test network agent which has no foreground request associated to it.
@@ -2097,7 +2069,6 @@ public class ConnectivityManagerTest {
}
testNetworkInterface.getFileDescriptor().close();
}, new String[] { android.Manifest.permission.MANAGE_TEST_NETWORKS });
unregisterCallbackQuietly(callback);
}
}
@@ -2137,8 +2108,9 @@ public class ConnectivityManagerTest {
final int myUid = Process.myUid();
final int otherUid = UserHandle.getUid(5, Process.FIRST_APPLICATION_UID);
final Handler handler = new Handler(Looper.getMainLooper());
mCm.registerDefaultNetworkCallback(myUidCallback, handler);
mCmShim.registerDefaultNetworkCallbackForUid(otherUid, otherUidCallback, handler);
registerDefaultNetworkCallback(myUidCallback, handler);
registerDefaultNetworkCallbackForUid(otherUid, otherUidCallback, handler);
final Network defaultNetwork = mCm.getActiveNetwork();
final List<DetailedBlockedStatusCallback> allCallbacks =
@@ -2185,16 +2157,15 @@ public class ConnectivityManagerTest {
assertNotNull(info);
assertEquals(DetailedState.CONNECTED, info.getDetailedState());
final TestableNetworkCallback callback = new TestableNetworkCallback();
try {
mCmShim.setLegacyLockdownVpnEnabled(true);
// setLegacyLockdownVpnEnabled is asynchronous and only takes effect when the
// ConnectivityService handler thread processes it. Ensure it has taken effect by doing
// something that blocks until the handler thread is idle.
final TestableNetworkCallback callback = new TestableNetworkCallback();
mCm.registerDefaultNetworkCallback(callback);
registerDefaultNetworkCallback(callback);
waitForAvailable(callback);
mCm.unregisterNetworkCallback(callback);
// Test one of the effects of setLegacyLockdownVpnEnabled: the fact that any NetworkInfo
// in state CONNECTED is degraded to CONNECTING if the legacy VPN is not connected.
@@ -2383,7 +2354,6 @@ public class ConnectivityManagerTest {
// Cleanup any prior test state from setOemNetworkPreference
clearOemNetworkPreference();
unregisterTestOemNetworkPreferenceCallbacks(defaultCallback, systemDefaultCallback);
}
}
@@ -2427,23 +2397,15 @@ public class ConnectivityManagerTest {
// The default (non-test) network should be available as the network pref was cleared.
waitForAvailable(defaultCallback);
unregisterTestOemNetworkPreferenceCallbacks(defaultCallback, systemDefaultCallback);
}
}
private void unregisterTestOemNetworkPreferenceCallbacks(
@NonNull final TestableNetworkCallback defaultCallback,
@NonNull final TestableNetworkCallback systemDefaultCallback) {
mCm.unregisterNetworkCallback(defaultCallback);
mCm.unregisterNetworkCallback(systemDefaultCallback);
}
private void registerTestOemNetworkPreferenceCallbacks(
@NonNull final TestableNetworkCallback defaultCallback,
@NonNull final TestableNetworkCallback systemDefaultCallback) {
mCm.registerDefaultNetworkCallback(defaultCallback);
registerDefaultNetworkCallback(defaultCallback);
runWithShellPermissionIdentity(() ->
mCmShim.registerSystemDefaultNetworkCallback(systemDefaultCallback,
registerSystemDefaultNetworkCallback(systemDefaultCallback,
new Handler(Looper.getMainLooper())), NETWORK_SETTINGS);
}
@@ -2544,7 +2506,7 @@ public class ConnectivityManagerTest {
// Wait for partial connectivity to be detected on the network
final Network network = preparePartialConnectivity();
mCm.requestNetwork(makeWifiNetworkRequest(), cb);
requestNetwork(makeWifiNetworkRequest(), cb);
runAsShell(NETWORK_SETTINGS, () -> {
// The always bit is verified in NetworkAgentTest
mCm.setAcceptPartialConnectivity(network, false /* accept */, false /* always */);
@@ -2552,7 +2514,6 @@ public class ConnectivityManagerTest {
// Reject partial connectivity network should cause the network being torn down
assertEquals(network, cb.waitForLost());
} finally {
unregisterCallbackQuietly(cb);
resetValidationConfig();
// Wifi will not automatically reconnect to the network. ensureWifiDisconnected cannot
// apply here. Thus, turn off wifi first and restart to restore.
@@ -2587,13 +2548,12 @@ public class ConnectivityManagerTest {
// guarantee that it won't become the default in the future.
assertNotEquals(wifiNetwork, mCm.getActiveNetwork());
mCm.registerNetworkCallback(makeWifiNetworkRequest(), wifiCb);
registerNetworkCallback(makeWifiNetworkRequest(), wifiCb);
runAsShell(NETWORK_SETTINGS, () -> {
mCm.setAcceptUnvalidated(wifiNetwork, false /* accept */, false /* always */);
});
waitForLost(wifiCb);
} finally {
unregisterCallbackQuietly(wifiCb);
resetValidationConfig();
/// Wifi will not automatically reconnect to the network. ensureWifiDisconnected cannot
// apply here. Thus, turn off wifi first and restart to restore.
@@ -2623,8 +2583,8 @@ public class ConnectivityManagerTest {
final Network cellNetwork = mCtsNetUtils.connectToCell();
final Network wifiNetwork = prepareValidatedNetwork();
mCm.registerDefaultNetworkCallback(defaultCb);
mCm.registerNetworkCallback(makeWifiNetworkRequest(), wifiCb);
registerDefaultNetworkCallback(defaultCb);
registerNetworkCallback(makeWifiNetworkRequest(), wifiCb);
try {
// Verify wifi is the default network.
@@ -2658,8 +2618,6 @@ public class ConnectivityManagerTest {
wifiCb.assertNoCallbackThat(NO_CALLBACK_TIMEOUT_MS,
c -> !(c instanceof CallbackEntry.LinkPropertiesChanged));
} finally {
unregisterCallbackQuietly(wifiCb);
unregisterCallbackQuietly(defaultCb);
resetAvoidBadWifi(previousAvoidBadWifi);
resetValidationConfig();
// Reconnect wifi to reset the wifi status
@@ -2697,12 +2655,8 @@ public class ConnectivityManagerTest {
}
};
try {
mCm.registerNetworkCallback(new NetworkRequest.Builder().build(), cb);
registerNetworkCallback(new NetworkRequest.Builder().build(), cb);
return future.get(timeout, TimeUnit.MILLISECONDS);
} finally {
unregisterCallbackQuietly(cb);
}
}
private void resetValidationConfig() {
@@ -2788,14 +2742,6 @@ public class ConnectivityManagerTest {
System.currentTimeMillis() + WIFI_CONNECT_TIMEOUT_MS);
}
private void unregisterCallbackQuietly(NetworkCallback cb) {
try {
mCm.unregisterNetworkCallback(cb);
} catch (IllegalArgumentException e) {
Log.i(TAG, "Try to unregister a not registered NetworkCallback!");
}
}
@AppModeFull(reason = "Cannot get WifiManager in instant app mode")
@Test
public void testMobileDataPreferredUids() throws Exception {
@@ -2820,9 +2766,9 @@ public class ConnectivityManagerTest {
final TestableNetworkCallback defaultTrackingCb = new TestableNetworkCallback();
final TestableNetworkCallback systemDefaultCb = new TestableNetworkCallback();
final Handler h = new Handler(Looper.getMainLooper());
runWithShellPermissionIdentity(() -> mCm.registerSystemDefaultNetworkCallback(
runWithShellPermissionIdentity(() -> registerSystemDefaultNetworkCallback(
systemDefaultCb, h), NETWORK_SETTINGS);
mCm.registerDefaultNetworkCallback(defaultTrackingCb);
registerDefaultNetworkCallback(defaultTrackingCb);
try {
// CtsNetTestCases uid is not listed in MOBILE_DATA_PREFERRED_UIDS setting, so the
@@ -2858,9 +2804,6 @@ public class ConnectivityManagerTest {
// Active network for CtsNetTestCases uid should change back to wifi.
assertEquals(wifiNetwork, mCm.getActiveNetwork());
} finally {
unregisterCallbackQuietly(systemDefaultCb);
unregisterCallbackQuietly(defaultTrackingCb);
// Restore setting.
ConnectivitySettingsManager.setMobileDataPreferredUids(
mContext, mobileDataPreferredUids);
@@ -2911,7 +2854,7 @@ public class ConnectivityManagerTest {
final Handler h = new Handler(Looper.getMainLooper());
final TestableNetworkCallback testNetworkCb = new TestableNetworkCallback();
mCm.registerBestMatchingNetworkCallback(new NetworkRequest.Builder().clearCapabilities()
registerBestMatchingNetworkCallback(new NetworkRequest.Builder().clearCapabilities()
.addTransportType(NetworkCapabilities.TRANSPORT_TEST).build(), testNetworkCb, h);
// Create test network agent with restricted network.
@@ -2961,7 +2904,6 @@ public class ConnectivityManagerTest {
// TODD: Have a significant signal to know the uids has been send to netd.
assertBindSocketToNetworkSuccess(network);
} finally {
unregisterCallbackQuietly(testNetworkCb);
agent.unregister();
// Restore setting.
@@ -2970,4 +2912,58 @@ public class ConnectivityManagerTest {
mContext, originalUidsAllowedOnRestrictedNetworks), NETWORK_SETTINGS);
}
}
private void unregisterRegisteredCallbacks() {
for (NetworkCallback callback: mRegisteredCallbacks) {
mCm.unregisterNetworkCallback(callback);
}
}
private void registerDefaultNetworkCallback(NetworkCallback callback) {
mCm.registerDefaultNetworkCallback(callback);
mRegisteredCallbacks.add(callback);
}
private void registerDefaultNetworkCallback(NetworkCallback callback, Handler handler) {
mCm.registerDefaultNetworkCallback(callback, handler);
mRegisteredCallbacks.add(callback);
}
private void registerNetworkCallback(NetworkRequest request, NetworkCallback callback) {
mCm.registerNetworkCallback(request, callback);
mRegisteredCallbacks.add(callback);
}
private void registerSystemDefaultNetworkCallback(NetworkCallback callback, Handler handler) {
mCmShim.registerSystemDefaultNetworkCallback(callback, handler);
mRegisteredCallbacks.add(callback);
}
private void registerDefaultNetworkCallbackForUid(int uid, NetworkCallback callback,
Handler handler) throws Exception {
mCmShim.registerDefaultNetworkCallbackForUid(uid, callback, handler);
mRegisteredCallbacks.add(callback);
}
private void requestNetwork(NetworkRequest request, NetworkCallback callback) {
mCm.requestNetwork(request, callback);
mRegisteredCallbacks.add(callback);
}
private void requestNetwork(NetworkRequest request, NetworkCallback callback, int timeoutSec) {
mCm.requestNetwork(request, callback, timeoutSec);
mRegisteredCallbacks.add(callback);
}
private void registerBestMatchingNetworkCallback(NetworkRequest request,
NetworkCallback callback, Handler handler) {
mCm.registerBestMatchingNetworkCallback(request, callback, handler);
mRegisteredCallbacks.add(callback);
}
private void requestBackgroundNetwork(NetworkRequest request, NetworkCallback callback,
Handler handler) throws Exception {
mCmShim.requestBackgroundNetwork(request, callback, handler);
mRegisteredCallbacks.add(callback);
}
}