Prevent callbacks after onStop is called.
Bug: 74196862
Test: runtest --path
frameworks/base/packages/SettingsLib/tests/integ/src/com/android/settingslib/wifi/WifiTrackerTest.java
Change-Id: Idab27f7d493fdd097d55aac40750912e454addb2
diff --git a/packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java b/packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java
index 085e112..8f80527 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 @@
@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 @@
*
* <p>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 @@
@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 7fb4dc5..517db78 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.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 @@
@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<NetworkKey> mRequestedKeys = new ArrayList<>();
@@ -205,7 +207,7 @@
mAccessPointsChangedLatch.countDown();
}
return null;
- }).when(mockWifiListenerExecutor).onAccessPointsChanged();
+ }).when(mockWifiListener).onAccessPointsChanged();
// Turn on Scoring UI features
mOriginalScoringUiSettingValue = Settings.Global.getInt(
@@ -271,7 +273,7 @@
private WifiTracker createMockedWifiTracker() {
final WifiTracker wifiTracker = new WifiTracker(
mContext,
- mockWifiListenerExecutor,
+ mockWifiListener,
mockWifiManager,
mockConnectivityManager,
mockNetworkScoreManager,
@@ -690,7 +692,7 @@
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 @@
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();
@@ -762,6 +764,41 @@
}
@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 {
WifiTracker tracker = createMockedWifiTracker();
@@ -778,7 +815,7 @@
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 @@
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 @@
@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 @@
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 @@
tracker.mReceiver.onReceive(mContext, intent);
assertThat(tracker.isConnected()).isFalse();
- verify(mockWifiListenerExecutor, times(2)).onConnectedChanged();
+ verify(mockWifiListener, times(2)).onConnectedChanged();
}
@Test