From f9bff5b0dc8603f3d7de4ee6d28bd3ae5f7af95c Mon Sep 17 00:00:00 2001 From: lucaslin Date: Wed, 20 Mar 2019 18:21:59 +0800 Subject: [PATCH] Improve partial connectivity Improve the design and fix some nits. Bug: 113450764 Test: 1. Build pass 2. atest FrameworksNetTests 3. atest NetworkStackTests 4. Change captive_portal_https_url to https://invalid.com to simulate partial connectivity. Change-Id: Ia56645841d00d2ed8406cfeacb86a4a27fd58650 --- .../android/server/ConnectivityService.java | 34 +++++++------------ .../server/ConnectivityServiceTest.java | 9 +++-- 2 files changed, 16 insertions(+), 27 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 524d5c66b3..8384ca054f 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -2552,19 +2552,11 @@ public class ConnectivityService extends IConnectivityManager.Stub final boolean partialConnectivity = (msg.arg1 == NETWORK_TEST_RESULT_PARTIAL_CONNECTIVITY) - // If user accepts partial connectivity network, NetworkMonitor - // will skip https probing. It will make partial connectivity - // network becomes valid. But user still need to know this - // network is limited. So, it's needed to refer to - // acceptPartialConnectivity to add - // NET_CAPABILITY_PARTIAL_CONNECTIVITY into NetworkCapabilities - // of this network. So that user can see "Limited connection" - // in the settings. || (nai.networkMisc.acceptPartialConnectivity && nai.partialConnectivity); // Once a network is determined to have partial connectivity, it cannot // go back to full connectivity without a disconnect. - final boolean partialConnectivityChange = + final boolean partialConnectivityChanged = (partialConnectivity && !nai.partialConnectivity); final boolean valid = (msg.arg1 == NETWORK_TEST_RESULT_VALID); @@ -2575,17 +2567,6 @@ public class ConnectivityService extends IConnectivityManager.Stub nai.captivePortalLoginNotified = true; showNetworkNotification(nai, NotificationType.LOGGED_IN); } - // If this network has just connected and partial connectivity has just been - // detected, tell NetworkMonitor if the user accepted partial connectivity on a - // previous connect. - if ((msg.arg1 == NETWORK_TEST_RESULT_PARTIAL_CONNECTIVITY) - && nai.networkMisc.acceptPartialConnectivity) { - try { - nai.networkMonitor().notifyAcceptPartialConnectivity(); - } catch (RemoteException e) { - e.rethrowFromSystemServer(); - } - } final String redirectUrl = (msg.obj instanceof String) ? (String) msg.obj : ""; @@ -2615,7 +2596,7 @@ public class ConnectivityService extends IConnectivityManager.Stub mNotifier.clearNotification(nai.network.netId, NotificationType.LOST_INTERNET); } - } else if (partialConnectivityChange) { + } else if (partialConnectivityChanged) { nai.partialConnectivity = partialConnectivity; updateCapabilities(nai.getCurrentScore(), nai, nai.networkCapabilities); } @@ -3369,8 +3350,11 @@ public class ConnectivityService extends IConnectivityManager.Stub // Tear down the network. teardownUnneededNetwork(nai); } else { + // Inform NetworkMonitor that partial connectivity is acceptable. This will likely + // result in a partial connectivity result which will be processed by + // maybeHandleNetworkMonitorMessage. try { - nai.networkMonitor().notifyAcceptPartialConnectivity(); + nai.networkMonitor().setAcceptPartialConnectivity(); } catch (RemoteException e) { e.rethrowFromSystemServer(); } @@ -3578,6 +3562,9 @@ public class ConnectivityService extends IConnectivityManager.Stub // because we're already prompting the user to sign in. if (nai == null || nai.everValidated || nai.everCaptivePortalDetected || !nai.networkMisc.explicitlySelected || nai.networkMisc.acceptUnvalidated + // TODO: Once the value of acceptPartialConnectivity is moved to IpMemoryStore, + // we should reevaluate how to handle acceptPartialConnectivity when network just + // connected. || nai.networkMisc.acceptPartialConnectivity) { return; } @@ -6386,6 +6373,9 @@ public class ConnectivityService extends IConnectivityManager.Stub // NetworkMonitor seeing the correct LinkProperties when starting. // TODO: pass LinkProperties to the NetworkMonitor in the notifyNetworkConnected call. try { + if (networkAgent.networkMisc.acceptPartialConnectivity) { + networkAgent.networkMonitor().setAcceptPartialConnectivity(); + } networkAgent.networkMonitor().notifyNetworkConnected(); } catch (RemoteException e) { e.rethrowFromSystemServer(); diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 30936294e6..c14d0af0a4 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -495,7 +495,7 @@ public class ConnectivityServiceTest { try { doAnswer(validateAnswer).when(mNetworkMonitor).notifyNetworkConnected(); doAnswer(validateAnswer).when(mNetworkMonitor).forceReevaluation(anyInt()); - doAnswer(validateAnswer).when(mNetworkMonitor).notifyAcceptPartialConnectivity(); + doAnswer(validateAnswer).when(mNetworkMonitor).setAcceptPartialConnectivity(); } catch (RemoteException e) { fail(e.getMessage()); } @@ -2552,8 +2552,7 @@ public class ConnectivityServiceTest { verifyActiveNetwork(TRANSPORT_CELLULAR); } - // TODO: deflake and re-enable - // @Test + @Test public void testPartialConnectivity() { // Register network callback. NetworkRequest request = new NetworkRequest.Builder() @@ -2587,7 +2586,7 @@ public class ConnectivityServiceTest { waitForIdle(); try { verify(mWiFiNetworkAgent.mNetworkMonitor, - timeout(TIMEOUT_MS).times(1)).notifyAcceptPartialConnectivity(); + timeout(TIMEOUT_MS).times(1)).setAcceptPartialConnectivity(); } catch (RemoteException e) { fail(e.getMessage()); } @@ -2643,7 +2642,7 @@ public class ConnectivityServiceTest { waitForIdle(); try { verify(mWiFiNetworkAgent.mNetworkMonitor, - timeout(TIMEOUT_MS).times(1)).notifyAcceptPartialConnectivity(); + timeout(TIMEOUT_MS).times(1)).setAcceptPartialConnectivity(); } catch (RemoteException e) { fail(e.getMessage()); }