From beafc363604d10422266f43caf536bcbae25a533 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Fri, 2 Jun 2017 17:55:55 +0900 Subject: [PATCH 1/2] Minor tweaks to WifiDetailPreferenceControllerTest. Add makeNetworkCapabilities and updateNetworkCapabilities methods, and use them. These are not very useful in the current code but we introduce in their own CL to limit test changes in an upcoming CL that will make use them to test new code. Bug: 62209358 Test: make -j64 RunSettingsRoboTests Change-Id: I67269e1add40ecb3c2b693548e8bf29ae776a79f --- .../WifiDetailPreferenceControllerTest.java | 27 +++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/tests/robotests/src/com/android/settings/wifi/details/WifiDetailPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/wifi/details/WifiDetailPreferenceControllerTest.java index 41825a5fc21..5a4eafc5ad2 100644 --- a/tests/robotests/src/com/android/settings/wifi/details/WifiDetailPreferenceControllerTest.java +++ b/tests/robotests/src/com/android/settings/wifi/details/WifiDetailPreferenceControllerTest.java @@ -445,6 +445,17 @@ public class WifiDetailPreferenceControllerTest { mCallbackCaptor.getValue().onLinkPropertiesChanged(mockNetwork, new LinkProperties(lp)); } + private void updateNetworkCapabilities(NetworkCapabilities nc) { + mCallbackCaptor.getValue().onCapabilitiesChanged(mockNetwork, new NetworkCapabilities(nc)); + } + + private NetworkCapabilities makeNetworkCapabilities() { + NetworkCapabilities nc = new NetworkCapabilities(); + nc.clearAll(); + nc.addTransportType(NetworkCapabilities.TRANSPORT_WIFI); + return nc; + } + private void verifyDisplayedIpv6Addresses(InOrder inOrder, LinkAddress... addresses) { String text = Arrays.stream(addresses) .map(address -> asString(address)) @@ -589,7 +600,7 @@ public class WifiDetailPreferenceControllerTest { } @Test - public void networkDisconnectdState_shouldFinishActivity() { + public void networkDisconnectedState_shouldFinishActivity() { mController.onResume(); when(mockConnectivityManager.getNetworkInfo(any(Network.class))).thenReturn(null); @@ -644,22 +655,16 @@ public class WifiDetailPreferenceControllerTest { inOrder.verify(mockSignInButton).setVisibility(View.INVISIBLE); - NetworkCapabilities nc = new NetworkCapabilities(); - nc.clearAll(); - nc.addTransportType(NetworkCapabilities.TRANSPORT_WIFI); - - NetworkCallback callback = mCallbackCaptor.getValue(); - callback.onCapabilitiesChanged(mockNetwork, nc); + NetworkCapabilities nc = makeNetworkCapabilities(); + updateNetworkCapabilities(nc); inOrder.verify(mockSignInButton).setVisibility(View.INVISIBLE); - nc = new NetworkCapabilities(nc); nc.addCapability(NetworkCapabilities.NET_CAPABILITY_CAPTIVE_PORTAL); - callback.onCapabilitiesChanged(mockNetwork, nc); + updateNetworkCapabilities(nc); inOrder.verify(mockSignInButton).setVisibility(View.VISIBLE); - nc = new NetworkCapabilities(nc); nc.removeCapability(NetworkCapabilities.NET_CAPABILITY_CAPTIVE_PORTAL); - callback.onCapabilitiesChanged(mockNetwork, nc); + updateNetworkCapabilities(nc); inOrder.verify(mockSignInButton).setVisibility(View.INVISIBLE); } From 2561df808328f78f18b3fa0c936fa09ffc91e2c2 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Fri, 2 Jun 2017 18:00:40 +0900 Subject: [PATCH 2/2] When validation state changes, update the AP summary. This stops the AP summary from getting stuck on "Connected, no Internet" even when the system has validated Internet access. Bug: 62209358 Test: make -j64 RunSettingsRoboTests Change-Id: I235404408f7d8b958653d25656d97da8206e35ce --- .../WifiDetailPreferenceController.java | 16 ++++++++ .../WifiDetailPreferenceControllerTest.java | 39 +++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/src/com/android/settings/wifi/details/WifiDetailPreferenceController.java b/src/com/android/settings/wifi/details/WifiDetailPreferenceController.java index c0dd0a87700..0d585c72ad5 100644 --- a/src/com/android/settings/wifi/details/WifiDetailPreferenceController.java +++ b/src/com/android/settings/wifi/details/WifiDetailPreferenceController.java @@ -16,6 +16,7 @@ package com.android.settings.wifi.details; import static android.net.NetworkCapabilities.NET_CAPABILITY_CAPTIVE_PORTAL; +import static android.net.NetworkCapabilities.NET_CAPABILITY_VALIDATED; import static android.net.NetworkCapabilities.TRANSPORT_WIFI; import android.app.Fragment; @@ -165,9 +166,24 @@ public class WifiDetailPreferenceController extends PreferenceController impleme } } + private boolean hasCapabilityChanged(NetworkCapabilities nc, int cap) { + // If this is the first time we get NetworkCapabilities, report that something changed. + if (mNetworkCapabilities == null) return true; + + // nc can never be null, see ConnectivityService#callCallbackForRequest. + return mNetworkCapabilities.hasCapability(cap) != nc.hasCapability(cap); + } + @Override public void onCapabilitiesChanged(Network network, NetworkCapabilities nc) { + // If the network just validated or lost Internet access, refresh network state. + // Don't do this on every NetworkCapabilities change because refreshNetworkState + // sends IPCs to the system server from the UI thread, which can cause jank. if (network.equals(mNetwork) && !nc.equals(mNetworkCapabilities)) { + if (hasCapabilityChanged(nc, NET_CAPABILITY_VALIDATED) || + hasCapabilityChanged(nc, NET_CAPABILITY_CAPTIVE_PORTAL)) { + refreshNetworkState(); + } mNetworkCapabilities = nc; updateIpLayerInfo(); } diff --git a/tests/robotests/src/com/android/settings/wifi/details/WifiDetailPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/wifi/details/WifiDetailPreferenceControllerTest.java index 5a4eafc5ad2..18cfa4e5418 100644 --- a/tests/robotests/src/com/android/settings/wifi/details/WifiDetailPreferenceControllerTest.java +++ b/tests/robotests/src/com/android/settings/wifi/details/WifiDetailPreferenceControllerTest.java @@ -518,6 +518,45 @@ public class WifiDetailPreferenceControllerTest { inOrder.verify(mockDnsPref).setVisible(true); } + @Test + public void onCapabilitiesChanged_callsRefreshIfNecessary() { + NetworkCapabilities nc = makeNetworkCapabilities(); + when(mockConnectivityManager.getNetworkCapabilities(mockNetwork)) + .thenReturn(new NetworkCapabilities(nc)); + + String summary = "Connected, no Internet"; + when(mockAccessPoint.getSettingsSummary()).thenReturn(summary); + + InOrder inOrder = inOrder(mockConnectionDetailPref); + mController.displayPreference(mockScreen); + mController.onResume(); + inOrder.verify(mockConnectionDetailPref).setTitle(summary); + + // Check that an irrelevant capability update does not update the access point summary, as + // doing so could cause unnecessary jank... + summary = "Connected"; + when(mockAccessPoint.getSettingsSummary()).thenReturn(summary); + updateNetworkCapabilities(nc); + inOrder.verify(mockConnectionDetailPref, never()).setTitle(any()); + + // ... but that if the network validates, then we do refresh. + nc.addCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED); + updateNetworkCapabilities(nc); + inOrder.verify(mockConnectionDetailPref).setTitle(summary); + + summary = "Connected, no Internet"; + when(mockAccessPoint.getSettingsSummary()).thenReturn(summary); + + // Another irrelevant update won't cause the UI to refresh... + updateNetworkCapabilities(nc); + inOrder.verify(mockConnectionDetailPref, never()).setTitle(any()); + + // ... but if the network is no longer validated, then we display "connected, no Internet". + nc.removeCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED); + updateNetworkCapabilities(nc); + inOrder.verify(mockConnectionDetailPref).setTitle(summary); + } + @Test public void canForgetNetwork_noNetwork() { when(mockAccessPoint.getConfig()).thenReturn(null);