From e801309908e26e60496a787e4a19cc906830dcdb Mon Sep 17 00:00:00 2001 From: Sundeep Ghuman Date: Wed, 21 Jun 2017 22:35:30 -0700 Subject: [PATCH] Clear AccessPoints in WifiTracker when Wifi is Off. When Wifi is not Enabled, we should clear existing access points. This avoids showing stale data when wifi is turned back on. Since this method can now invoke onAccessPointsChanged callbacks, add extra gating for mStaleScanResults. Improve test flakiness by waiting for Work and Main handlers to finish processing messages before continuing. Bug: b/37832771 Test: runtest --path frameworks/base/packages/SettingsLib/tests/integ/src/com/android/settingslib/wifi/WifiTrackerTest.java Change-Id: I34faabec01a7188debe3c3b2fe6a0e02f49b320f --- .../android/settingslib/wifi/WifiTracker.java | 31 +++++++-- .../settingslib/wifi/WifiTrackerTest.java | 69 +++++++++++++------ 2 files changed, 75 insertions(+), 25 deletions(-) diff --git a/packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java b/packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java index 9083d90b90854..85a453d24e6fc 100644 --- a/packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java +++ b/packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java @@ -91,7 +91,7 @@ public class WifiTracker { private final boolean mIncludeScans; private final boolean mIncludePasspoints; @VisibleForTesting final MainHandler mMainHandler; - private final WorkHandler mWorkHandler; + @VisibleForTesting final WorkHandler mWorkHandler; private WifiTrackerNetworkCallback mNetworkCallback; @@ -653,6 +653,7 @@ public class WifiTracker { /* sticky broadcasts can call this when wifi is disabled */ if (!mWifiManager.isWifiEnabled()) { mMainHandler.sendEmptyMessage(MainHandler.MSG_PAUSE_SCANNING); + clearAccessPointsAndConditionallyUpdate(); return; } @@ -696,6 +697,17 @@ public class WifiTracker { } } + private void clearAccessPointsAndConditionallyUpdate() { + synchronized (mLock) { + if (!mInternalAccessPoints.isEmpty()) { + mInternalAccessPoints.clear(); + if (!mMainHandler.hasMessages(MainHandler.MSG_ACCESS_POINT_CHANGED)) { + mMainHandler.sendEmptyMessage(MainHandler.MSG_ACCESS_POINT_CHANGED); + } + } + } + } + /** * Update all the internal access points rankingScores, badge and metering. * @@ -720,6 +732,9 @@ public class WifiTracker { private void updateWifiState(int state) { mWorkHandler.obtainMessage(WorkHandler.MSG_UPDATE_WIFI_STATE, state, 0).sendToTarget(); + if (!mWifiManager.isWifiEnabled()) { + clearAccessPointsAndConditionallyUpdate(); + } } public static List getCurrentAccessPoints(Context context, boolean includeSaved, @@ -803,8 +818,15 @@ public class WifiTracker { mListener.onWifiStateChanged(msg.arg1); break; case MSG_ACCESS_POINT_CHANGED: - copyAndNotifyListeners(true /*notifyListeners*/); - mListener.onAccessPointsChanged(); + // Only notify listeners of changes if we have fresh scan results, otherwise the + // UI will be updated with stale results. We want to copy the APs regardless, + // for instances where forceUpdate was invoked by the caller. + if (mStaleScanResults) { + copyAndNotifyListeners(false /*notifyListeners*/); + } else { + copyAndNotifyListeners(true /*notifyListeners*/); + mListener.onAccessPointsChanged(); + } break; case MSG_RESUME_SCANNING: if (mScanner != null) { @@ -828,7 +850,8 @@ public class WifiTracker { } } - private final class WorkHandler extends Handler { + @VisibleForTesting + final class WorkHandler extends Handler { private static final int MSG_UPDATE_ACCESS_POINTS = 0; private static final int MSG_UPDATE_NETWORK_INFO = 1; private static final int MSG_RESUME = 2; 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 340ef016d74d0..d2733e5e53a7c 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 @@ -256,6 +256,7 @@ public class WifiTrackerTest { } sendScanResultsAndProcess(tracker); + waitForHandlersToProcessCurrentlyEnqueuedMessages(tracker); return tracker; } @@ -341,6 +342,23 @@ public class WifiTrackerTest { return createTrackerWithImmediateBroadcastsAndInjectInitialScanResults(intent); } + private void waitForHandlersToProcessCurrentlyEnqueuedMessages(WifiTracker tracker) + throws InterruptedException { + CountDownLatch workerLatch = new CountDownLatch(1); + tracker.mWorkHandler.post(() -> { + workerLatch.countDown(); + }); + assertTrue("Latch timed out while waiting for WorkerHandler", + workerLatch.await(LATCH_TIMEOUT, TimeUnit.MILLISECONDS)); + + CountDownLatch mainLatch = new CountDownLatch(1); + tracker.mMainHandler.post(() -> { + mainLatch.countDown(); + }); + assertTrue("Latch timed out while waiting for MainHandler", + mainLatch.await(LATCH_TIMEOUT, TimeUnit.MILLISECONDS)); + } + @Test public void testAccessPointListenerSetWhenLookingUpUsingScanResults() { ScanResult scanResult = new ScanResult(); @@ -426,7 +444,7 @@ public class WifiTrackerTest { @Test public void startTrackingShouldSetConnectedAccessPointAsActive() throws InterruptedException { - WifiTracker tracker = createTrackerWithScanResultsAndAccessPoint1Connected(); + WifiTracker tracker = createTrackerWithScanResultsAndAccessPoint1Connected(); List aps = tracker.getAccessPoints(); @@ -460,16 +478,18 @@ public class WifiTrackerTest { startTracking(tracker); sendScanResultsAndProcess(tracker); - updateScoresAndWaitForAccessPointsChangedCallback(); + updateScoresAndWaitForAccessPointsChangedCallback(tracker); } - private void updateScoresAndWaitForAccessPointsChangedCallback() throws InterruptedException { + private void updateScoresAndWaitForAccessPointsChangedCallback(WifiTracker tracker) + throws InterruptedException { // Updating scores can happen together or one after the other, so the latch countdown is set // to 2. - mAccessPointsChangedLatch = new CountDownLatch(2); + mAccessPointsChangedLatch = new CountDownLatch(1); updateScores(); - assertTrue("onAccessPointChanged was not called twice", + assertTrue("onAccessPointChanged was not called after updating scores", mAccessPointsChangedLatch.await(LATCH_TIMEOUT, TimeUnit.MILLISECONDS)); + waitForHandlersToProcessCurrentlyEnqueuedMessages(tracker); } @Test @@ -480,7 +500,7 @@ public class WifiTrackerTest { assertEquals(aps.get(0).getSsidStr(), SSID_1); assertEquals(aps.get(1).getSsidStr(), SSID_2); - updateScoresAndWaitForAccessPointsChangedCallback(); + updateScoresAndWaitForAccessPointsChangedCallback(tracker); aps = tracker.getAccessPoints(); assertTrue(aps.size() == 2); @@ -502,7 +522,7 @@ public class WifiTrackerTest { assertEquals(aps.get(0).getSsidStr(), SSID_1); assertEquals(aps.get(1).getSsidStr(), SSID_2); - updateScoresAndWaitForAccessPointsChangedCallback(); + updateScoresAndWaitForAccessPointsChangedCallback(tracker); aps = tracker.getAccessPoints(); assertTrue(aps.size() == 2); @@ -514,7 +534,7 @@ public class WifiTrackerTest { public void scoreCacheUpdateScoresShouldInsertSpeedIntoAccessPoint() throws InterruptedException { WifiTracker tracker = createTrackerWithImmediateBroadcastsAndInjectInitialScanResults(); - updateScoresAndWaitForAccessPointsChangedCallback(); + updateScoresAndWaitForAccessPointsChangedCallback(tracker); List aps = tracker.getAccessPoints(); @@ -531,7 +551,7 @@ public class WifiTrackerTest { public void scoreCacheUpdateMeteredShouldUpdateAccessPointMetering() throws InterruptedException { WifiTracker tracker = createTrackerWithImmediateBroadcastsAndInjectInitialScanResults(); - updateScoresAndWaitForAccessPointsChangedCallback(); + updateScoresAndWaitForAccessPointsChangedCallback(tracker); List aps = tracker.getAccessPoints(); @@ -553,7 +573,7 @@ public class WifiTrackerTest { 0 /* disabled */); WifiTracker tracker = createTrackerWithImmediateBroadcastsAndInjectInitialScanResults(); - updateScoresAndWaitForAccessPointsChangedCallback(); + updateScoresAndWaitForAccessPointsChangedCallback(tracker); List aps = tracker.getAccessPoints(); @@ -754,13 +774,10 @@ public class WifiTrackerTest { throws Exception { WifiTracker tracker = createMockedWifiTracker(); startTracking(tracker); - tracker.stopTracking(); + waitForHandlersToProcessCurrentlyEnqueuedMessages(tracker); - CountDownLatch latch1 = new CountDownLatch(1); - tracker.mMainHandler.post(() -> { - latch1.countDown(); - }); - assertTrue("Latch 1 timed out", latch1.await(LATCH_TIMEOUT, TimeUnit.MILLISECONDS)); + tracker.stopTracking(); + waitForHandlersToProcessCurrentlyEnqueuedMessages(tracker); startTracking(tracker); @@ -770,14 +787,24 @@ public class WifiTrackerTest { tracker.mReceiver.onReceive( mContext, new Intent(WifiManager.LINK_CONFIGURATION_CHANGED_ACTION)); - CountDownLatch latch2 = new CountDownLatch(1); - tracker.mMainHandler.post(() -> { - latch2.countDown(); - }); - assertTrue("Latch 2 timed out", latch2.await(LATCH_TIMEOUT, TimeUnit.MILLISECONDS)); + waitForHandlersToProcessCurrentlyEnqueuedMessages(tracker); verify(mockWifiListener, never()).onAccessPointsChanged(); sendScanResultsAndProcess(tracker); // verifies onAccessPointsChanged is invoked } + + @Test + public void disablingWifiShouldClearExistingAccessPoints() throws Exception { + WifiTracker tracker = createTrackerWithScanResultsAndAccessPoint1Connected(); + + when(mockWifiManager.isWifiEnabled()).thenReturn(false); + mAccessPointsChangedLatch = new CountDownLatch(1); + tracker.mReceiver.onReceive(mContext, new Intent(WifiManager.WIFI_STATE_CHANGED_ACTION)); + + mAccessPointsChangedLatch.await(LATCH_TIMEOUT, TimeUnit.MILLISECONDS); + waitForHandlersToProcessCurrentlyEnqueuedMessages(tracker); + + assertThat(tracker.getAccessPoints()).isEmpty(); + } }