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

2 files changed, 70 insertions, 31 deletions

diff --git a/packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java b/packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java
index 085e1123d67e..8f80527036ec 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
      *
      * <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 @@ 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 7fb4dc544d28..517db78d5dd1 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<NetworkKey> 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();
@@ -762,6 +764,41 @@ public class WifiTrackerTest {
     }
 
     @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 @@ 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