From 678722f9d0b73c3a080d21daa33b87de2534bd05 Mon Sep 17 00:00:00 2001 From: Anarghya Mitra Date: Fri, 11 May 2018 14:29:54 -0700 Subject: [PATCH] Change status bar icons upon capability changes in the default network. SysUI status bar updates currently happen upon receiving either the CONNECTIVITY_ACTION broadcast (which is deprecated) and INET_CONDITION_ACTION broadcast (which is sent upon validation state change of networks only). This leads to status bar showing stale connectivity state. The correct fix for this is to listen to changes in network state by registering NetworkCallbacks (see more details in http://b/79286300#comment9). In the P timeframe, not listening to the broadcasts completely is out of scope. So this CL just listens for changing network capabilities of the default data network, which should fix all the cases where the broadcasts are not getting sent. Later, when we stop relying on the broadcasts, we will also have to override onAvailable and onLost (and perhaps the other callbacks too). Test: runtest --path frameworks/base/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/ Bug: 71904788 Change-Id: I2e58b9cfceb9937a0b54874dee116ead5339b37b --- .../policy/NetworkControllerImpl.java | 33 +++++++++ .../policy/NetworkControllerBaseTest.java | 36 ++++++++-- .../policy/NetworkControllerDataTest.java | 6 +- .../policy/NetworkControllerEthernetTest.java | 2 +- .../policy/NetworkControllerSignalTest.java | 6 +- .../policy/NetworkControllerWifiTest.java | 71 ++++++++++++++++--- 6 files changed, 131 insertions(+), 23 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/policy/NetworkControllerImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/policy/NetworkControllerImpl.java index 4c100cdb99ffd..25261c0ad2cae 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/policy/NetworkControllerImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/policy/NetworkControllerImpl.java @@ -23,6 +23,7 @@ import android.content.IntentFilter; import android.content.res.Configuration; import android.content.res.Resources; import android.net.ConnectivityManager; +import android.net.Network; import android.net.NetworkCapabilities; import android.net.wifi.WifiManager; import android.os.AsyncTask; @@ -219,6 +220,38 @@ public class NetworkControllerImpl extends BroadcastReceiver deviceProvisionedController.getCurrentUser())); } }); + + ConnectivityManager.NetworkCallback callback = new ConnectivityManager.NetworkCallback(){ + private Network mLastNetwork; + private NetworkCapabilities mLastNetworkCapabilities; + + @Override + public void onCapabilitiesChanged( + Network network, NetworkCapabilities networkCapabilities) { + boolean lastValidated = (mLastNetworkCapabilities != null) && + mLastNetworkCapabilities.hasCapability(NET_CAPABILITY_VALIDATED); + boolean validated = + networkCapabilities.hasCapability(NET_CAPABILITY_VALIDATED); + + // This callback is invoked a lot (i.e. when RSSI changes), so avoid updating + // icons when connectivity state has remained the same. + if (network.equals(mLastNetwork) && + networkCapabilities.equalsTransportTypes(mLastNetworkCapabilities) && + validated == lastValidated) { + return; + } + mLastNetwork = network; + mLastNetworkCapabilities = networkCapabilities; + updateConnectivity(); + } + }; + // Even though this callback runs on the receiver handler thread which also processes the + // CONNECTIVITY_ACTION broadcasts, the broadcast and callback might come in at different + // times. This is safe since updateConnectivity() builds the list of transports from + // scratch. + // TODO: Move off of the deprecated CONNECTIVITY_ACTION broadcast and rely on callbacks + // exclusively for status bar icons. + mConnectivityManager.registerDefaultNetworkCallback(callback, mReceiverHandler); } public DataSaverController getDataSaverController() { diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/NetworkControllerBaseTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/NetworkControllerBaseTest.java index 714ad1f049cf1..35f0dba12cb9c 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/NetworkControllerBaseTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/NetworkControllerBaseTest.java @@ -17,20 +17,25 @@ package com.android.systemui.statusbar.policy; import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertNotNull; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyBoolean; import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; +import static org.mockito.Matchers.isA; +import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import android.content.Intent; import android.net.ConnectivityManager; +import android.net.Network; import android.net.NetworkCapabilities; import android.net.wifi.WifiManager; +import android.os.Handler; import android.provider.Settings; import android.provider.Settings.Global; import android.telephony.PhoneStateListener; @@ -90,6 +95,7 @@ public class NetworkControllerBaseTest extends SysuiTestCase { protected int mSubId; private NetworkCapabilities mNetCapabilities; + private ConnectivityManager.NetworkCallback mNetworkCallback; @Rule public TestWatcher failWatcher = new TestWatcher() { @@ -154,6 +160,13 @@ public class NetworkControllerBaseTest extends SysuiTestCase { setSubscriptions(mSubId); mMobileSignalController = mNetworkController.mMobileSignalControllers.get(mSubId); mPhoneStateListener = mMobileSignalController.mPhoneStateListener; + + ArgumentCaptor callbackArg = + ArgumentCaptor.forClass(ConnectivityManager.NetworkCallback.class); + Mockito.verify(mMockCm, atLeastOnce()) + .registerDefaultNetworkCallback(callbackArg.capture(), isA(Handler.class)); + mNetworkCallback = callbackArg.getValue(); + assertNotNull(mNetworkCallback); } protected void setDefaultSubId(int subId) { @@ -195,24 +208,37 @@ public class NetworkControllerBaseTest extends SysuiTestCase { setLevel(DEFAULT_LEVEL); updateDataConnectionState(TelephonyManager.DATA_CONNECTED, TelephonyManager.NETWORK_TYPE_UMTS); - setConnectivity(NetworkCapabilities.TRANSPORT_CELLULAR, true, true); + setConnectivityViaBroadcast( + NetworkCapabilities.TRANSPORT_CELLULAR, true, true); } - public void setConnectivity(int networkType, boolean inetCondition, boolean isConnected) { + public void setConnectivityViaBroadcast( + int networkType, boolean validated, boolean isConnected) { + setConnectivityCommon(networkType, validated, isConnected); Intent i = new Intent(ConnectivityManager.INET_CONDITION_ACTION); + mNetworkController.onReceive(mContext, i); + } + + public void setConnectivityViaCallback( + int networkType, boolean validated, boolean isConnected){ + setConnectivityCommon(networkType, validated, isConnected); + mNetworkCallback.onCapabilitiesChanged( + mock(Network.class), new NetworkCapabilities(mNetCapabilities)); + } + + private void setConnectivityCommon( + int networkType, boolean validated, boolean isConnected){ // TODO: Separate out into several NetworkCapabilities. if (isConnected) { mNetCapabilities.addTransportType(networkType); } else { mNetCapabilities.removeTransportType(networkType); } - if (inetCondition) { + if (validated) { mNetCapabilities.addCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED); } else { mNetCapabilities.removeCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED); } - - mNetworkController.onReceive(mContext, i); } public void setGsmRoaming(boolean isRoaming) { diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/NetworkControllerDataTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/NetworkControllerDataTest.java index 365a9b28e8a86..d42940a3d2a25 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/NetworkControllerDataTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/NetworkControllerDataTest.java @@ -118,7 +118,7 @@ public class NetworkControllerDataTest extends NetworkControllerBaseTest { when(mMockTm.getDataEnabled(mSubId)).thenReturn(false); setupDefaultSignal(); updateDataConnectionState(TelephonyManager.DATA_CONNECTED, 0); - setConnectivity(NetworkCapabilities.TRANSPORT_CELLULAR, false, false); + setConnectivityViaBroadcast(NetworkCapabilities.TRANSPORT_CELLULAR, false, false); // Verify that a SignalDrawable with a cut out is used to display data disabled. verifyLastMobileDataIndicators(true, DEFAULT_SIGNAL_STRENGTH, 0, @@ -132,7 +132,7 @@ public class NetworkControllerDataTest extends NetworkControllerBaseTest { when(mMockTm.getDataEnabled(mSubId)).thenReturn(false); setupDefaultSignal(); updateDataConnectionState(TelephonyManager.DATA_DISCONNECTED, 0); - setConnectivity(NetworkCapabilities.TRANSPORT_CELLULAR, false, false); + setConnectivityViaBroadcast(NetworkCapabilities.TRANSPORT_CELLULAR, false, false); // Verify that a SignalDrawable with a cut out is used to display data disabled. verifyLastMobileDataIndicators(true, DEFAULT_SIGNAL_STRENGTH, 0, @@ -146,7 +146,7 @@ public class NetworkControllerDataTest extends NetworkControllerBaseTest { when(mMockTm.getDataEnabled(mSubId)).thenReturn(false); setupDefaultSignal(); updateDataConnectionState(TelephonyManager.DATA_DISCONNECTED, 0); - setConnectivity(NetworkCapabilities.TRANSPORT_CELLULAR, false, false); + setConnectivityViaBroadcast(NetworkCapabilities.TRANSPORT_CELLULAR, false, false); when(mMockProvisionController.isUserSetup(anyInt())).thenReturn(false); mUserCallback.onUserSetupChanged(); TestableLooper.get(this).processAllMessages(); diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/NetworkControllerEthernetTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/NetworkControllerEthernetTest.java index d2d73470efe79..eefdeee1001eb 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/NetworkControllerEthernetTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/NetworkControllerEthernetTest.java @@ -38,7 +38,7 @@ public class NetworkControllerEthernetTest extends NetworkControllerBaseTest { } protected void setEthernetState(boolean connected, boolean validated) { - setConnectivity(NetworkCapabilities.TRANSPORT_ETHERNET, validated, connected); + setConnectivityViaBroadcast(NetworkCapabilities.TRANSPORT_ETHERNET, validated, connected); } protected void verifyLastEthernetIcon(boolean visible, int icon) { diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/NetworkControllerSignalTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/NetworkControllerSignalTest.java index 33aa417bedce0..c24336d3cbffa 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/NetworkControllerSignalTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/NetworkControllerSignalTest.java @@ -143,7 +143,7 @@ public class NetworkControllerSignalTest extends NetworkControllerBaseTest { testStrength, DEFAULT_ICON); // Verify low inet number indexing. - setConnectivity(NetworkCapabilities.TRANSPORT_CELLULAR, false, true); + setConnectivityViaBroadcast(NetworkCapabilities.TRANSPORT_CELLULAR, false, true); verifyLastMobileDataIndicators(true, testStrength, DEFAULT_ICON, false, false); } @@ -230,8 +230,8 @@ public class NetworkControllerSignalTest extends NetworkControllerBaseTest { @Test public void testNoBangWithWifi() { setupDefaultSignal(); - setConnectivity(mMobileSignalController.mTransportType, false, false); - setConnectivity(NetworkCapabilities.TRANSPORT_WIFI, true, true); + setConnectivityViaBroadcast(mMobileSignalController.mTransportType, false, false); + setConnectivityViaBroadcast(NetworkCapabilities.TRANSPORT_WIFI, true, true); verifyLastMobileDataIndicators(true, DEFAULT_LEVEL, 0); } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/NetworkControllerWifiTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/NetworkControllerWifiTest.java index 6591715526096..dff06659906e5 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/NetworkControllerWifiTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/NetworkControllerWifiTest.java @@ -18,9 +18,9 @@ import org.mockito.Mockito; import static junit.framework.Assert.assertEquals; -import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyBoolean; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @SmallTest @@ -44,9 +44,9 @@ public class NetworkControllerWifiTest extends NetworkControllerBaseTest { for (int testLevel = 0; testLevel < WifiIcons.WIFI_LEVEL_COUNT; testLevel++) { setWifiLevel(testLevel); - setConnectivity(NetworkCapabilities.TRANSPORT_WIFI, true, true); + setConnectivityViaBroadcast(NetworkCapabilities.TRANSPORT_WIFI, true, true); verifyLastWifiIcon(true, WifiIcons.WIFI_SIGNAL_STRENGTH[1][testLevel]); - setConnectivity(NetworkCapabilities.TRANSPORT_WIFI, false, true); + setConnectivityViaBroadcast(NetworkCapabilities.TRANSPORT_WIFI, false, true); verifyLastWifiIcon(true, WifiIcons.WIFI_SIGNAL_STRENGTH[0][testLevel]); } } @@ -65,10 +65,10 @@ public class NetworkControllerWifiTest extends NetworkControllerBaseTest { for (int testLevel = 0; testLevel < WifiIcons.WIFI_LEVEL_COUNT; testLevel++) { setWifiLevel(testLevel); - setConnectivity(NetworkCapabilities.TRANSPORT_WIFI, true, true); + setConnectivityViaBroadcast(NetworkCapabilities.TRANSPORT_WIFI, true, true); verifyLastQsWifiIcon(true, true, WifiIcons.QS_WIFI_SIGNAL_STRENGTH[1][testLevel], testSsid); - setConnectivity(NetworkCapabilities.TRANSPORT_WIFI, false, true); + setConnectivityViaBroadcast(NetworkCapabilities.TRANSPORT_WIFI, false, true); verifyLastQsWifiIcon(true, true, WifiIcons.QS_WIFI_SIGNAL_STRENGTH[0][testLevel], testSsid); } @@ -82,7 +82,7 @@ public class NetworkControllerWifiTest extends NetworkControllerBaseTest { setWifiEnabled(true); setWifiState(true, testSsid); setWifiLevel(testLevel); - setConnectivity(NetworkCapabilities.TRANSPORT_WIFI, true, true); + setConnectivityViaBroadcast(NetworkCapabilities.TRANSPORT_WIFI, true, true); verifyLastQsWifiIcon(true, true, WifiIcons.QS_WIFI_SIGNAL_STRENGTH[1][testLevel], testSsid); @@ -107,19 +107,68 @@ public class NetworkControllerWifiTest extends NetworkControllerBaseTest { setWifiEnabled(true); setWifiState(true, testSsid); setWifiLevel(testLevel); - setConnectivity(NetworkCapabilities.TRANSPORT_WIFI, true, true); + setConnectivityViaBroadcast(NetworkCapabilities.TRANSPORT_WIFI, true, true); verifyLastWifiIcon(true, WifiIcons.WIFI_SIGNAL_STRENGTH[1][testLevel]); setupDefaultSignal(); setGsmRoaming(true); // Still be on wifi though. - setConnectivity(NetworkCapabilities.TRANSPORT_WIFI, true, true); - setConnectivity(NetworkCapabilities.TRANSPORT_CELLULAR, false, false); + setConnectivityViaBroadcast(NetworkCapabilities.TRANSPORT_WIFI, true, true); + setConnectivityViaBroadcast(NetworkCapabilities.TRANSPORT_CELLULAR, false, false); verifyLastMobileDataIndicators(true, DEFAULT_LEVEL, 0, true); } + @Test + public void testWifiIconInvalidatedViaCallback() { + // Setup normal connection + String testSsid = "Test SSID"; + int testLevel = 2; + setWifiEnabled(true); + setWifiState(true, testSsid); + setWifiLevel(testLevel); + setConnectivityViaBroadcast(NetworkCapabilities.TRANSPORT_WIFI, true, true); + verifyLastWifiIcon(true, WifiIcons.WIFI_SIGNAL_STRENGTH[1][testLevel]); + + setConnectivityViaCallback(NetworkCapabilities.TRANSPORT_WIFI, false, true); + verifyLastWifiIcon(true, WifiIcons.WIFI_SIGNAL_STRENGTH[0][testLevel]); + } + + @Test + public void testWifiIconDisconnectedViaCallback(){ + // Setup normal connection + String testSsid = "Test SSID"; + int testLevel = 2; + setWifiEnabled(true); + setWifiState(true, testSsid); + setWifiLevel(testLevel); + setConnectivityViaBroadcast(NetworkCapabilities.TRANSPORT_WIFI, true, true); + verifyLastWifiIcon(true, WifiIcons.WIFI_SIGNAL_STRENGTH[1][testLevel]); + + setWifiState(false, testSsid); + setConnectivityViaCallback(NetworkCapabilities.TRANSPORT_WIFI, false, false); + verifyLastWifiIcon(false, WifiIcons.WIFI_NO_NETWORK); + } + + @Test + public void testVpnWithUnderlyingWifi(){ + String testSsid = "Test SSID"; + int testLevel = 2; + setWifiEnabled(true); + verifyLastWifiIcon(false, WifiIcons.WIFI_NO_NETWORK); + + setConnectivityViaCallback(NetworkCapabilities.TRANSPORT_VPN, false, true); + setConnectivityViaCallback(NetworkCapabilities.TRANSPORT_VPN, true, true); + verifyLastWifiIcon(false, WifiIcons.WIFI_NO_NETWORK); + + // Mock calling setUnderlyingNetworks. + setWifiState(true, testSsid); + setWifiLevel(testLevel); + setConnectivityViaCallback(NetworkCapabilities.TRANSPORT_WIFI, true, true); + verifyLastWifiIcon(true, WifiIcons.WIFI_SIGNAL_STRENGTH[1][testLevel]); + } + protected void setWifiActivity(int activity) { // TODO: Not this, because this variable probably isn't sticking around. mNetworkController.mWifiSignalController.setActivity(activity); @@ -143,10 +192,10 @@ public class NetworkControllerWifiTest extends NetworkControllerBaseTest { protected void setWifiState(boolean connected, String ssid) { Intent i = new Intent(WifiManager.NETWORK_STATE_CHANGED_ACTION); - NetworkInfo networkInfo = Mockito.mock(NetworkInfo.class); + NetworkInfo networkInfo = mock(NetworkInfo.class); when(networkInfo.isConnected()).thenReturn(connected); - WifiInfo wifiInfo = Mockito.mock(WifiInfo.class); + WifiInfo wifiInfo = mock(WifiInfo.class); when(wifiInfo.getSSID()).thenReturn(ssid); when(mMockWm.getConnectionInfo()).thenReturn(wifiInfo);