WifiNetworkFactory: Coalesce scan results for matches

Since scans are flaky by nature, use a simple scan cache (with very stale scan
results weeded out). This prevents cases where we don't find a specific
bssid in the latest set of scan resuls (which results in us not setting
the BSSID and hence results in bssid selection in firmware).

Bug: 164441864
Test: atest com.android.server.wifi
Merged-In: I772663f03dcfb5b58fe2f36acec95da19459b111
Change-Id: I772663f03dcfb5b58fe2f36acec95da19459b111
(cherry picked from commit 17bd63ebfc4554f33828ba3794944eab591abe03)

2 files changed, 212 insertions, 29 deletions

diff --git a/service/java/com/android/server/wifi/WifiNetworkFactory.java b/service/java/com/android/server/wifi/WifiNetworkFactory.java
index 9dc89addc..d1f9fbb1f 100644
--- a/service/java/com/android/server/wifi/WifiNetworkFactory.java
+++ b/service/java/com/android/server/wifi/WifiNetworkFactory.java
@@ -65,6 +65,7 @@ import com.android.server.wifi.util.WifiPermissionsUtil;
 import java.io.FileDescriptor;
 import java.io.PrintWriter;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -74,6 +75,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
+import java.util.stream.Collectors;
 
 /**
  * Network factory to handle trusted wifi network requests.
@@ -128,20 +130,23 @@ public class WifiNetworkFactory extends NetworkFactory {
     private WifiScanner mWifiScanner;
     private CompanionDeviceManager mCompanionDeviceManager;
     // Temporary approval set by shell commands.
-    private String mApprovedApp = null;
+    @Nullable private String mApprovedApp = null;
 
     private int mGenericConnectionReqCount = 0;
     // Request that is being actively processed. All new requests start out as an "active" request
     // because we're processing it & handling all the user interactions associated with it. Once we
     // successfully connect to the network, we transition that request to "connected".
-    private NetworkRequest mActiveSpecificNetworkRequest;
-    private WifiNetworkSpecifier mActiveSpecificNetworkRequestSpecifier;
+    @Nullable private NetworkRequest mActiveSpecificNetworkRequest;
+    @Nullable private WifiNetworkSpecifier mActiveSpecificNetworkRequestSpecifier;
     // Request corresponding to the the network that the device is currently connected to.
-    private NetworkRequest mConnectedSpecificNetworkRequest;
-    private WifiNetworkSpecifier mConnectedSpecificNetworkRequestSpecifier;
-    private WifiConfiguration mUserSelectedNetwork;
+    @Nullable private NetworkRequest mConnectedSpecificNetworkRequest;
+    @Nullable private WifiNetworkSpecifier mConnectedSpecificNetworkRequestSpecifier;
+    @Nullable private WifiConfiguration mUserSelectedNetwork;
     private int mUserSelectedNetworkConnectRetryCount;
-    private List<ScanResult> mActiveMatchedScanResults;
+    // Map of bssid to latest scan results for all scan results matching a request. Will be
+    //  - null, if there are no active requests.
+    //  - empty, if there are no matching scan results received for the active request.
+    @Nullable private Map<String, ScanResult> mActiveMatchedScanResults;
     // Verbose logging flag.
     private boolean mVerboseLoggingEnabled = false;
     private boolean mPeriodicScanTimerSet = false;
@@ -237,7 +242,10 @@ public class WifiNetworkFactory extends NetworkFactory {
                 Log.v(TAG, "Received " + scanResults.length + " scan results");
             }
             handleScanResults(scanResults);
-            sendNetworkRequestMatchCallbacksForActiveRequest(mActiveMatchedScanResults);
+            if (mActiveMatchedScanResults != null) {
+                sendNetworkRequestMatchCallbacksForActiveRequest(
+                        mActiveMatchedScanResults.values());
+            }
             scheduleNextPeriodicScan();
         }
 
@@ -432,7 +440,10 @@ public class WifiNetworkFactory extends NetworkFactory {
 
         // If we are already in the midst of processing a request, send matching callbacks
         // immediately on registering the callback.
-        sendNetworkRequestMatchCallbacksForActiveRequest(mActiveMatchedScanResults);
+        if (mActiveMatchedScanResults != null) {
+            sendNetworkRequestMatchCallbacksForActiveRequest(
+                    mActiveMatchedScanResults.values());
+        }
     }
 
     /**
@@ -610,8 +621,11 @@ public class WifiNetworkFactory extends NetworkFactory {
                 if (mVerboseLoggingEnabled) {
                     Log.v(TAG, "Using cached " + cachedScanResults.length + " scan results");
                 }
-                handleScanResults(cachedScanResults);
-                sendNetworkRequestMatchCallbacksForActiveRequest(mActiveMatchedScanResults);
+                handleScanResults(cachedScanResults);          
+                if (mActiveMatchedScanResults != null) {
+                    sendNetworkRequestMatchCallbacksForActiveRequest(
+                            mActiveMatchedScanResults.values());
+                }
                 startPeriodicScans();
             }
         }
@@ -1163,8 +1177,8 @@ public class WifiNetworkFactory extends NetworkFactory {
     }
 
     private void sendNetworkRequestMatchCallbacksForActiveRequest(
-            @Nullable List<ScanResult> matchedScanResults) {
-        if (matchedScanResults == null || matchedScanResults.isEmpty()) return;
+            @NonNull Collection<ScanResult> matchedScanResults) {
+        if (matchedScanResults.isEmpty()) return;
         if (mRegisteredCallbacks.getNumCallbacks() == 0) {
             Log.e(TAG, "No callback registered for sending network request matches. "
                     + "Ignoring...");
@@ -1172,7 +1186,7 @@ public class WifiNetworkFactory extends NetworkFactory {
         }
         for (INetworkRequestMatchCallback callback : mRegisteredCallbacks.getCallbacks()) {
             try {
-                callback.onMatch(matchedScanResults);
+                callback.onMatch(new ArrayList<>(matchedScanResults));
             } catch (RemoteException e) {
                 Log.e(TAG, "Unable to invoke network request match callback " + callback, e);
             }
@@ -1265,6 +1279,7 @@ public class WifiNetworkFactory extends NetworkFactory {
         if (mActiveSpecificNetworkRequestSpecifier == null
                 || mActiveMatchedScanResults == null) return null;
         ScanResult selectedScanResult = mActiveMatchedScanResults
+                .values()
                 .stream()
                 .filter(scanResult -> Objects.equals(
                         ScanResultMatchInfo.fromScanResult(scanResult),
@@ -1354,7 +1369,7 @@ public class WifiNetworkFactory extends NetworkFactory {
 
         ScanResultMatchInfo fromWifiConfiguration =
                 ScanResultMatchInfo.fromWifiConfiguration(network);
-        for (ScanResult scanResult : mActiveMatchedScanResults) {
+        for (ScanResult scanResult : mActiveMatchedScanResults.values()) {
             ScanResultMatchInfo fromScanResult = ScanResultMatchInfo.fromScanResult(scanResult);
             if (fromScanResult.equals(fromWifiConfiguration)) {
                 AccessPoint approvedAccessPoint =
@@ -1433,7 +1448,19 @@ public class WifiNetworkFactory extends NetworkFactory {
             mWifiMetrics.incrementNetworkRequestApiMatchSizeHistogram(
                     matchedScanResults.size());
         }
-        mActiveMatchedScanResults = matchedScanResults;
+        // First set of scan results for this request.
+        if (mActiveMatchedScanResults == null) mActiveMatchedScanResults = new HashMap<>();
+        // Coalesce the new set of scan results with previous scan results received for request.
+        mActiveMatchedScanResults.putAll(matchedScanResults
+                .stream()
+                .collect(Collectors.toMap(
+                        scanResult -> scanResult.BSSID, scanResult -> scanResult)));
+        // Weed out any stale cached scan results.
+        long currentTimeInMillis = mClock.getElapsedSinceBootMillis();
+        mActiveMatchedScanResults.entrySet().removeIf(
+                e -> ((currentTimeInMillis - (e.getValue().timestamp / 1000))
+                        >= CACHED_SCAN_RESULTS_MAX_AGE_IN_MILLIS));
+
     }
 
     /**
diff --git a/tests/wifitests/src/com/android/server/wifi/WifiNetworkFactoryTest.java b/tests/wifitests/src/com/android/server/wifi/WifiNetworkFactoryTest.java
index 5f9e0333e..f6621a698 100644
--- a/tests/wifitests/src/com/android/server/wifi/WifiNetworkFactoryTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/WifiNetworkFactoryTest.java
@@ -52,6 +52,7 @@ import android.net.wifi.WifiNetworkSpecifier;
 import android.net.wifi.WifiScanner;
 import android.net.wifi.WifiScanner.ScanListener;
 import android.net.wifi.WifiScanner.ScanSettings;
+import android.net.wifi.WifiSsid;
 import android.os.Binder;
 import android.os.IBinder;
 import android.os.Message;
@@ -1122,9 +1123,7 @@ public class WifiNetworkFactoryTest extends WifiBaseTest {
         verify(mWifiMetrics).setNominatorForNetwork(anyInt(),
                 eq(WifiMetricsProto.ConnectionEvent.NOMINATOR_SPECIFIER));
 
-        verify(mClientModeImpl).disconnectCommand();
-        verify(mClientModeImpl).connect(eq(null), eq(TEST_NETWORK_ID_1), any(Binder.class),
-                mConnectListenerArgumentCaptor.capture(), anyInt(), anyInt());
+
     }
 
     /**
@@ -1333,6 +1332,134 @@ public class WifiNetworkFactoryTest extends WifiBaseTest {
     }
 
     /**
+     * Verify handling of user selection to trigger connection to a network when the selected bssid
+     * is no longer seen in scan results within the cache expiry duration. Ensure we fill up the
+     * BSSID field.
+     */
+    @Test
+    public void
+            testNetworkSpecifierHandleUserSelectionConnectToNetworkMissingBssidInLatest()
+            throws Exception {
+        WifiScanner.ScanData[] scanDatas1 =
+                ScanTestUtil.createScanDatas(new int[][]{ { 2417, 2427, 5180, 5170 }});
+        setupScanData(scanDatas1, SCAN_RESULT_TYPE_WPA_PSK,
+                TEST_SSID_1, TEST_SSID_2, TEST_SSID_3, TEST_SSID_4);
+        // Modify the next set of scan results to simulate missing |TEST_SSID_1| ScanResult.
+        WifiScanner.ScanData[] scanDatas2 =
+                ScanTestUtil.createScanDatas(new int[][]{ { 2417, 2427, 5180, 5170 }});
+        setupScanData(scanDatas2, SCAN_RESULT_TYPE_WPA_PSK,
+                TEST_SSID_2, TEST_SSID_2, TEST_SSID_3, TEST_SSID_4);
+
+        // Make a specific AP request.
+        ScanResult matchingScanResult = scanDatas1[0].getResults()[0];
+        PatternMatcher ssidPatternMatch =
+                new PatternMatcher(TEST_SSID_1, PatternMatcher.PATTERN_LITERAL);
+        Pair<MacAddress, MacAddress> bssidPatternMatch =
+                Pair.create(MacAddress.fromString(matchingScanResult.BSSID),
+                        MacAddress.BROADCAST_ADDRESS);
+        WifiConfiguration wifiConfiguration = new WifiConfiguration();
+        wifiConfiguration.allowedKeyManagement.set(WifiConfiguration.KeyMgmt.WPA_PSK);
+        wifiConfiguration.preSharedKey = TEST_WPA_PRESHARED_KEY;
+        attachWifiNetworkSpecifierAndAppInfo(
+                ssidPatternMatch, bssidPatternMatch, wifiConfiguration, TEST_UID_1,
+                TEST_PACKAGE_NAME_1);
+        mWifiNetworkFactory.needNetworkFor(mNetworkRequest, 0);
+        mWifiNetworkFactory.addCallback(mAppBinder, mNetworkRequestMatchCallback,
+                TEST_CALLBACK_IDENTIFIER);
+        verify(mNetworkRequestMatchCallback).onUserSelectionCallbackRegistration(
+                mNetworkRequestUserSelectionCallback.capture());
+        verifyPeriodicScans(
+                0L,
+                new PeriodicScanParams(0, scanDatas1),
+                new PeriodicScanParams(PERIODIC_SCAN_INTERVAL_MS, scanDatas2));
+
+        // Now trigger user selection to the network.
+        mSelectedNetwork = ScanResultUtil.createNetworkFromScanResult(matchingScanResult);
+        mSelectedNetwork.SSID = "\"" + matchingScanResult.SSID + "\"";
+        INetworkRequestUserSelectionCallback networkRequestUserSelectionCallback =
+                mNetworkRequestUserSelectionCallback.getValue();
+        assertNotNull(networkRequestUserSelectionCallback);
+        sendUserSelectionSelect(networkRequestUserSelectionCallback, mSelectedNetwork);
+        mLooper.dispatchAll();
+
+        // Verify WifiConfiguration params.
+        validateConnectParams(mSelectedNetwork.SSID, matchingScanResult.BSSID);
+        verify(mWifiMetrics).setNominatorForNetwork(anyInt(),
+                eq(WifiMetricsProto.ConnectionEvent.NOMINATOR_SPECIFIER));
+
+        verify(mClientModeImpl).disconnectCommand();
+        verify(mClientModeImpl).connect(eq(null), eq(TEST_NETWORK_ID_1), any(Binder.class),
+                mConnectListenerArgumentCaptor.capture(), anyInt(), anyInt());
+    }
+
+    /**
+     * Verify handling of user selection to trigger connection to a network when the selected bssid
+     * is no longer seen in scan results beyond the cache expiry duration. Ensure we don't fill up
+     * the BSSID field.
+     */
+    @Test
+    public void
+            testNetworkSpecifierHandleUserSelectionConnectToNetworkStaleMissingBssidInLatest()
+            throws Exception {
+        WifiScanner.ScanData[] scanDatas1 =
+                ScanTestUtil.createScanDatas(new int[][]{ { 2417, 2427, 5180, 5170 }});
+        setupScanData(scanDatas1, SCAN_RESULT_TYPE_WPA_PSK,
+                TEST_SSID_1, TEST_SSID_2, TEST_SSID_3, TEST_SSID_4);
+        // Modify the next set of scan results to simulate missing |TEST_SSID_1| ScanResult.
+        WifiScanner.ScanData[] scanDatas2 =
+                ScanTestUtil.createScanDatas(new int[][]{ { 2417, 2427, 5180, 5170 }});
+        setupScanData(scanDatas2, SCAN_RESULT_TYPE_WPA_PSK,
+                TEST_SSID_2, TEST_SSID_2, TEST_SSID_3, TEST_SSID_4);
+
+        // Make a specific AP request.
+        ScanResult matchingScanResult = scanDatas1[0].getResults()[0];
+        PatternMatcher ssidPatternMatch =
+                new PatternMatcher(TEST_SSID_1, PatternMatcher.PATTERN_LITERAL);
+        Pair<MacAddress, MacAddress> bssidPatternMatch =
+                Pair.create(MacAddress.fromString(matchingScanResult.BSSID),
+                        MacAddress.BROADCAST_ADDRESS);
+        WifiConfiguration wifiConfiguration = new WifiConfiguration();
+        wifiConfiguration.allowedKeyManagement.set(WifiConfiguration.KeyMgmt.WPA_PSK);
+        wifiConfiguration.preSharedKey = TEST_WPA_PRESHARED_KEY;
+        attachWifiNetworkSpecifierAndAppInfo(
+                ssidPatternMatch, bssidPatternMatch, wifiConfiguration, TEST_UID_1,
+                TEST_PACKAGE_NAME_1);
+        mWifiNetworkFactory.needNetworkFor(mNetworkRequest, 0);
+        mWifiNetworkFactory.addCallback(mAppBinder, mNetworkRequestMatchCallback,
+                TEST_CALLBACK_IDENTIFIER);
+        verify(mNetworkRequestMatchCallback).onUserSelectionCallbackRegistration(
+                mNetworkRequestUserSelectionCallback.capture());
+
+        long nowMs = WifiNetworkFactory.CACHED_SCAN_RESULTS_MAX_AGE_IN_MILLIS + 1;
+        scanDatas2[0].getResults()[0].timestamp = nowMs;
+        scanDatas2[0].getResults()[1].timestamp = nowMs;
+        scanDatas2[0].getResults()[2].timestamp = nowMs;
+        scanDatas2[0].getResults()[3].timestamp = nowMs;
+        verifyPeriodicScans(
+                nowMs,
+                new PeriodicScanParams(0, scanDatas1),
+                new PeriodicScanParams(PERIODIC_SCAN_INTERVAL_MS, scanDatas2));
+
+        // Now trigger user selection to the network.
+        mSelectedNetwork = ScanResultUtil.createNetworkFromScanResult(matchingScanResult);
+        mSelectedNetwork.SSID = "\"" + matchingScanResult.SSID + "\"";
+        INetworkRequestUserSelectionCallback networkRequestUserSelectionCallback =
+                mNetworkRequestUserSelectionCallback.getValue();
+        assertNotNull(networkRequestUserSelectionCallback);
+        sendUserSelectionSelect(networkRequestUserSelectionCallback, mSelectedNetwork);
+        mLooper.dispatchAll();
+
+        // Verify WifiConfiguration params.
+        validateConnectParams(mSelectedNetwork.SSID, null);
+        verify(mWifiMetrics).setNominatorForNetwork(anyInt(),
+                eq(WifiMetricsProto.ConnectionEvent.NOMINATOR_SPECIFIER));
+
+        verify(mClientModeImpl).disconnectCommand();
+        verify(mClientModeImpl).connect(eq(null), eq(TEST_NETWORK_ID_1), any(Binder.class),
+                mConnectListenerArgumentCaptor.capture(), anyInt(), anyInt());
+    }
+
+    /**
      * Verify handling of user selection to reject the request.
      */
     @Test
@@ -2669,13 +2796,33 @@ public class WifiNetworkFactoryTest extends WifiBaseTest {
         verify(mNetworkRequestMatchCallback, atLeastOnce()).onMatch(anyList());
     }
 
+    private void verifyPeriodicScans(long...expectedIntervalsInSeconds) {
+        PeriodicScanParams[] periodicScanParams =
+                new PeriodicScanParams[expectedIntervalsInSeconds.length];
+        for (int i = 0; i < expectedIntervalsInSeconds.length; i++) {
+            periodicScanParams[i] =
+                    new PeriodicScanParams(expectedIntervalsInSeconds[i], mTestScanDatas);
+        }
+        verifyPeriodicScans(0L, periodicScanParams);
+    }
+
+    private static class PeriodicScanParams {
+        public final long expectedIntervalInSeconds;
+        public final WifiScanner.ScanData[] scanDatas;
+
+        PeriodicScanParams(long expectedIntervalInSeconds, WifiScanner.ScanData[] scanDatas) {
+            this.expectedIntervalInSeconds = expectedIntervalInSeconds;
+            this.scanDatas = scanDatas;
+        }
+    }
+
     // Simulates the periodic scans performed to find a matching network.
     // a) Start scan
     // b) Scan results received.
     // c) Set alarm for next scan at the expected interval.
     // d) Alarm fires, go to step a) again and repeat.
-    private void verifyPeriodicScans(long...expectedIntervalsInSeconds) {
-        when(mClock.getElapsedSinceBootMillis()).thenReturn(0L);
+    private void verifyPeriodicScans(long nowMs, PeriodicScanParams... scanParams) {
+        when(mClock.getElapsedSinceBootMillis()).thenReturn(nowMs);
 
         OnAlarmListener alarmListener = null;
         ScanListener scanListener = null;
@@ -2685,9 +2832,9 @@ public class WifiNetworkFactoryTest extends WifiBaseTest {
         // Before we start scans, ensure that we look at the latest cached scan results.
         mInOrder.verify(mWifiScanner).getSingleScanResults();
 
-        for (int i = 0; i < expectedIntervalsInSeconds.length - 1; i++) {
-            long expectedCurrentIntervalInMs = expectedIntervalsInSeconds[i];
-            long expectedNextIntervalInMs = expectedIntervalsInSeconds[i + 1];
+        for (int i = 0; i < scanParams.length - 1; i++) {
+            long expectedCurrentIntervalInMs = scanParams[i].expectedIntervalInSeconds;
+            long expectedNextIntervalInMs = scanParams[i + 1].expectedIntervalInSeconds;
 
             // First scan is immediately fired, so need for the alarm to fire.
             if (expectedCurrentIntervalInMs != 0) {
@@ -2700,10 +2847,10 @@ public class WifiNetworkFactoryTest extends WifiBaseTest {
             assertNotNull(scanListener);
 
             // Now trigger the scan results callback and verify the alarm set for the next scan.
-            scanListener.onResults(mTestScanDatas);
+            scanListener.onResults(scanParams[i].scanDatas);
 
             mInOrder.verify(mAlarmManager).set(eq(AlarmManager.ELAPSED_REALTIME_WAKEUP),
-                    eq(expectedNextIntervalInMs), any(),
+                    eq(expectedNextIntervalInMs + nowMs), any(),
                     mPeriodicScanListenerArgumentCaptor.capture(), any());
             alarmListener = mPeriodicScanListenerArgumentCaptor.getValue();
             assertNotNull(alarmListener);
@@ -2770,30 +2917,39 @@ public class WifiNetworkFactoryTest extends WifiBaseTest {
         return "";
     }
 
-    // Helper method to setup the scan data for verifying the matching algo.
     private void setupScanData(int scanResultType, String ssid1, String ssid2, String ssid3,
             String ssid4) {
+        setupScanData(mTestScanDatas, scanResultType, ssid1, ssid2, ssid3, ssid4);
+    }
+
+    // Helper method to setup the scan data for verifying the matching algo.
+    private void setupScanData(WifiScanner.ScanData[] testScanDatas, int scanResultType,
+            String ssid1, String ssid2, String ssid3, String ssid4) {
         // 4 scan results,
-        assertEquals(1, mTestScanDatas.length);
-        ScanResult[] scanResults = mTestScanDatas[0].getResults();
+        assertEquals(1, testScanDatas.length);
+        ScanResult[] scanResults = testScanDatas[0].getResults();
         assertEquals(4, scanResults.length);
 
         String caps = getScanResultCapsForType(scanResultType);
 
         // Scan results have increasing RSSI.
         scanResults[0].SSID = ssid1;
+        scanResults[0].wifiSsid = WifiSsid.createFromAsciiEncoded(ssid1);
         scanResults[0].BSSID = TEST_BSSID_1;
         scanResults[0].capabilities = caps;
         scanResults[0].level = -45;
         scanResults[1].SSID = ssid2;
+        scanResults[1].wifiSsid = WifiSsid.createFromAsciiEncoded(ssid2);
         scanResults[1].BSSID = TEST_BSSID_2;
         scanResults[1].capabilities = caps;
         scanResults[1].level = -35;
         scanResults[2].SSID = ssid3;
+        scanResults[2].wifiSsid = WifiSsid.createFromAsciiEncoded(ssid3);
         scanResults[2].BSSID = TEST_BSSID_3;
         scanResults[2].capabilities = caps;
         scanResults[2].level = -25;
         scanResults[3].SSID = ssid4;
+        scanResults[3].wifiSsid = WifiSsid.createFromAsciiEncoded(ssid4);
         scanResults[3].BSSID = TEST_BSSID_4;
         scanResults[3].capabilities = caps;
         scanResults[3].level = -15;