WifiNetworkFactory: Retry connection on failure

Any intermediate connection failures should trigger retries to
workaround any transient issues. Limit the number of retries to 3 for
now.

Bug: 119207115
Test: ./frameworks/opt/net/wifi/tests/wifitests/runtests.sh
Test: act.py -c wifi_manager.config -tb dut-name -tc WifiNetworkRequestTest
Change-Id: I466b6f43b4cede40850731d5a5190ee66d1ab69f

2 files changed, 109 insertions, 33 deletions

diff --git a/service/java/com/android/server/wifi/WifiNetworkFactory.java b/service/java/com/android/server/wifi/WifiNetworkFactory.java
index d1f3295dd..dc2ad87f8 100644
--- a/service/java/com/android/server/wifi/WifiNetworkFactory.java
+++ b/service/java/com/android/server/wifi/WifiNetworkFactory.java
@@ -68,6 +68,8 @@ public class WifiNetworkFactory extends NetworkFactory {
     public static final int PERIODIC_SCAN_INTERVAL_MS = 10 * 1000; // 10 seconds
     @VisibleForTesting
     public static final int NETWORK_CONNECTION_TIMEOUT_MS = 30 * 1000; // 30 seconds
+    @VisibleForTesting
+    public static final int USER_SELECTED_NETWORK_CONNECT_RETRY_MAX = 3; // max of 3 retries.
 
     private final Context mContext;
     private final ActivityManager mActivityManager;
@@ -89,6 +91,7 @@ public class WifiNetworkFactory extends NetworkFactory {
     private NetworkRequest mActiveSpecificNetworkRequest;
     private WifiNetworkSpecifier mActiveSpecificNetworkRequestSpecifier;
     private WifiConfiguration mUserSelectedNetwork;
+    private int mUserSelectedNetworkConnectRetryCount;
     private List<ScanResult> mActiveMatchedScanResults;
     // Verbose logging flag.
     private boolean mVerboseLoggingEnabled = false;
@@ -411,6 +414,7 @@ public class WifiNetworkFactory extends NetworkFactory {
                 Log.e(TAG, "Network specifier does not match the active request. Ignoring");
                 return;
             }
+            Log.w(TAG, "App released request, cancelling " + mActiveSpecificNetworkRequest);
             resetStateForActiveRequestEnd();
         }
     }
@@ -447,14 +451,11 @@ public class WifiNetworkFactory extends NetworkFactory {
                 : Process.INVALID_UID;
     }
 
-    private void handleConnectToNetworkUserSelection(WifiConfiguration network) {
-        Log.d(TAG, "User initiated connect to  network: " + network.SSID);
-
-        // Cancel the ongoing scans after user selection.
-        cancelPeriodicScans();
-
-        // Mark the network ephemeral so that it's automatically removed at the end of connection.
-        network.ephemeral = true;
+    // Helper method to trigger a connection request & schedule a timeout alarm to track the
+    // connection request.
+    private void connectToNetwork(@NonNull WifiConfiguration network) {
+        // Cancel connection timeout alarm for any previous connection attempts.
+        cancelConnectionTimeout();
 
         // Send the connect request to ClientModeImpl.
         Message msg = Message.obtain();
@@ -464,15 +465,27 @@ public class WifiNetworkFactory extends NetworkFactory {
         msg.replyTo = mSrcMessenger;
         mWifiInjector.getClientModeImpl().sendMessage(msg);
 
+        // Post an alarm to handle connection timeout.
+        scheduleConnectionTimeout();
+    }
+
+    private void handleConnectToNetworkUserSelection(WifiConfiguration network) {
+        Log.d(TAG, "User initiated connect to  network: " + network.SSID);
+
+        // Cancel the ongoing scans after user selection.
+        cancelPeriodicScans();
+
+        // Mark the network ephemeral so that it's automatically removed at the end of connection.
+        network.ephemeral = true;
         // Store the user selected network.
         mUserSelectedNetwork = network;
 
-        // Post an alarm to handle connection timeout.
-        scheduleConnectionTimeout();
+        // Trigger connection to the network.
+        connectToNetwork(network);
     }
 
     private void handleRejectUserSelection() {
-        Log.w(TAG, "User dismissed notification");
+        Log.w(TAG, "User dismissed notification, cancelling " + mActiveSpecificNetworkRequest);
         resetStateForActiveRequestEnd();
     }
 
@@ -532,7 +545,14 @@ public class WifiNetworkFactory extends NetworkFactory {
             Log.w(TAG, "Connection failed to unknown network " + failedNetwork + ". Ignoring...");
             return;
         }
-        Log.e(TAG, "Failed to Connect to network " + mUserSelectedNetwork);
+        Log.w(TAG, "Failed to connect to network " + mUserSelectedNetwork);
+        if (mUserSelectedNetworkConnectRetryCount++ < USER_SELECTED_NETWORK_CONNECT_RETRY_MAX) {
+            Log.i(TAG, "Retrying connection attempt, attempt# "
+                    + mUserSelectedNetworkConnectRetryCount);
+            connectToNetwork(mUserSelectedNetwork);
+            return;
+        }
+        Log.e(TAG, "Connection failures, cancelling " + mUserSelectedNetwork);
         for (INetworkRequestMatchCallback callback : mRegisteredCallbacks.getCallbacks()) {
             try {
                 callback.onUserSelectionConnectFailure(mUserSelectedNetwork);
@@ -561,6 +581,7 @@ public class WifiNetworkFactory extends NetworkFactory {
         mActiveSpecificNetworkRequest = null;
         mActiveSpecificNetworkRequestSpecifier = null;
         mUserSelectedNetwork = null;
+        mUserSelectedNetworkConnectRetryCount = 0;
         mIsConnectedToUserSelectedNetwork = false;
         cancelPeriodicScans();
         cancelConnectionTimeout();
diff --git a/tests/wifitests/src/com/android/server/wifi/WifiNetworkFactoryTest.java b/tests/wifitests/src/com/android/server/wifi/WifiNetworkFactoryTest.java
index 08b6598e7..1e9c54030 100644
--- a/tests/wifitests/src/com/android/server/wifi/WifiNetworkFactoryTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/WifiNetworkFactoryTest.java
@@ -114,6 +114,7 @@ public class WifiNetworkFactoryTest {
             ArgumentCaptor.forClass(OnAlarmListener.class);
     ArgumentCaptor<OnAlarmListener> mConnectionTimeoutAlarmListenerArgumentCaptor =
             ArgumentCaptor.forClass(OnAlarmListener.class);
+    InOrder mInOrder;
 
     private WifiNetworkFactory mWifiNetworkFactory;
 
@@ -872,14 +873,22 @@ public class WifiNetworkFactoryTest {
 
     /**
      * Verify handling of connection timeout.
+     * The timeouts should trigger connection retries until we hit the max.
      */
     @Test
     public void testNetworkSpecifierHandleConnectionTimeout() throws Exception {
         sendNetworkRequestAndSetupForConnectionStatus();
 
-        // Simulate connection timeout.
-        mConnectionTimeoutAlarmListenerArgumentCaptor.getValue().onAlarm();
+        // Simulate connection timeout beyond the retry limit to trigger the failure handling.
+        for (int i = 0; i <= WifiNetworkFactory.USER_SELECTED_NETWORK_CONNECT_RETRY_MAX; i++) {
+            mConnectionTimeoutAlarmListenerArgumentCaptor.getValue().onAlarm();
+            mLooper.dispatchAll();
+        }
+
+        mInOrder = inOrder(mAlarmManager, mClientModeImpl);
+        validateConnectionRetryAttempts();
 
+        // Fail the request after all the retries are exhausted.
         verify(mNetworkRequestMatchCallback).onAbort();
         // Verify that we sent the connection failure callback.
         verify(mNetworkRequestMatchCallback).onUserSelectionConnectFailure(mSelectedNetwork);
@@ -889,43 +898,61 @@ public class WifiNetworkFactoryTest {
 
     /**
      * Verify handling of connection trigger failure.
+     * The trigger failures should trigger connection retries until we hit the max.
      */
     @Test
     public void testNetworkSpecifierHandleConnectionTriggerFailure() throws Exception {
         Messenger replyToMsgr = sendNetworkRequestAndSetupForConnectionStatus();
 
-        // Send failure message.
-        Message failureMsg = Message.obtain();
-        failureMsg.what = WifiManager.CONNECT_NETWORK_FAILED;
-        replyToMsgr.send(failureMsg);
-        mLooper.dispatchAll();
+        // Send failure message beyond the retry limit to trigger the failure handling.
+        for (int i = 0; i <= WifiNetworkFactory.USER_SELECTED_NETWORK_CONNECT_RETRY_MAX; i++) {
+            Message failureMsg = Message.obtain();
+            failureMsg.what = WifiManager.CONNECT_NETWORK_FAILED;
+            replyToMsgr.send(failureMsg);
+            mLooper.dispatchAll();
+        }
+
+        mInOrder = inOrder(mAlarmManager, mClientModeImpl);
+        validateConnectionRetryAttempts();
 
+        // Fail the request after all the retries are exhausted.
         verify(mNetworkRequestMatchCallback).onAbort();
         // Verify that we sent the connection failure callback.
         verify(mNetworkRequestMatchCallback).onUserSelectionConnectFailure(mSelectedNetwork);
         // verify we canceled the timeout alarm.
-        verify(mAlarmManager).cancel(mConnectionTimeoutAlarmListenerArgumentCaptor.getValue());
+        mInOrder.verify(mAlarmManager).cancel(
+                mConnectionTimeoutAlarmListenerArgumentCaptor.getValue());
         // Verify we reset the network request handling.
         verify(mWifiConnectivityManager).setSpecificNetworkRequestInProgress(false);
     }
 
     /**
      * Verify handling of connection failure.
+     * The connection failures should trigger connection retries until we hit the max.
      */
     @Test
     public void testNetworkSpecifierHandleConnectionFailure() throws Exception {
         sendNetworkRequestAndSetupForConnectionStatus();
 
-        // Send network connection failure indication.
         assertNotNull(mSelectedNetwork);
-        mWifiNetworkFactory.handleConnectionAttemptEnded(
-                WifiMetrics.ConnectionEvent.FAILURE_DHCP, mSelectedNetwork);
+
+        // Send network connection failure indication beyond the retry limit to trigger the failure
+        // handling.
+        for (int i = 0; i <= WifiNetworkFactory.USER_SELECTED_NETWORK_CONNECT_RETRY_MAX; i++) {
+            mWifiNetworkFactory.handleConnectionAttemptEnded(
+                    WifiMetrics.ConnectionEvent.FAILURE_DHCP, mSelectedNetwork);
+            mLooper.dispatchAll();
+        }
+
+        mInOrder = inOrder(mAlarmManager, mClientModeImpl);
+        validateConnectionRetryAttempts();
 
         verify(mNetworkRequestMatchCallback).onAbort();
         // Verify that we sent the connection failure callback.
         verify(mNetworkRequestMatchCallback).onUserSelectionConnectFailure(mSelectedNetwork);
         // verify we canceled the timeout alarm.
-        verify(mAlarmManager).cancel(mConnectionTimeoutAlarmListenerArgumentCaptor.getValue());
+        mInOrder.verify(mAlarmManager).cancel(
+                mConnectionTimeoutAlarmListenerArgumentCaptor.getValue());
         // Verify we reset the network request handling.
         verify(mWifiConnectivityManager).setSpecificNetworkRequestInProgress(false);
     }
@@ -944,7 +971,7 @@ public class WifiNetworkFactoryTest {
         mWifiNetworkFactory.handleConnectionAttemptEnded(
                 WifiMetrics.ConnectionEvent.FAILURE_DHCP, connectedNetwork);
 
-        // Verify that we sent the connection failure callback.
+        // Verify that we did not send the connection failure callback.
         verify(mNetworkRequestMatchCallback, never())
                 .onUserSelectionConnectFailure(mSelectedNetwork);
         // verify we canceled the timeout alarm.
@@ -954,14 +981,22 @@ public class WifiNetworkFactoryTest {
         verify(mWifiConnectivityManager, never())
                 .setSpecificNetworkRequestInProgress(false);
 
-        // Send network connection success to the correct network indication.
-        mWifiNetworkFactory.handleConnectionAttemptEnded(
-                WifiMetrics.ConnectionEvent.FAILURE_DHCP, mSelectedNetwork);
+        // Send network connection failure indication beyond the retry limit to trigger the failure
+        // handling.
+        for (int i = 0; i <= WifiNetworkFactory.USER_SELECTED_NETWORK_CONNECT_RETRY_MAX; i++) {
+            mWifiNetworkFactory.handleConnectionAttemptEnded(
+                    WifiMetrics.ConnectionEvent.FAILURE_DHCP, mSelectedNetwork);
+            mLooper.dispatchAll();
+        }
+
+        mInOrder = inOrder(mAlarmManager, mClientModeImpl);
+        validateConnectionRetryAttempts();
 
         // Verify that we sent the connection failure callback.
         verify(mNetworkRequestMatchCallback).onUserSelectionConnectFailure(mSelectedNetwork);
         // verify we canceled the timeout alarm.
-        verify(mAlarmManager).cancel(mConnectionTimeoutAlarmListenerArgumentCaptor.getValue());
+        mInOrder.verify(mAlarmManager).cancel(
+                mConnectionTimeoutAlarmListenerArgumentCaptor.getValue());
         // Verify we reset the network request handling.
         verify(mWifiConnectivityManager).setSpecificNetworkRequestInProgress(false);
     }
@@ -1290,8 +1325,7 @@ public class WifiNetworkFactoryTest {
                 ArgumentCaptor.forClass(ScanListener.class);
         ScanListener scanListener = null;
 
-        InOrder inOrder = inOrder(mWifiScanner, mAlarmManager);
-
+        mInOrder = inOrder(mWifiScanner, mAlarmManager);
         for (int i = 0; i < expectedIntervalsInSeconds.length - 1; i++) {
             long expectedCurrentIntervalInMs = expectedIntervalsInSeconds[i];
             long expectedNextIntervalInMs = expectedIntervalsInSeconds[i + 1];
@@ -1301,7 +1335,7 @@ public class WifiNetworkFactoryTest {
                 // Fire the alarm and ensure that we started the next scan.
                 alarmListener.onAlarm();
             }
-            inOrder.verify(mWifiScanner).startScan(
+            mInOrder.verify(mWifiScanner).startScan(
                     any(), scanListenerArgumentCaptor.capture(), any());
             scanListener = scanListenerArgumentCaptor.getValue();
             assertNotNull(scanListener);
@@ -1309,7 +1343,7 @@ public class WifiNetworkFactoryTest {
             // Now trigger the scan results callback and verify the alarm set for the next scan.
             scanListener.onResults(mTestScanDatas);
 
-            inOrder.verify(mAlarmManager).set(eq(AlarmManager.ELAPSED_REALTIME_WAKEUP),
+            mInOrder.verify(mAlarmManager).set(eq(AlarmManager.ELAPSED_REALTIME_WAKEUP),
                     eq(expectedNextIntervalInMs), any(),
                     mPeriodicScanListenerArgumentCaptor.capture(), any());
             alarmListener = mPeriodicScanListenerArgumentCaptor.getValue();
@@ -1390,4 +1424,25 @@ public class WifiNetworkFactoryTest {
             ScanTestUtil.assertScanResultEquals(expectedScanResult, actualScanResult);
         }
     }
+
+    private void validateConnectionRetryAttempts() {
+        for (int i = 0; i < WifiNetworkFactory.USER_SELECTED_NETWORK_CONNECT_RETRY_MAX; i++) {
+            // Cancel the existing connection timeout.
+            mInOrder.verify(mAlarmManager).cancel(
+                    mConnectionTimeoutAlarmListenerArgumentCaptor.getValue());
+
+            // Trigger new connection.
+            ArgumentCaptor<Message> messageCaptor = ArgumentCaptor.forClass(Message.class);
+            mInOrder.verify(mClientModeImpl).sendMessage(messageCaptor.capture());
+            Message message = messageCaptor.getValue();
+            assertNotNull(message);
+            assertEquals(WifiManager.CONNECT_NETWORK, message.what);
+
+            // Start the new connection timeout alarm.
+            mInOrder.verify(mAlarmManager).set(eq(AlarmManager.ELAPSED_REALTIME_WAKEUP),
+                    eq((long) WifiNetworkFactory.NETWORK_CONNECTION_TIMEOUT_MS), any(),
+                    mConnectionTimeoutAlarmListenerArgumentCaptor.capture(), any());
+            assertNotNull(mConnectionTimeoutAlarmListenerArgumentCaptor.getValue());
+        }
+    }
 }