Merge "[WLRW] don't take bugreport if reconnect on new network" into pi-dev

3 files changed, 123 insertions, 14 deletions

diff --git a/service/java/com/android/server/wifi/WifiLastResortWatchdog.java b/service/java/com/android/server/wifi/WifiLastResortWatchdog.java
index 34504909c..b839f86be 100644
--- a/service/java/com/android/server/wifi/WifiLastResortWatchdog.java
+++ b/service/java/com/android/server/wifi/WifiLastResortWatchdog.java
@@ -26,9 +26,11 @@ import android.util.Pair;
 import com.android.internal.annotations.VisibleForTesting;
 
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 /**
  * This Class is a Work-In-Progress, intended behavior is as follows:
@@ -72,12 +74,17 @@ public class WifiLastResortWatchdog {
      * Key:BSSID, Value:Counters of failure types
      */
     private Map<String, AvailableNetworkFailureCount> mRecentAvailableNetworks = new HashMap<>();
+
     /**
      * Map of SSID to <FailureCount, AP count>, used to count failures & number of access points
      * belonging to an SSID.
      */
     private Map<String, Pair<AvailableNetworkFailureCount, Integer>> mSsidFailureCount =
             new HashMap<>();
+
+    // Set of SSIDs that watchdog tried to connect to before finally triggering a restart
+    private Set<String> mSsidAvailableAtFailure = new HashSet<>();
+
     // Tracks: if WifiStateMachine is in ConnectedState
     private boolean mWifiIsConnected = false;
     // Is Watchdog allowed to trigger now? Set to false after triggering. Set to true after
@@ -210,6 +217,8 @@ public class WifiLastResortWatchdog {
             // Stop the watchdog from triggering until re-enabled
             setWatchdogTriggerEnabled(false);
             Log.e(TAG, "Watchdog triggering recovery");
+            mSsidAvailableAtFailure.clear();
+            mSsidAvailableAtFailure.addAll(mSsidFailureCount.keySet());
             mSelfRecovery.trigger(SelfRecovery.REASON_LAST_RESORT_WATCHDOG);
             // increment various watchdog trigger count stats
             incrementWifiMetricsTriggerCounts();
@@ -222,8 +231,9 @@ public class WifiLastResortWatchdog {
      * Handles transitions entering and exiting WifiStateMachine ConnectedState
      * Used to track wifistate, and perform watchdog count reseting
      * @param isEntering true if called from ConnectedState.enter(), false for exit()
+     * @param ssid the ssid of the network doing the transition
      */
-    public void connectedStateTransition(boolean isEntering) {
+    public void connectedStateTransition(boolean isEntering, String ssid) {
         if (mVerboseLoggingEnabled) {
             Log.v(TAG, "connectedStateTransition: isEntering = " + isEntering);
         }
@@ -232,7 +242,11 @@ public class WifiLastResortWatchdog {
             // WiFi has connected after a Watchdog trigger, without any new networks becoming
             // available, log a Watchdog success in wifi metrics
             mWifiMetrics.incrementNumLastResortWatchdogSuccesses();
-            takeBugReportWithCurrentProbability("Wifi fixed after restart");
+            if (mSsidAvailableAtFailure.contains(ssid)) {
+                Log.e(TAG, "Wifi failed to connect on " + mSsidAvailableAtFailure.size()
+                        + " networks but was able to reconnect to one of them after a restart.");
+                takeBugReportWithCurrentProbability("Wifi fixed after restart");
+            }
         }
         if (isEntering) {
             // We connected to something! Reset failure counts for everything
@@ -397,7 +411,7 @@ public class WifiLastResortWatchdog {
         for (Map.Entry<String, AvailableNetworkFailureCount> entry
                 : mRecentAvailableNetworks.entrySet()) {
             final AvailableNetworkFailureCount failureCount = entry.getValue();
-            entry.getValue().resetCounts();
+            failureCount.resetCounts();
         }
         for (Map.Entry<String, Pair<AvailableNetworkFailureCount, Integer>> entry
                 : mSsidFailureCount.entrySet()) {
diff --git a/service/java/com/android/server/wifi/WifiStateMachine.java b/service/java/com/android/server/wifi/WifiStateMachine.java
index 100503713..4081ddefa 100644
--- a/service/java/com/android/server/wifi/WifiStateMachine.java
+++ b/service/java/com/android/server/wifi/WifiStateMachine.java
@@ -5593,8 +5593,9 @@ public class WifiStateMachine extends StateMachine {
             }
 
             mLastDriverRoamAttempt = 0;
+            String curSsid = getTargetSsid();
             mTargetNetworkId = WifiConfiguration.INVALID_NETWORK_ID;
-            mWifiInjector.getWifiLastResortWatchdog().connectedStateTransition(true);
+            mWifiInjector.getWifiLastResortWatchdog().connectedStateTransition(true, curSsid);
             mWifiStateTracker.updateState(WifiStateTracker.CONNECTED);
         }
         @Override
@@ -5749,7 +5750,8 @@ public class WifiStateMachine extends StateMachine {
                      WifiConnectivityManager.WIFI_STATE_TRANSITIONING);
 
             mLastDriverRoamAttempt = 0;
-            mWifiInjector.getWifiLastResortWatchdog().connectedStateTransition(false);
+            mWifiInjector.getWifiLastResortWatchdog().connectedStateTransition(false,
+                    getTargetSsid());
         }
     }
 
diff --git a/tests/wifitests/src/com/android/server/wifi/WifiLastResortWatchdogTest.java b/tests/wifitests/src/com/android/server/wifi/WifiLastResortWatchdogTest.java
index 876950865..4abc77f91 100644
--- a/tests/wifitests/src/com/android/server/wifi/WifiLastResortWatchdogTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/WifiLastResortWatchdogTest.java
@@ -900,7 +900,7 @@ public class WifiLastResortWatchdogTest {
         int dhcpFailures = 11;
 
         // Set Watchdogs internal wifi state tracking to 'connected'
-        mLastResortWatchdog.connectedStateTransition(true);
+        mLastResortWatchdog.connectedStateTransition(true, "");
 
         // Buffer potential candidates 1,2,3 & 4
         List<Pair<ScanDetail, WifiConfiguration>> candidates = createFilteredQnsCandidates(mSsids,
@@ -979,7 +979,7 @@ public class WifiLastResortWatchdogTest {
         assertFailureCountEquals(mBssids[2], 0, 0, dhcpFailures);
 
         // Transition to 'ConnectedState'
-        mLastResortWatchdog.connectedStateTransition(true);
+        mLastResortWatchdog.connectedStateTransition(true, "");
 
         // Check that we have no failures
         for (int i = 0; i < mSsids.length; i++) {
@@ -1233,7 +1233,7 @@ public class WifiLastResortWatchdogTest {
         mLastResortWatchdog.updateAvailableNetworks(candidates);
 
         // Set Watchdogs internal wifi state tracking to 'connected'
-        mLastResortWatchdog.connectedStateTransition(true);
+        mLastResortWatchdog.connectedStateTransition(true, "");
 
         // Count failures on all 4 networks until all of them are over the failure threshold
         boolean watchdogTriggered = false;
@@ -1348,8 +1348,8 @@ public class WifiLastResortWatchdogTest {
         }
 
         // transition Watchdog wifi state tracking to 'connected' then back to 'disconnected'
-        mLastResortWatchdog.connectedStateTransition(true);
-        mLastResortWatchdog.connectedStateTransition(false);
+        mLastResortWatchdog.connectedStateTransition(true, "");
+        mLastResortWatchdog.connectedStateTransition(false, "");
 
         // Fail 3/4 networks until they're over threshold
         for (int i = 0; i < WifiLastResortWatchdog.FAILURE_THRESHOLD + 1; i++) {
@@ -1421,7 +1421,7 @@ public class WifiLastResortWatchdogTest {
     }
 
     /**
-     * Case 26: Test Metrics collection
+     * Case 28: Test Metrics collection
      * Setup 5 networks (unique SSIDs). Fail them until watchdog triggers, with 1 network failing
      * association, 1 failing authentication, 2 failing dhcp and one failing both authentication and
      * dhcp, (over threshold for all these failures)
@@ -1486,7 +1486,7 @@ public class WifiLastResortWatchdogTest {
         verify(mWifiMetrics, times(1)).incrementNumLastResortWatchdogTriggersWithBadDhcp();
 
         // Simulate wifi connecting after triggering
-        mLastResortWatchdog.connectedStateTransition(true);
+        mLastResortWatchdog.connectedStateTransition(true, "\"test1\"");
 
         // Verify that WifiMetrics counted this as a Watchdog success
         verify(mWifiMetrics, times(1)).incrementNumLastResortWatchdogSuccesses();
@@ -1495,7 +1495,7 @@ public class WifiLastResortWatchdogTest {
         verify(mWifiStateMachine, times(1)).takeBugReport(anyString(), anyString());
 
         // Simulate wifi disconnecting
-        mLastResortWatchdog.connectedStateTransition(false);
+        mLastResortWatchdog.connectedStateTransition(false, "\"test1\"");
 
         // Verify that WifiMetrics has still only counted one success
         verify(mWifiMetrics, times(1)).incrementNumLastResortWatchdogSuccesses();
@@ -1537,7 +1537,7 @@ public class WifiLastResortWatchdogTest {
         mLastResortWatchdog.updateAvailableNetworks(candidates);
 
         // Simulate wifi connecting
-        mLastResortWatchdog.connectedStateTransition(true);
+        mLastResortWatchdog.connectedStateTransition(true, "");
 
         // Verify that WifiMetrics did not count another success, as the connection could be due
         // to the newly available network #5
@@ -1717,4 +1717,97 @@ public class WifiLastResortWatchdogTest {
         output = withNullConfig.toString();
         assertTrue(output.contains("HasEverConnected: null_config"));
     }
+
+    /**
+     * Case 29: Test connection success after wifi restart with an unexpected SSID
+     * Setup 1 network. Fail the network until watchcdog triggers. Trigger a connection success on
+     * a network which has a different SSID than the network that has been failing.
+     * Expected behavior: bugreport is not triggered
+     */
+    @Test
+    public void testConnectionSuccessWithUnexpectedSsidDoesNotTriggerBugreport() {
+        String[] ssids = {"\"test1\""};
+        String[] bssids = {"6c:f3:7f:ae:8c:f3"};
+        int[] frequencies = {2437};
+        String[] caps = {"[WPA2-EAP-CCMP][ESS]"};
+        int[] levels = {-60};
+        boolean[] isEphemeral = {false};
+        boolean[] hasEverConnected = {true};
+        List<Pair<ScanDetail, WifiConfiguration>> candidates = createFilteredQnsCandidates(ssids,
+                bssids, frequencies, caps, levels, isEphemeral, hasEverConnected);
+        mLastResortWatchdog.updateAvailableNetworks(candidates);
+
+        // Ensure new networks have zero'ed failure counts
+        for (int i = 0; i < ssids.length; i++) {
+            assertFailureCountEquals(bssids[i], 0, 0, 0);
+        }
+
+        //Increment failure counts
+        for (int i = 0; i < WifiLastResortWatchdog.FAILURE_THRESHOLD; i++) {
+            mLastResortWatchdog.noteConnectionFailureAndTriggerIfNeeded(
+                    ssids[0], bssids[0], WifiLastResortWatchdog.FAILURE_CODE_ASSOCIATION);
+        }
+
+        // Verify relevant WifiMetrics calls were made once with appropriate arguments
+        verify(mWifiMetrics, times(1)).incrementNumLastResortWatchdogTriggers();
+
+        // Simulate wifi connecting after triggering on a unexpected SSID
+        mLastResortWatchdog.connectedStateTransition(true, "blahssss");
+        // Verify takeBugReport is not called
+        mLooper.dispatchAll();
+        verify(mWifiStateMachine, times(0)).takeBugReport(anyString(), anyString());
+
+        // Simulate wifi connecting after triggering is the expected SSID, which should be ignored
+        // because watchdog state should already be reset to detect for failures
+        mLastResortWatchdog.connectedStateTransition(true, "\"test1\"");
+        // Verify takeBugReport is not called
+        mLooper.dispatchAll();
+        verify(mWifiStateMachine, times(0)).takeBugReport(anyString(), anyString());
+    }
+
+    /**
+     * Case 30: Test connection success after wifi restart with a previously failing SSID
+     * Setup 1 network. Fail the network until watchcdog triggers. Trigger a connection success on
+     * a network which has the same SSID than the network that has been failing.
+     * Expected behavior: bugreport is triggered
+     */
+    @Test
+    public void testConnectionSuccessWithPreviouslyFailingSsidTriggersBugreport() {
+        String[] ssids = {"\"test1\""};
+        String[] bssids = {"6c:f3:7f:ae:8c:f3"};
+        int[] frequencies = {2437};
+        String[] caps = {"[WPA2-EAP-CCMP][ESS]"};
+        int[] levels = {-60};
+        boolean[] isEphemeral = {false};
+        boolean[] hasEverConnected = {true};
+        List<Pair<ScanDetail, WifiConfiguration>> candidates = createFilteredQnsCandidates(ssids,
+                bssids, frequencies, caps, levels, isEphemeral, hasEverConnected);
+        mLastResortWatchdog.updateAvailableNetworks(candidates);
+
+        // Ensure new networks have zero'ed failure counts
+        for (int i = 0; i < ssids.length; i++) {
+            assertFailureCountEquals(bssids[i], 0, 0, 0);
+        }
+
+        //Increment failure counts
+        for (int i = 0; i < WifiLastResortWatchdog.FAILURE_THRESHOLD; i++) {
+            mLastResortWatchdog.noteConnectionFailureAndTriggerIfNeeded(
+                    ssids[0], bssids[0], WifiLastResortWatchdog.FAILURE_CODE_ASSOCIATION);
+        }
+
+        // Verify relevant WifiMetrics calls were made once with appropriate arguments
+        verify(mWifiMetrics, times(1)).incrementNumLastResortWatchdogTriggers();
+
+        // Simulate wifi connecting after triggering
+        mLastResortWatchdog.connectedStateTransition(true, "\"test1\"");
+        // Verify takeBugReport is called
+        mLooper.dispatchAll();
+        verify(mWifiStateMachine, times(1)).takeBugReport(anyString(), anyString());
+
+        // Simulate wifi connecting after triggering
+        mLastResortWatchdog.connectedStateTransition(true, "\"test1\"");
+        // Verify takeBugReport is not called again
+        mLooper.dispatchAll();
+        verify(mWifiStateMachine, times(1)).takeBugReport(anyString(), anyString());
+    }
 }