WifiScanningService: Cache only results of full single scans

Change the single scan results caching logic inside WifiScanner:
1. Only cache scan results if they were for full scans. We've expanded
the meaning of "full scans" to include both 2g + 5g and 2g + 5g + dfs.
2. Filter out any scan results from the cache if they're over 3 minutes
old.

The motivation of this CL is to solve these problems:
1. Do not overwrite full scan results requested by an external app (ex.
Settings) with results from a partial scan
(ex. from WifiConnectivityManager).
This avoids the race condition where the settings app performs
a scan and gets a partial list of results back because it was ovewritten
by an internal partial scan request in between.
2. Scan results retrieved by apps will not contain stale scan
results. In N, wpa_supplicant evicted any scan results which were older
than 3 minutes from the cache. So, this CL restores that behavior.

Bug: 38212080
Test: Verified that both the settings and wifi wake is working properly
with the fix.
Test: Will send for full regression tests (b/62711875).
Test: New unit tests to verify that the partial scan results don't
override the cache.
Test: Modified existing unit tests to account for the change in
behavior. The broadcast (and cache) is now only updated on full scans.
Some of the tests were modified to produce full scan results, while the
other tests expectations were modified for partial scan results.

Change-Id: I4d6fb994d50995434f76218386272aa2562476fe

3 files changed, 170 insertions, 23 deletions

diff --git a/service/java/com/android/server/wifi/scanner/WifiScanningServiceImpl.java b/service/java/com/android/server/wifi/scanner/WifiScanningServiceImpl.java
index 08c9e1359..af874b9e2 100644
--- a/service/java/com/android/server/wifi/scanner/WifiScanningServiceImpl.java
+++ b/service/java/com/android/server/wifi/scanner/WifiScanningServiceImpl.java
@@ -438,6 +438,15 @@ public class WifiScanningServiceImpl extends IWifiScanner.Stub {
      * executed after transitioning back to IdleState.
      */
     class WifiSingleScanStateMachine extends StateMachine implements WifiNative.ScanEventHandler {
+        /**
+         * Maximum age of results that we return from our cache via
+         * {@link WifiScanner#getScanResults()}.
+         * This is currently set to 3 minutes to restore parity with the wpa_supplicant's scan
+         * result cache expiration policy. (See b/62253332 for details)
+         */
+        @VisibleForTesting
+        public static final int CACHED_SCAN_RESULTS_MAX_AGE_IN_MILLIS = 180 * 1000;
+
         private final DefaultState mDefaultState = new DefaultState();
         private final DriverStartedState mDriverStartedState = new DriverStartedState();
         private final IdleState  mIdleState  = new IdleState();
@@ -447,7 +456,8 @@ public class WifiScanningServiceImpl extends IWifiScanner.Stub {
         private RequestList<ScanSettings> mActiveScans = new RequestList<>();
         private RequestList<ScanSettings> mPendingScans = new RequestList<>();
 
-        private ScanResult[] mCachedScanResults = new ScanResult[0];
+        // Scan results cached from the last full single scan request.
+        private final List<ScanResult> mCachedScanResults = new ArrayList<>();
 
         WifiSingleScanStateMachine(Looper looper) {
             super("WifiSingleScanStateMachine", looper);
@@ -534,13 +544,30 @@ public class WifiScanningServiceImpl extends IWifiScanner.Stub {
                         if (DBG) localLog("ignored full scan result event");
                         return HANDLED;
                     case WifiScanner.CMD_GET_SINGLE_SCAN_RESULTS:
-                        msg.obj = new WifiScanner.ParcelableScanResults(mCachedScanResults.clone());
+                        msg.obj = new WifiScanner.ParcelableScanResults(
+                            filterCachedScanResultsByAge());
                         replySucceeded(msg);
                         return HANDLED;
                     default:
                         return NOT_HANDLED;
                 }
+            }
 
+            /**
+             * Filter out  any scan results that are older than
+             * {@link #CACHED_SCAN_RESULTS_MAX_AGE_IN_MILLIS}.
+             *
+             * @return Filtered list of scan results.
+             */
+            private ScanResult[] filterCachedScanResultsByAge() {
+                // Using ScanResult.timestamp here to ensure that we use the same fields as
+                // WificondScannerImpl for filtering stale results.
+                long currentTimeInMillis = mClock.getElapsedSinceBootMillis();
+                return mCachedScanResults.stream()
+                        .filter(scanResult
+                                -> ((currentTimeInMillis - (scanResult.timestamp / 1000))
+                                        < CACHED_SCAN_RESULTS_MAX_AGE_IN_MILLIS))
+                        .toArray(ScanResult[]::new);
             }
         }
 
@@ -553,7 +580,7 @@ public class WifiScanningServiceImpl extends IWifiScanner.Stub {
             @Override
             public void exit() {
                 // clear scan results when scan mode is not active
-                mCachedScanResults = new ScanResult[0];
+                mCachedScanResults.clear();
 
                 mWifiMetrics.incrementScanReturnEntry(
                         WifiMetricsProto.WifiLog.SCAN_FAILURE_INTERRUPTED,
@@ -879,13 +906,15 @@ public class WifiScanningServiceImpl extends IWifiScanner.Stub {
                 entry.reportEvent(WifiScanner.CMD_SCAN_RESULT, 0, parcelableAllResults);
             }
 
-            // Cache the results here so that apps can retrieve them.
-            mCachedScanResults = results.getResults();
-            sendScanResultBroadcast(true);
+            if (results.isAllChannelsScanned()) {
+                mCachedScanResults.clear();
+                mCachedScanResults.addAll(Arrays.asList(results.getResults()));
+                sendScanResultBroadcast(true);
+            }
         }
 
         List<ScanResult> getCachedScanResultsAsList() {
-            return Arrays.asList(mCachedScanResults);
+            return mCachedScanResults;
         }
     }
 
diff --git a/service/java/com/android/server/wifi/scanner/WificondScannerImpl.java b/service/java/com/android/server/wifi/scanner/WificondScannerImpl.java
index 4aa9bc972..fd7fddb73 100644
--- a/service/java/com/android/server/wifi/scanner/WificondScannerImpl.java
+++ b/service/java/com/android/server/wifi/scanner/WificondScannerImpl.java
@@ -579,6 +579,17 @@ public class WificondScannerImpl extends WifiScannerImpl implements Handler.Call
         }
     }
 
+    /**
+     * Check if the provided channel collection contains all the channels.
+     */
+    private static boolean isAllChannelsScanned(ChannelCollection channelCollection) {
+        // TODO(b/62253332): Get rid of this hack.
+        // We're treating 2g + 5g and 2g + 5g + dfs as all channels scanned to work around
+        // the lack of a proper cache.
+        return (channelCollection.containsBand(WifiScanner.WIFI_BAND_24_GHZ)
+                && channelCollection.containsBand(WifiScanner.WIFI_BAND_5_GHZ));
+    }
+
     private void pollLatestScanData() {
         synchronized (mSettingsLock) {
             if (mLastScanSettings == null) {
@@ -665,7 +676,7 @@ public class WificondScannerImpl extends WifiScannerImpl implements Handler.Call
                 }
                 Collections.sort(singleScanResults, SCAN_RESULT_SORT_COMPARATOR);
                 mLatestSingleScanResult = new WifiScanner.ScanData(mLastScanSettings.scanId, 0, 0,
-                        mLastScanSettings.singleScanFreqs.isAllChannels(),
+                        isAllChannelsScanned(mLastScanSettings.singleScanFreqs),
                         singleScanResults.toArray(new ScanResult[singleScanResults.size()]));
                 mLastScanSettings.singleScanEventHandler
                         .onScanStatus(WifiNative.WIFI_SCAN_RESULTS_AVAILABLE);
diff --git a/tests/wifitests/src/com/android/server/wifi/scanner/WifiScanningServiceTest.java b/tests/wifitests/src/com/android/server/wifi/scanner/WifiScanningServiceTest.java
index e7c5fa962..85b19fec8 100644
--- a/tests/wifitests/src/com/android/server/wifi/scanner/WifiScanningServiceTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/scanner/WifiScanningServiceTest.java
@@ -17,6 +17,8 @@
 package com.android.server.wifi.scanner;
 
 import static com.android.server.wifi.ScanTestUtil.*;
+import static com.android.server.wifi.scanner.WifiScanningServiceImpl.WifiSingleScanStateMachine
+        .CACHED_SCAN_RESULTS_MAX_AGE_IN_MILLIS;
 
 import static org.junit.Assert.*;
 import static org.mockito.Mockito.*;
@@ -123,6 +125,7 @@ public class WifiScanningServiceTest {
         mWifiAsyncChannel.setWifiLog(mLog);
         when(mFrameworkFacade.makeWifiAsyncChannel(anyString())).thenReturn(mWifiAsyncChannel);
         when(mWifiInjector.getFrameworkFacade()).thenReturn(mFrameworkFacade);
+        when(mWifiInjector.getClock()).thenReturn(mClock);
         mWifiScanningServiceImpl = new WifiScanningServiceImpl(mContext, mLooper.getLooper(),
                 mWifiScannerImplFactory, mBatteryStats, mWifiInjector);
     }
@@ -527,7 +530,11 @@ public class WifiScanningServiceTest {
         mLooper.dispatchAll();
         verifyScanResultsReceived(order, handler, requestId, results.getScanData());
         verifySingleScanCompletedReceived(order, handler, requestId);
-        verify(mContext).sendBroadcastAsUser(any(Intent.class), eq(UserHandle.ALL));
+        if (results.getScanData().isAllChannelsScanned()) {
+            verify(mContext).sendBroadcastAsUser(any(Intent.class), eq(UserHandle.ALL));
+        } else {
+            verify(mContext, never()).sendBroadcastAsUser(any(Intent.class), eq(UserHandle.ALL));
+        }
         verifyNoMoreInteractions(handler);
         verify(mBatteryStats).noteWifiScanStoppedFromSource(eq(workSource));
         assertDumpContainsRequestLog("addSingleScanRequest", requestId);
@@ -777,13 +784,13 @@ public class WifiScanningServiceTest {
         WifiScanner.ScanSettings requestSettings1 = createRequest(channelsToSpec(2400), 0,
                 0, 20, WifiScanner.REPORT_EVENT_AFTER_EACH_SCAN);
         int requestId1 = 12;
-        ScanResults results1 = ScanResults.create(0, 2400);
+        ScanResults results1 = ScanResults.create(0, true, 2400);
 
 
         WifiScanner.ScanSettings requestSettings2 = createRequest(channelsToSpec(2450, 5175), 0,
                 0, 20, WifiScanner.REPORT_EVENT_AFTER_EACH_SCAN);
         int requestId2 = 13;
-        ScanResults results2 = ScanResults.create(0, 2450);
+        ScanResults results2 = ScanResults.create(0, true, 2450);
 
 
         startServiceAndLoadDriver();
@@ -848,12 +855,12 @@ public class WifiScanningServiceTest {
         WifiScanner.ScanSettings requestSettings1 = createRequest(channelsToSpec(2400), 0,
                 0, 20, WifiScanner.REPORT_EVENT_AFTER_EACH_SCAN);
         int requestId1 = 12;
-        ScanResults results1 = ScanResults.create(0, 2400);
+        ScanResults results1 = ScanResults.create(0, true, 2400);
 
         WifiScanner.ScanSettings requestSettings2 = createRequest(channelsToSpec(2450, 5175), 0,
                 0, 20, WifiScanner.REPORT_EVENT_AFTER_EACH_SCAN);
         int requestId2 = 13;
-        ScanResults results2 = ScanResults.create(0, 2450);
+        ScanResults results2 = ScanResults.create(0, true, 2450);
 
 
         startServiceAndLoadDriver();
@@ -918,19 +925,19 @@ public class WifiScanningServiceTest {
                 0, 20, WifiScanner.REPORT_EVENT_AFTER_EACH_SCAN);
         int requestId1 = 12;
         WorkSource workSource1 = new WorkSource(1121);
-        ScanResults results1 = ScanResults.create(0, 2400);
+        ScanResults results1 = ScanResults.create(0, false, 2400);
 
         WifiScanner.ScanSettings requestSettings2 = createRequest(channelsToSpec(2450, 5175), 0,
                 0, 20, WifiScanner.REPORT_EVENT_AFTER_EACH_SCAN);
         int requestId2 = 13;
         WorkSource workSource2 = new WorkSource(Binder.getCallingUid()); // don't explicitly set
-        ScanResults results2 = ScanResults.create(0, 2450, 5175, 2450);
+        ScanResults results2 = ScanResults.create(0, false, 2450, 5175, 2450);
 
         WifiScanner.ScanSettings requestSettings3 = createRequest(channelsToSpec(5150), 0,
                 0, 20, WifiScanner.REPORT_EVENT_AFTER_EACH_SCAN);
         int requestId3 = 15;
         WorkSource workSource3 = new WorkSource(2292);
-        ScanResults results3 = ScanResults.create(0, 5150, 5150, 5150, 5150);
+        ScanResults results3 = ScanResults.create(0, false, 5150, 5150, 5150, 5150);
 
         WifiNative.ScanSettings nativeSettings2and3 = createSingleScanNativeSettingsForChannels(
                 WifiScanner.REPORT_EVENT_AFTER_EACH_SCAN, channelsToSpec(2450, 5175, 5150));
@@ -980,7 +987,7 @@ public class WifiScanningServiceTest {
         mLooper.dispatchAll();
         verifyScanResultsReceived(handlerOrder, handler, requestId1, results1.getScanData());
         verifySingleScanCompletedReceived(handlerOrder, handler, requestId1);
-        verify(mContext).sendBroadcastAsUser(any(Intent.class), eq(UserHandle.ALL));
+        verify(mContext, never()).sendBroadcastAsUser(any(Intent.class), eq(UserHandle.ALL));
         verify(mBatteryStats).noteWifiScanStoppedFromSource(eq(workSource1));
         verify(mBatteryStats).noteWifiScanStartedFromSource(eq(workSource2and3));
 
@@ -1161,7 +1168,7 @@ public class WifiScanningServiceTest {
 
         verifyScanResultsReceived(handlerOrder, handler, requestId2, results2.getScanData());
         verifySingleScanCompletedReceived(handlerOrder, handler, requestId2);
-        verify(mContext, times(2)).sendBroadcastAsUser(any(Intent.class), eq(UserHandle.ALL));
+        verify(mContext, never()).sendBroadcastAsUser(any(Intent.class), eq(UserHandle.ALL));
         assertEquals(mWifiMetrics.getOneshotScanCount(), 3);
         assertEquals(mWifiMetrics.getScanReturnEntry(WifiMetricsProto.WifiLog.SCAN_SUCCESS), 3);
 
@@ -1257,10 +1264,10 @@ public class WifiScanningServiceTest {
     }
 
     /**
-     * Verify that the newest scan results are returned by WifiService.getSingleScanResults.
+     * Verify that the newest full scan results are returned by WifiService.getSingleScanResults.
      */
     @Test
-    public void retrieveMostRecentSingleScanResults() throws Exception {
+    public void retrieveMostRecentFullSingleScanResults() throws Exception {
         WifiScanner.ScanSettings requestSettings = createRequest(WifiScanner.WIFI_BAND_BOTH, 0,
                 0, 20, WifiScanner.REPORT_EVENT_AFTER_EACH_SCAN);
         ScanResults expectedResults = ScanResults.create(0, true, 2400, 5150, 5175);
@@ -1312,6 +1319,106 @@ public class WifiScanningServiceTest {
     }
 
     /**
+     * Verify that the newest partial scan results are not returned by
+     * WifiService.getSingleScanResults.
+     */
+    @Test
+    public void doesNotRetrieveMostRecentPartialSingleScanResults() throws Exception {
+        WifiScanner.ScanSettings fullRequestSettings = createRequest(WifiScanner.WIFI_BAND_BOTH, 0,
+                0, 20, WifiScanner.REPORT_EVENT_AFTER_EACH_SCAN);
+        ScanResults expectedFullResults = ScanResults.create(0, true, 2400, 5150, 5175);
+        doSuccessfulSingleScan(fullRequestSettings,
+                computeSingleScanNativeSettings(fullRequestSettings),
+                expectedFullResults);
+
+        Handler handler = mock(Handler.class);
+        BidirectionalAsyncChannel controlChannel = connectChannel(handler);
+        InOrder order = inOrder(handler, mWifiScannerImpl);
+
+        controlChannel.sendMessage(
+                Message.obtain(null, WifiScanner.CMD_GET_SINGLE_SCAN_RESULTS, 0));
+        mLooper.dispatchAll();
+        Message response = verifyHandleMessageAndGetMessage(order, handler);
+
+        List<ScanResult> results = Arrays.asList(
+                ((WifiScanner.ParcelableScanResults) response.obj).getResults());
+        assertEquals(results.size(), expectedFullResults.getRawScanResults().length);
+
+        // now update with a new scan that only has one result
+        int secondScanRequestId = 35;
+        WifiScanner.ScanSettings partialRequestSettings = createRequest(WifiScanner.WIFI_BAND_BOTH,
+                0, 0, 20, WifiScanner.REPORT_EVENT_AFTER_EACH_SCAN);
+        ScanResults expectedPartialResults = ScanResults.create(0, false, 5150);
+        sendSingleScanRequest(controlChannel, secondScanRequestId, partialRequestSettings, null);
+
+        mLooper.dispatchAll();
+        WifiNative.ScanEventHandler eventHandler = verifyStartSingleScan(order,
+                computeSingleScanNativeSettings(partialRequestSettings));
+        verifySuccessfulResponse(order, handler, secondScanRequestId);
+
+        // dispatch scan 2 results
+        when(mWifiScannerImpl.getLatestSingleScanResults())
+                .thenReturn(expectedPartialResults.getScanData());
+        eventHandler.onScanStatus(WifiNative.WIFI_SCAN_RESULTS_AVAILABLE);
+
+        mLooper.dispatchAll();
+        verifyScanResultsReceived(order, handler, secondScanRequestId,
+                expectedPartialResults.getScanData());
+        verifySingleScanCompletedReceived(order, handler, secondScanRequestId);
+
+        controlChannel.sendMessage(
+                Message.obtain(null, WifiScanner.CMD_GET_SINGLE_SCAN_RESULTS, 0));
+        mLooper.dispatchAll();
+        Message response2 = verifyHandleMessageAndGetMessage(order, handler);
+
+        List<ScanResult> results2 = Arrays.asList(
+                ((WifiScanner.ParcelableScanResults) response2.obj).getResults());
+        assertEquals(results2.size(), expectedFullResults.getRawScanResults().length);
+    }
+
+    /**
+     * Verify that the scan results returned by WifiService.getSingleScanResults are not older
+     * than {@link com.android.server.wifi.scanner.WifiScanningServiceImpl
+     * .WifiSingleScanStateMachine#CACHED_SCAN_RESULTS_MAX_AGE_IN_MILLIS}.
+     */
+    @Test
+    public void doesNotRetrieveStaleScanResultsFromLastFullSingleScan() throws Exception {
+        WifiScanner.ScanSettings requestSettings = createRequest(WifiScanner.WIFI_BAND_BOTH, 0,
+                0, 20, WifiScanner.REPORT_EVENT_AFTER_EACH_SCAN);
+        ScanResults scanResults = ScanResults.create(0, true, 2400, 5150, 5175);
+
+        // Out of the 3 scan results, modify the timestamp of 2 of them to be within the expiration
+        // age and 1 out of it.
+        long currentTimeInMillis = CACHED_SCAN_RESULTS_MAX_AGE_IN_MILLIS * 2;
+        when(mClock.getElapsedSinceBootMillis()).thenReturn(currentTimeInMillis);
+        scanResults.getRawScanResults()[0].timestamp = (currentTimeInMillis - 1) * 1000;
+        scanResults.getRawScanResults()[1].timestamp = (currentTimeInMillis - 2) * 1000;
+        scanResults.getRawScanResults()[2].timestamp =
+                (currentTimeInMillis - CACHED_SCAN_RESULTS_MAX_AGE_IN_MILLIS) * 1000;
+        List<ScanResult> expectedResults = new ArrayList<ScanResult>() {{
+                add(scanResults.getRawScanResults()[0]);
+                add(scanResults.getRawScanResults()[1]);
+            }};
+
+        doSuccessfulSingleScan(requestSettings,
+                computeSingleScanNativeSettings(requestSettings), scanResults);
+
+        Handler handler = mock(Handler.class);
+        BidirectionalAsyncChannel controlChannel = connectChannel(handler);
+        InOrder order = inOrder(handler, mWifiScannerImpl);
+
+        controlChannel.sendMessage(
+                Message.obtain(null, WifiScanner.CMD_GET_SINGLE_SCAN_RESULTS, 0));
+        mLooper.dispatchAll();
+        Message response = verifyHandleMessageAndGetMessage(order, handler);
+
+        List<ScanResult> results = Arrays.asList(
+                ((WifiScanner.ParcelableScanResults) response.obj).getResults());
+        assertScanResultsEquals(expectedResults.toArray(new ScanResult[expectedResults.size()]),
+                results.toArray(new ScanResult[results.size()]));
+    }
+
+    /**
      * Cached scan results should be cleared after the driver is unloaded.
      */
     @Test
@@ -1391,7 +1498,7 @@ public class WifiScanningServiceTest {
         verifyScanResultsReceived(order, handler, requestId, results.getScanData());
         verifySingleScanCompletedReceived(order, handler, requestId);
         verifyScanResultsReceived(order, handler, listenerRequestId, results.getScanData());
-        verify(mContext).sendBroadcastAsUser(any(Intent.class), eq(UserHandle.ALL));
+        verify(mContext, never()).sendBroadcastAsUser(any(Intent.class), eq(UserHandle.ALL));
         verifyNoMoreInteractions(handler);
 
         assertDumpContainsRequestLog("registerScanListener", listenerRequestId);
@@ -1442,7 +1549,7 @@ public class WifiScanningServiceTest {
         mLooper.dispatchAll();
         verifyScanResultsReceived(order, handler, requestId, results.getScanData());
         verifySingleScanCompletedReceived(order, handler, requestId);
-        verify(mContext).sendBroadcastAsUser(any(Intent.class), eq(UserHandle.ALL));
+        verify(mContext, never()).sendBroadcastAsUser(any(Intent.class), eq(UserHandle.ALL));
         verifyNoMoreInteractions(handler);
 
         assertDumpContainsRequestLog("registerScanListener", listenerRequestId);
@@ -1521,7 +1628,7 @@ public class WifiScanningServiceTest {
         verifyScanResultsReceived(handlerOrder, handler, requestId1, results1.getScanData());
         verifySingleScanCompletedReceived(handlerOrder, handler, requestId1);
         verifyScanResultsReceived(handlerOrder, handler, listenerRequestId, results1.getScanData());
-        verify(mContext).sendBroadcastAsUser(any(Intent.class), eq(UserHandle.ALL));
+        verify(mContext, never()).sendBroadcastAsUser(any(Intent.class), eq(UserHandle.ALL));
 
         // now that the first scan completed we expect the second and third ones to start
         WifiNative.ScanEventHandler eventHandler2and3 = verifyStartSingleScan(nativeOrder,