From 85bbc6e0a057c1953e8dfafbd37bea46315657c8 Mon Sep 17 00:00:00 2001 From: Quang Luong Date: Thu, 28 Mar 2019 15:35:22 -0700 Subject: [PATCH] Increase ScanResult cache eviction timeout for failed/aborted scans Currently ScanResults are evicted from WifiTracker's cache when they are more than 15 seconds old. Since scans are conducted every 10 seconds, one aborted scan will cause no fresh scans to be seen until 20 seconds later, resulting in all scans to be evicted and the wifi-picker completely emptying. This change doubles the eviction timeout to 30 seconds if the last scan was aborted, avoiding this case. Bug: 113132408 Test: atest WifiTrackerTest Change-Id: I2e3aad3a1470f52c3d87e657262a2c4de1bd2418 --- .../android/settingslib/wifi/WifiTracker.java | 20 +++++++-- .../settingslib/wifi/WifiTrackerTest.java | 41 +++++++++++++++++-- 2 files changed, 54 insertions(+), 7 deletions(-) diff --git a/packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java b/packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java index fdc0fd33e6d73..5f2bc4e1d4904 100644 --- a/packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java +++ b/packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java @@ -84,7 +84,7 @@ public class WifiTracker implements LifecycleObserver, OnStart, OnStop, OnDestro private static final long DEFAULT_MAX_CACHED_SCORE_AGE_MILLIS = 20 * DateUtils.MINUTE_IN_MILLIS; /** Maximum age of scan results to hold onto while actively scanning. **/ - private static final long MAX_SCAN_RESULT_AGE_MILLIS = 15000; + @VisibleForTesting static final long MAX_SCAN_RESULT_AGE_MILLIS = 15000; private static final String TAG = "WifiTracker"; private static final boolean DBG() { @@ -142,6 +142,13 @@ public class WifiTracker implements LifecycleObserver, OnStart, OnStop, OnDestro */ private boolean mStaleScanResults = true; + /** + * Tracks whether the latest SCAN_RESULTS_AVAILABLE_ACTION contained new scans. If not, then + * we treat the last scan as an aborted scan and increase the eviction timeout window to avoid + * completely flushing the AP list before the next successful scan completes. + */ + private boolean mLastScanSucceeded = true; + // Does not need to be locked as it only updated on the worker thread, with the exception of // during onStart, which occurs before the receiver is registered on the work handler. private final HashMap mScanResultCache = new HashMap<>(); @@ -478,17 +485,22 @@ public class WifiTracker implements LifecycleObserver, OnStart, OnStop, OnDestro } /** - * Remove old scan results from the cache. + * Remove old scan results from the cache. If {@link #mLastScanSucceeded} is false, then + * increase the timeout window to avoid completely flushing the AP list before the next + * successful scan completes. * *

Should only ever be invoked from {@link #updateScanResultCache(List)} when * {@link #mStaleScanResults} is false. */ private void evictOldScans() { + long evictionTimeoutMillis = mLastScanSucceeded ? MAX_SCAN_RESULT_AGE_MILLIS + : MAX_SCAN_RESULT_AGE_MILLIS * 2; + long nowMs = SystemClock.elapsedRealtime(); for (Iterator iter = mScanResultCache.values().iterator(); iter.hasNext(); ) { ScanResult result = iter.next(); // result timestamp is in microseconds - if (nowMs - result.timestamp / 1000 > MAX_SCAN_RESULT_AGE_MILLIS) { + if (nowMs - result.timestamp / 1000 > evictionTimeoutMillis) { iter.remove(); } } @@ -840,6 +852,8 @@ public class WifiTracker implements LifecycleObserver, OnStart, OnStop, OnDestro WifiManager.WIFI_STATE_UNKNOWN)); } else if (WifiManager.SCAN_RESULTS_AVAILABLE_ACTION.equals(action)) { mStaleScanResults = false; + mLastScanSucceeded = + intent.getBooleanExtra(WifiManager.EXTRA_RESULTS_UPDATED, true); fetchScansAndConfigsAndUpdateAccessPoints(); } else if (WifiManager.CONFIGURED_NETWORKS_CHANGED_ACTION.equals(action) 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 edf414ddf3238..683ec8bb5a6c7 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 @@ -270,7 +270,7 @@ public class WifiTrackerTest { SystemClock.elapsedRealtime() * 1000 /* microsecond timestamp */); } - private static ScanResult buildStaleScanResult() { + private static ScanResult buildScanResultWithTimestamp(long timestampMillis) { return new ScanResult( WifiSsid.createFromAsciiEncoded(SSID_3), BSSID_3, @@ -280,7 +280,7 @@ public class WifiTrackerTest { "", // capabilities RSSI_3, 0, // frequency - 0 /* microsecond timestamp */); + timestampMillis * 1000 /* microsecond timestamp */); } private static WifiConfiguration buildPasspointConfiguration(String fqdn, String friendlyName) { @@ -379,6 +379,12 @@ public class WifiTrackerTest { tracker.mReceiver.onReceive(mContext, i); } + private void sendFailedScanResults(WifiTracker tracker) throws InterruptedException { + Intent i = new Intent(WifiManager.SCAN_RESULTS_AVAILABLE_ACTION); + i.putExtra(WifiManager.EXTRA_RESULTS_UPDATED, false); + tracker.mReceiver.onReceive(mContext, i); + } + private void sendUpdatedScores() throws InterruptedException { Bundle attr1 = new Bundle(); attr1.putParcelable(ScoredNetwork.ATTRIBUTES_KEY_BADGING_CURVE, mockBadgeCurve1); @@ -982,8 +988,8 @@ public class WifiTrackerTest { @Test public void onStart_updateScanResults_evictOldScanResult() { - when(mockWifiManager.getScanResults()).thenReturn( - Arrays.asList(buildScanResult1(), buildScanResult2(), buildStaleScanResult())); + when(mockWifiManager.getScanResults()).thenReturn(Arrays.asList( + buildScanResult1(), buildScanResult2(), buildScanResultWithTimestamp(0))); WifiTracker tracker = createMockedWifiTracker(); tracker.forceUpdate(); @@ -994,6 +1000,33 @@ public class WifiTrackerTest { assertThat(tracker.getAccessPoints().get(1).getBssid()).isEqualTo(BSSID_2); } + /** + * Verifies that a failed scan reported on SCAN_RESULTS_AVAILABLE_ACTION should increase the + * ScanResult eviction timeout to twice the default. + */ + @Test + public void failedScan_increasesEvictionTimeout() throws InterruptedException { + when(mockWifiManager.getScanResults()).thenReturn(Arrays.asList( + buildScanResult1(), buildScanResult2(), buildScanResultWithTimestamp( + SystemClock.elapsedRealtime() - WifiTracker.MAX_SCAN_RESULT_AGE_MILLIS))); + WifiTracker tracker = createMockedWifiTracker(); + + sendFailedScanResults(tracker); + + // Failed scan increases timeout window to include the stale scan + assertThat(tracker.getAccessPoints()).hasSize(3); + assertThat(tracker.getAccessPoints().get(0).getBssid()).isEqualTo(BSSID_1); + assertThat(tracker.getAccessPoints().get(1).getBssid()).isEqualTo(BSSID_2); + assertThat(tracker.getAccessPoints().get(2).getBssid()).isEqualTo(BSSID_3); + + sendScanResults(tracker); + + // Successful scan resets the timeout window to remove the stale scan + assertThat(tracker.getAccessPoints()).hasSize(2); + assertThat(tracker.getAccessPoints().get(0).getBssid()).isEqualTo(BSSID_1); + assertThat(tracker.getAccessPoints().get(1).getBssid()).isEqualTo(BSSID_2); + } + /** * Verifies that updatePasspointAccessPoints will only return AccessPoints whose * isPasspoint() evaluates as true.