From c014decc74edc3a415efb78893f597f3b800a212 Mon Sep 17 00:00:00 2001 From: Jason Monk Date: Fri, 12 Dec 2014 11:49:55 -0500 Subject: [PATCH] Don't unregister MobileSignalControllers still used Also add some more testing for this section of code that manages when MobileSignalControllers are added/removed to make sure we are all good. Bug: 18728593 Change-Id: I9902854c54d2e1deb58b38b7bd97dac1617831c0 --- .../policy/NetworkControllerImpl.java | 12 ++- packages/SystemUI/tests/AndroidManifest.xml | 1 + .../policy/NetworkControllerSignalTest.java | 91 ++++++++++++++++++- 3 files changed, 97 insertions(+), 7 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 5eebf3c4be567..b289155ada59e 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/policy/NetworkControllerImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/policy/NetworkControllerImpl.java @@ -134,7 +134,8 @@ public class NetworkControllerImpl extends BroadcastReceiver private ArrayList mSignalClusters = new ArrayList(); private ArrayList mSignalsChangedCallbacks = new ArrayList(); - private boolean mListening; + @VisibleForTesting + boolean mListening; // The current user ID. private int mCurrentUserId; @@ -474,7 +475,7 @@ public class NetworkControllerImpl extends BroadcastReceiver int subId = subscriptions.get(i).getSubscriptionId(); // If we have a copy of this controller already reuse it, otherwise make a new one. if (cachedControllers.containsKey(subId)) { - mMobileSignalControllers.put(subId, cachedControllers.get(subId)); + mMobileSignalControllers.put(subId, cachedControllers.remove(subId)); } else { MobileSignalController controller = new MobileSignalController(mContext, mConfig, mHasMobileDataFeature, mPhone, mSignalsChangedCallbacks, mSignalClusters, @@ -502,7 +503,8 @@ public class NetworkControllerImpl extends BroadcastReceiver updateAirplaneMode(true /* force */); } - private boolean hasCorrectMobileControllers(List allSubscriptions) { + @VisibleForTesting + boolean hasCorrectMobileControllers(List allSubscriptions) { if (allSubscriptions.size() != mMobileSignalControllers.size()) { return false; } @@ -992,8 +994,8 @@ public class NetworkControllerImpl extends BroadcastReceiver } // TODO: Move to its own file. - static class MobileSignalController extends SignalController { + public static class MobileSignalController extends SignalController< + MobileSignalController.MobileState, MobileSignalController.MobileIconGroup> { private final TelephonyManager mPhone; private final String mNetworkNameDefault; private final String mNetworkNameSeparator; diff --git a/packages/SystemUI/tests/AndroidManifest.xml b/packages/SystemUI/tests/AndroidManifest.xml index e52806d6a7acd..c21af24570306 100644 --- a/packages/SystemUI/tests/AndroidManifest.xml +++ b/packages/SystemUI/tests/AndroidManifest.xml @@ -18,6 +18,7 @@ package="com.android.systemui.tests"> + 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 27a4052f3941b..525dd2092d831 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 @@ -2,14 +2,21 @@ package com.android.systemui.statusbar.policy; import static org.mockito.Mockito.mock; +import java.util.ArrayList; +import java.util.List; + +import org.mockito.Mockito; + +import android.content.Intent; import android.net.ConnectivityManager; import android.telephony.ServiceState; import android.telephony.SignalStrength; +import android.telephony.SubscriptionInfo; import android.telephony.TelephonyManager; +import com.android.internal.telephony.TelephonyIntents; import com.android.systemui.R; - -import org.mockito.Mockito; +import com.android.systemui.statusbar.policy.NetworkControllerImpl.MobileSignalController; public class NetworkControllerSignalTest extends NetworkControllerBaseTest { @@ -145,6 +152,86 @@ public class NetworkControllerSignalTest extends NetworkControllerBaseTest { //verifyLastMobileDataIndicators(true, R.drawable.stat_sys_signal_null, 0 /* No Icon */); } + // Some tests of actual NetworkController code, just internals not display stuff + // TODO: Put this somewhere else, maybe in its own file. + public void testHasCorrectMobileControllers() { + int[] testSubscriptions = new int[] { 1, 5, 3 }; + int notTestSubscription = 0; + MobileSignalController mobileSignalController = Mockito.mock(MobileSignalController.class); + + mNetworkController.mMobileSignalControllers.clear(); + List subscriptions = new ArrayList<>(); + for (int i = 0; i < testSubscriptions.length; i++) { + // Force the test controllers into NetworkController. + mNetworkController.mMobileSignalControllers.put(testSubscriptions[i], + mobileSignalController); + + // Generate a list of subscriptions we will tell the NetworkController to use. + SubscriptionInfo mockSubInfo = Mockito.mock(SubscriptionInfo.class); + Mockito.when(mockSubInfo.getSubscriptionId()).thenReturn(testSubscriptions[i]); + subscriptions.add(mockSubInfo); + } + assertTrue(mNetworkController.hasCorrectMobileControllers(subscriptions)); + + // Add a subscription that the NetworkController doesn't know about. + SubscriptionInfo mockSubInfo = Mockito.mock(SubscriptionInfo.class); + Mockito.when(mockSubInfo.getSubscriptionId()).thenReturn(notTestSubscription); + subscriptions.add(mockSubInfo); + assertFalse(mNetworkController.hasCorrectMobileControllers(subscriptions)); + } + + public void testSetCurrentSubscriptions() { + // We will not add one controller to make sure it gets created. + int indexToSkipController = 0; + // We will not add one subscription to make sure it's controller gets removed. + int indexToSkipSubscription = 1; + + int[] testSubscriptions = new int[] { 1, 5, 3 }; + MobileSignalController[] mobileSignalControllers = new MobileSignalController[] { + Mockito.mock(MobileSignalController.class), + Mockito.mock(MobileSignalController.class), + Mockito.mock(MobileSignalController.class), + }; + mNetworkController.mMobileSignalControllers.clear(); + List subscriptions = new ArrayList<>(); + for (int i = 0; i < testSubscriptions.length; i++) { + if (i != indexToSkipController) { + // Force the test controllers into NetworkController. + mNetworkController.mMobileSignalControllers.put(testSubscriptions[i], + mobileSignalControllers[i]); + } + + if (i != indexToSkipSubscription) { + // Generate a list of subscriptions we will tell the NetworkController to use. + SubscriptionInfo mockSubInfo = Mockito.mock(SubscriptionInfo.class); + Mockito.when(mockSubInfo.getSubscriptionId()).thenReturn(testSubscriptions[i]); + Mockito.when(mockSubInfo.getSimSlotIndex()).thenReturn(testSubscriptions[i]); + subscriptions.add(mockSubInfo); + } + } + + // We can only test whether unregister gets called if it thinks its in a listening + // state. + mNetworkController.mListening = true; + mNetworkController.setCurrentSubscriptions(subscriptions); + + for (int i = 0; i < testSubscriptions.length; i++) { + if (i == indexToSkipController) { + // Make sure a controller was created despite us not adding one. + assertTrue(mNetworkController.mMobileSignalControllers.containsKey( + testSubscriptions[i])); + } else if (i == indexToSkipSubscription) { + // Make sure the controller that did exist was removed + assertFalse(mNetworkController.mMobileSignalControllers.containsKey( + testSubscriptions[i])); + } else { + // If a MobileSignalController is around it needs to not be unregistered. + Mockito.verify(mobileSignalControllers[i], Mockito.never()) + .unregisterListener(); + } + } + } + private void setCdma() { setIsGsm(false); updateDataConnectionState(TelephonyManager.DATA_CONNECTED,