diff --git a/packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java b/packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java index 085e1123d67e1..8f80527036ec7 100644 --- a/packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java +++ b/packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java @@ -176,33 +176,34 @@ public class WifiTracker implements LifecycleObserver, OnStart, OnStop, OnDestro @Deprecated public WifiTracker(Context context, WifiListener wifiListener, boolean includeSaved, boolean includeScans) { - this(context, new WifiListenerExecutor(wifiListener), + this(context, wifiListener, context.getSystemService(WifiManager.class), context.getSystemService(ConnectivityManager.class), context.getSystemService(NetworkScoreManager.class), newIntentFilter()); } - // TODO(Sghuman): Clean up includeSaved and includeScans from all constructors and linked + // TODO(sghuman): Clean up includeSaved and includeScans from all constructors and linked // calling apps once IC window is complete public WifiTracker(Context context, WifiListener wifiListener, @NonNull Lifecycle lifecycle, boolean includeSaved, boolean includeScans) { - this(context, new WifiListenerExecutor(wifiListener), + this(context, wifiListener, context.getSystemService(WifiManager.class), context.getSystemService(ConnectivityManager.class), context.getSystemService(NetworkScoreManager.class), newIntentFilter()); + lifecycle.addObserver(this); } @VisibleForTesting - WifiTracker(Context context, WifiListenerExecutor wifiListenerExecutor, + WifiTracker(Context context, WifiListener wifiListener, WifiManager wifiManager, ConnectivityManager connectivityManager, NetworkScoreManager networkScoreManager, IntentFilter filter) { mContext = context; mWifiManager = wifiManager; - mListener = wifiListenerExecutor; + mListener = new WifiListenerExecutor(wifiListener); mConnectivityManager = connectivityManager; // check if verbose logging developer option has been turned on or off @@ -853,8 +854,7 @@ public class WifiTracker implements LifecycleObserver, OnStart, OnStop, OnDestro * *

Also logs all callbacks invocations when verbose logging is enabled. */ - @VisibleForTesting - public static class WifiListenerExecutor implements WifiListener { + @VisibleForTesting class WifiListenerExecutor implements WifiListener { private final WifiListener mDelegatee; @@ -864,27 +864,29 @@ public class WifiTracker implements LifecycleObserver, OnStart, OnStop, OnDestro @Override public void onWifiStateChanged(int state) { - if (isVerboseLoggingEnabled()) { - Log.i(TAG, - String.format("Invoking onWifiStateChanged callback with state %d", state)); - } - ThreadUtils.postOnMainThread(() -> mDelegatee.onWifiStateChanged(state)); + runAndLog(() -> mDelegatee.onWifiStateChanged(state), + String.format("Invoking onWifiStateChanged callback with state %d", state)); } @Override public void onConnectedChanged() { - if (isVerboseLoggingEnabled()) { - Log.i(TAG, "Invoking onConnectedChanged callback"); - } - ThreadUtils.postOnMainThread(() -> mDelegatee.onConnectedChanged()); + runAndLog(mDelegatee::onConnectedChanged, "Invoking onConnectedChanged callback"); } @Override public void onAccessPointsChanged() { - if (isVerboseLoggingEnabled()) { - Log.i(TAG, "Invoking onAccessPointsChanged callback"); - } - ThreadUtils.postOnMainThread(() -> mDelegatee.onAccessPointsChanged()); + runAndLog(mDelegatee::onAccessPointsChanged, "Invoking onAccessPointsChanged callback"); + } + + private void runAndLog(Runnable r, String verboseLog) { + ThreadUtils.postOnMainThread(() -> { + if (mRegistered) { + if (isVerboseLoggingEnabled()) { + Log.i(TAG, verboseLog); + } + r.run(); + } + }); } } diff --git a/packages/SettingsLib/tests/integ/src/com/android/settingslib/wifi/WifiTrackerTest.java b/packages/SettingsLib/tests/integ/src/com/android/settingslib/wifi/WifiTrackerTest.java index 7fb4dc544d28e..517db78d5dd10 100644 --- a/packages/SettingsLib/tests/integ/src/com/android/settingslib/wifi/WifiTrackerTest.java +++ b/packages/SettingsLib/tests/integ/src/com/android/settingslib/wifi/WifiTrackerTest.java @@ -58,6 +58,8 @@ import android.support.test.InstrumentationRegistry; import android.support.test.filters.SmallTest; import android.support.test.runner.AndroidJUnit4; +import com.android.settingslib.utils.ThreadUtils; + import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -135,7 +137,7 @@ public class WifiTrackerTest { @Mock private RssiCurve mockBadgeCurve1; @Mock private RssiCurve mockBadgeCurve2; @Mock private WifiManager mockWifiManager; - @Mock private WifiTracker.WifiListenerExecutor mockWifiListenerExecutor; + @Mock private WifiTracker.WifiListener mockWifiListener; private final List mRequestedKeys = new ArrayList<>(); @@ -205,7 +207,7 @@ public class WifiTrackerTest { mAccessPointsChangedLatch.countDown(); } return null; - }).when(mockWifiListenerExecutor).onAccessPointsChanged(); + }).when(mockWifiListener).onAccessPointsChanged(); // Turn on Scoring UI features mOriginalScoringUiSettingValue = Settings.Global.getInt( @@ -271,7 +273,7 @@ public class WifiTrackerTest { private WifiTracker createMockedWifiTracker() { final WifiTracker wifiTracker = new WifiTracker( mContext, - mockWifiListenerExecutor, + mockWifiListener, mockWifiManager, mockConnectivityManager, mockNetworkScoreManager, @@ -690,7 +692,7 @@ public class WifiTrackerTest { verify(mockConnectivityManager).getNetworkInfo(any(Network.class)); // mStaleAccessPoints is true - verify(mockWifiListenerExecutor, never()).onAccessPointsChanged(); + verify(mockWifiListener, never()).onAccessPointsChanged(); assertThat(tracker.getAccessPoints().size()).isEqualTo(2); assertThat(tracker.getAccessPoints().get(0).isActive()).isTrue(); } @@ -719,7 +721,7 @@ public class WifiTrackerTest { verify(mockConnectivityManager).getNetworkInfo(any(Network.class)); // mStaleAccessPoints is true - verify(mockWifiListenerExecutor, never()).onAccessPointsChanged(); + verify(mockWifiListener, never()).onAccessPointsChanged(); assertThat(tracker.getAccessPoints()).hasSize(1); assertThat(tracker.getAccessPoints().get(0).isActive()).isTrue(); @@ -761,6 +763,41 @@ public class WifiTrackerTest { assertThat(executed.get()).isFalse(); } + @Test + public void stopTrackingShouldPreventCallbacksFromOngoingWork() throws Exception { + WifiTracker tracker = createMockedWifiTracker(); + startTracking(tracker); + + final CountDownLatch ready = new CountDownLatch(1); + final CountDownLatch latch = new CountDownLatch(1); + final CountDownLatch lock = new CountDownLatch(1); + tracker.mWorkHandler.post(() -> { + try { + ready.countDown(); + lock.await(); + + tracker.mReceiver.onReceive( + mContext, new Intent(WifiManager.WIFI_STATE_CHANGED_ACTION)); + + latch.countDown(); + } catch (InterruptedException e) { + fail("Interrupted Exception while awaiting lock release: " + e); + } + }); + + ready.await(); // Make sure we have entered the first message handler + tracker.onStop(); + lock.countDown(); + assertTrue("Latch timed out", latch.await(LATCH_TIMEOUT, TimeUnit.MILLISECONDS)); + + // Wait for main thread + final CountDownLatch latch2 = new CountDownLatch(1); + ThreadUtils.postOnMainThread(latch2::countDown); + latch2.await(); + + verify(mockWifiListener, never()).onWifiStateChanged(anyInt()); + } + @Test public void stopTrackingShouldSetStaleBitWhichPreventsCallbacksUntilNextScanResult() throws Exception { @@ -778,7 +815,7 @@ public class WifiTrackerTest { mContext, new Intent(WifiManager.LINK_CONFIGURATION_CHANGED_ACTION)); - verify(mockWifiListenerExecutor, never()).onAccessPointsChanged(); + verify(mockWifiListener, never()).onAccessPointsChanged(); sendScanResults(tracker); // verifies onAccessPointsChanged is invoked } @@ -795,7 +832,7 @@ public class WifiTrackerTest { tracker.mReceiver.onReceive( mContext, new Intent(WifiManager.LINK_CONFIGURATION_CHANGED_ACTION)); - verify(mockWifiListenerExecutor, never()).onAccessPointsChanged(); + verify(mockWifiListener, never()).onAccessPointsChanged(); sendScanResults(tracker); // verifies onAccessPointsChanged is invoked } @@ -816,7 +853,7 @@ public class WifiTrackerTest { @Test public void onConnectedChangedCallback_shouldNotBeInvokedWhenNoStateChange() throws Exception { WifiTracker tracker = createTrackerWithScanResultsAndAccessPoint1Connected(); - verify(mockWifiListenerExecutor, times(1)).onConnectedChanged(); + verify(mockWifiListener, times(1)).onConnectedChanged(); NetworkInfo networkInfo = new NetworkInfo( ConnectivityManager.TYPE_WIFI, 0, "Type Wifi", "subtype"); @@ -826,13 +863,13 @@ public class WifiTrackerTest { intent.putExtra(WifiManager.EXTRA_NETWORK_INFO, networkInfo); tracker.mReceiver.onReceive(mContext, intent); - verify(mockWifiListenerExecutor, times(1)).onConnectedChanged(); + verify(mockWifiListener, times(1)).onConnectedChanged(); } @Test public void onConnectedChangedCallback_shouldBeInvokedWhenStateChanges() throws Exception { WifiTracker tracker = createTrackerWithScanResultsAndAccessPoint1Connected(); - verify(mockWifiListenerExecutor, times(1)).onConnectedChanged(); + verify(mockWifiListener, times(1)).onConnectedChanged(); NetworkInfo networkInfo = new NetworkInfo( ConnectivityManager.TYPE_WIFI, 0, "Type Wifi", "subtype"); @@ -844,7 +881,7 @@ public class WifiTrackerTest { tracker.mReceiver.onReceive(mContext, intent); assertThat(tracker.isConnected()).isFalse(); - verify(mockWifiListenerExecutor, times(2)).onConnectedChanged(); + verify(mockWifiListener, times(2)).onConnectedChanged(); } @Test