Remove shouldDisconnect and redundant transition

This removes the redundant state transition upon
START_CONNECT request.
This also removes the shouldDisconnect parameter
because supplicant will implicitly disconnect current
connected network anyway upon a connect request.

Bug: 36535549
Test: compile, unit tests, integration test

Change-Id: Ib636eb05e9b37e54deeeb1f107dd47da7fe75ad0

4 files changed, 37 insertions, 63 deletions

diff --git a/service/java/com/android/server/wifi/SupplicantStaIfaceHal.java b/service/java/com/android/server/wifi/SupplicantStaIfaceHal.java
index 0fdcebe7e..2432db6be 100644
--- a/service/java/com/android/server/wifi/SupplicantStaIfaceHal.java
+++ b/service/java/com/android/server/wifi/SupplicantStaIfaceHal.java
@@ -385,18 +385,12 @@ public class SupplicantStaIfaceHal {
      * 5. Select the new network in wpa_supplicant.
      *
      * @param config WifiConfiguration parameters for the provided network.
-     * @param shouldDisconnect whether to trigger a disconnection or not.
      * @return {@code true} if it succeeds, {@code false} otherwise
      */
-    public boolean connectToNetwork(WifiConfiguration config, boolean shouldDisconnect) {
+    public boolean connectToNetwork(WifiConfiguration config) {
         mFrameworkNetworkId = WifiConfiguration.INVALID_NETWORK_ID;
         mCurrentNetwork = null;
-        logd("connectToNetwork " + config.configKey()
-                + " (shouldDisconnect " + shouldDisconnect + ")");
-        if (shouldDisconnect && !disconnect()) {
-            loge("Failed to trigger disconnect");
-            return false;
-        }
+        logd("connectToNetwork " + config.configKey());
         if (!removeAllNetworks()) {
             loge("Failed to remove existing networks");
             return false;
@@ -430,7 +424,7 @@ public class SupplicantStaIfaceHal {
         if (mFrameworkNetworkId != config.networkId || mCurrentNetwork == null) {
             Log.w(TAG, "Cannot roam to a different network, initiate new connection. "
                     + "Current network ID: " + mFrameworkNetworkId);
-            return connectToNetwork(config, false);
+            return connectToNetwork(config);
         }
         String bssid = config.getNetworkSelectionStatus().getNetworkSelectionBSSID();
         logd("roamToNetwork" + config.configKey() + " (bssid " + bssid + ")");
diff --git a/service/java/com/android/server/wifi/WifiNative.java b/service/java/com/android/server/wifi/WifiNative.java
index 68da5fbc4..cae705aff 100644
--- a/service/java/com/android/server/wifi/WifiNative.java
+++ b/service/java/com/android/server/wifi/WifiNative.java
@@ -673,19 +673,17 @@ public class WifiNative {
     /**
      * Add the provided network configuration to wpa_supplicant and initiate connection to it.
      * This method does the following:
-     * 1. Triggers disconnect command to wpa_supplicant (if |shouldDisconnect| is true).
-     * 2. Remove any existing network in wpa_supplicant.
-     * 3. Add a new network to wpa_supplicant.
-     * 4. Save the provided configuration to wpa_supplicant.
-     * 5. Select the new network in wpa_supplicant.
-     * 6. Triggers reconnect command to wpa_supplicant.
+     * 1. Remove any existing network in wpa_supplicant(This implicitly triggers disconnect).
+     * 2. Add a new network to wpa_supplicant.
+     * 3. Save the provided configuration to wpa_supplicant.
+     * 4. Select the new network in wpa_supplicant.
+     * 5. Triggers reconnect command to wpa_supplicant.
      *
      * @param configuration WifiConfiguration parameters for the provided network.
-     * @param shouldDisconnect whether to trigger a disconnection or not.
      * @return {@code true} if it succeeds, {@code false} otherwise
      */
-    public boolean connectToNetwork(WifiConfiguration configuration, boolean shouldDisconnect) {
-        return mSupplicantStaIfaceHal.connectToNetwork(configuration, shouldDisconnect);
+    public boolean connectToNetwork(WifiConfiguration configuration) {
+        return mSupplicantStaIfaceHal.connectToNetwork(configuration);
     }
 
     /**
diff --git a/service/java/com/android/server/wifi/WifiStateMachine.java b/service/java/com/android/server/wifi/WifiStateMachine.java
index 5c5ace5b9..50c8f4ef8 100644
--- a/service/java/com/android/server/wifi/WifiStateMachine.java
+++ b/service/java/com/android/server/wifi/WifiStateMachine.java
@@ -5011,17 +5011,14 @@ public class WifiStateMachine extends StateMachine implements WifiNative.WifiRss
 
                     reportConnectionAttemptStart(config, mTargetRoamBSSID,
                             WifiMetricsProto.ConnectionEvent.ROAM_UNRELATED);
-                    boolean shouldDisconnect = (getCurrentState() != mDisconnectedState);
-                    if (mWifiNative.connectToNetwork(config, shouldDisconnect)) {
+                    if (mWifiNative.connectToNetwork(config)) {
                         lastConnectAttemptTimestamp = mClock.getWallClockMillis();
                         targetWificonfiguration = config;
                         mAutoRoaming = false;
                         if (isRoaming() || isLinkDebouncing()) {
                             transitionTo(mRoamingState);
-                        } else if (shouldDisconnect) {
+                        } else if (getCurrentState() != mDisconnectedState) {
                             transitionTo(mDisconnectingState);
-                        } else {
-                            transitionTo(mDisconnectedState);
                         }
                     } else {
                         loge("CMD_START_CONNECT Failed to start connection to network " + config);
diff --git a/tests/wifitests/src/com/android/server/wifi/SupplicantStaIfaceHalTest.java b/tests/wifitests/src/com/android/server/wifi/SupplicantStaIfaceHalTest.java
index de96a7a2a..abf533d1f 100644
--- a/tests/wifitests/src/com/android/server/wifi/SupplicantStaIfaceHalTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/SupplicantStaIfaceHalTest.java
@@ -448,30 +448,21 @@ public class SupplicantStaIfaceHalTest {
     }
 
     /**
-     * Tests connection to a specified network without triggering disconnect.
+     * Tests connection to a specified network with empty existing network.
      */
     @Test
-    public void testConnectWithNoDisconnectAndEmptyExistingNetworks() throws Exception {
+    public void testConnectWithEmptyExistingNetwork() throws Exception {
         executeAndValidateInitializationSequence();
-        executeAndValidateConnectSequence(0, false, false);
+        executeAndValidateConnectSequence(0, false);
     }
 
     /**
-     * Tests connection to a specified network without triggering disconnect.
+     * Tests connection to a specified network with single existing network.
      */
     @Test
-    public void testConnectWithNoDisconnectAndSingleExistingNetwork() throws Exception {
+    public void testConnectWithSingleExistingNetwork() throws Exception {
         executeAndValidateInitializationSequence();
-        executeAndValidateConnectSequence(0, true, false);
-    }
-
-    /**
-     * Tests connection to a specified network, with a triggered disconnect.
-     */
-    @Test
-    public void testConnectWithDisconnectAndSingleExistingNetwork() throws Exception {
-        executeAndValidateInitializationSequence();
-        executeAndValidateConnectSequence(0, false, true);
+        executeAndValidateConnectSequence(0, true);
     }
 
     /**
@@ -489,7 +480,7 @@ public class SupplicantStaIfaceHalTest {
         }).when(mISupplicantStaIfaceMock).addNetwork(
                 any(ISupplicantStaIface.addNetworkCallback.class));
 
-        assertFalse(mDut.connectToNetwork(new WifiConfiguration(), false));
+        assertFalse(mDut.connectToNetwork(new WifiConfiguration()));
     }
 
     /**
@@ -503,7 +494,7 @@ public class SupplicantStaIfaceHalTest {
         when(mSupplicantStaNetworkMock.saveWifiConfiguration(any(WifiConfiguration.class)))
                 .thenReturn(false);
 
-        assertFalse(mDut.connectToNetwork(new WifiConfiguration(), false));
+        assertFalse(mDut.connectToNetwork(new WifiConfiguration()));
         // We should have removed the existing network once before connection and once more
         // on failure to save network configuration.
         verify(mISupplicantStaIfaceMock, times(2)).removeNetwork(anyInt());
@@ -519,7 +510,7 @@ public class SupplicantStaIfaceHalTest {
 
         when(mSupplicantStaNetworkMock.select()).thenReturn(false);
 
-        assertFalse(mDut.connectToNetwork(new WifiConfiguration(), false));
+        assertFalse(mDut.connectToNetwork(new WifiConfiguration()));
     }
 
     /**
@@ -547,7 +538,7 @@ public class SupplicantStaIfaceHalTest {
     public void testRoamFailureDueToBssidSet() throws Exception {
         executeAndValidateInitializationSequence();
         int connectedNetworkId = 5;
-        executeAndValidateConnectSequence(connectedNetworkId, false, false);
+        executeAndValidateConnectSequence(connectedNetworkId, false);
         when(mSupplicantStaNetworkMock.setBssid(anyString())).thenReturn(false);
 
         WifiConfiguration roamingConfig = new WifiConfiguration();
@@ -586,7 +577,7 @@ public class SupplicantStaIfaceHalTest {
     public void testRoamFailureDueToReassociate() throws Exception {
         executeAndValidateInitializationSequence();
         int connectedNetworkId = 5;
-        executeAndValidateConnectSequence(connectedNetworkId, false, false);
+        executeAndValidateConnectSequence(connectedNetworkId, false);
 
         doAnswer(new MockAnswerUtil.AnswerWithArguments() {
             public SupplicantStatus answer() throws RemoteException {
@@ -613,7 +604,7 @@ public class SupplicantStaIfaceHalTest {
         // Return null when not connected to the network.
         assertTrue(mDut.getCurrentNetworkWpsNfcConfigurationToken() == null);
         verify(mSupplicantStaNetworkMock, never()).getWpsNfcConfigurationToken();
-        executeAndValidateConnectSequence(4, false, false);
+        executeAndValidateConnectSequence(4, false);
         assertEquals(token, mDut.getCurrentNetworkWpsNfcConfigurationToken());
         verify(mSupplicantStaNetworkMock).getWpsNfcConfigurationToken();
     }
@@ -630,7 +621,7 @@ public class SupplicantStaIfaceHalTest {
         // Fail when not connected to a network.
         assertFalse(mDut.setCurrentNetworkBssid(bssidStr));
         verify(mSupplicantStaNetworkMock, never()).setBssid(eq(bssidStr));
-        executeAndValidateConnectSequence(4, false, false);
+        executeAndValidateConnectSequence(4, false);
         assertTrue(mDut.setCurrentNetworkBssid(bssidStr));
         verify(mSupplicantStaNetworkMock).setBssid(eq(bssidStr));
     }
@@ -648,7 +639,7 @@ public class SupplicantStaIfaceHalTest {
         // Fail when not connected to a network.
         assertFalse(mDut.sendCurrentNetworkEapIdentityResponse(identity));
         verify(mSupplicantStaNetworkMock, never()).sendNetworkEapIdentityResponse(eq(identity));
-        executeAndValidateConnectSequence(4, false, false);
+        executeAndValidateConnectSequence(4, false);
         assertTrue(mDut.sendCurrentNetworkEapIdentityResponse(identity));
         verify(mSupplicantStaNetworkMock).sendNetworkEapIdentityResponse(eq(identity));
     }
@@ -666,7 +657,7 @@ public class SupplicantStaIfaceHalTest {
         // Fail when not connected to a network.
         assertFalse(mDut.sendCurrentNetworkEapSimGsmAuthResponse(params));
         verify(mSupplicantStaNetworkMock, never()).sendNetworkEapSimGsmAuthResponse(eq(params));
-        executeAndValidateConnectSequence(4, false, false);
+        executeAndValidateConnectSequence(4, false);
         assertTrue(mDut.sendCurrentNetworkEapSimGsmAuthResponse(params));
         verify(mSupplicantStaNetworkMock).sendNetworkEapSimGsmAuthResponse(eq(params));
     }
@@ -684,7 +675,7 @@ public class SupplicantStaIfaceHalTest {
         // Fail when not connected to a network.
         assertFalse(mDut.sendCurrentNetworkEapSimUmtsAuthResponse(params));
         verify(mSupplicantStaNetworkMock, never()).sendNetworkEapSimUmtsAuthResponse(eq(params));
-        executeAndValidateConnectSequence(4, false, false);
+        executeAndValidateConnectSequence(4, false);
         assertTrue(mDut.sendCurrentNetworkEapSimUmtsAuthResponse(params));
         verify(mSupplicantStaNetworkMock).sendNetworkEapSimUmtsAuthResponse(eq(params));
     }
@@ -702,7 +693,7 @@ public class SupplicantStaIfaceHalTest {
         // Fail when not connected to a network.
         assertFalse(mDut.sendCurrentNetworkEapSimUmtsAutsResponse(params));
         verify(mSupplicantStaNetworkMock, never()).sendNetworkEapSimUmtsAutsResponse(eq(params));
-        executeAndValidateConnectSequence(4, false, false);
+        executeAndValidateConnectSequence(4, false);
         assertTrue(mDut.sendCurrentNetworkEapSimUmtsAutsResponse(params));
         verify(mSupplicantStaNetworkMock).sendNetworkEapSimUmtsAutsResponse(eq(params));
     }
@@ -870,7 +861,7 @@ public class SupplicantStaIfaceHalTest {
     public void testStateChangeToAssociatedCallback() throws Exception {
         executeAndValidateInitializationSequence();
         int frameworkNetworkId = 6;
-        executeAndValidateConnectSequence(frameworkNetworkId, false, false);
+        executeAndValidateConnectSequence(frameworkNetworkId, false);
         assertNotNull(mISupplicantStaIfaceCallback);
 
         mISupplicantStaIfaceCallback.onStateChanged(
@@ -890,7 +881,7 @@ public class SupplicantStaIfaceHalTest {
     public void testStateChangeToCompletedCallback() throws Exception {
         executeAndValidateInitializationSequence();
         int frameworkNetworkId = 6;
-        executeAndValidateConnectSequence(frameworkNetworkId, false, false);
+        executeAndValidateConnectSequence(frameworkNetworkId, false);
         assertNotNull(mISupplicantStaIfaceCallback);
 
         mISupplicantStaIfaceCallback.onStateChanged(
@@ -1427,11 +1418,7 @@ public class SupplicantStaIfaceHalTest {
      * Helper function to validate the connect sequence.
      */
     private void validateConnectSequence(
-            final boolean haveExistingNetwork, boolean shouldDisconnect, int numNetworkAdditions)
-            throws Exception {
-        if (shouldDisconnect) {
-            verify(mISupplicantStaIfaceMock).disconnect();
-        }
+            final boolean haveExistingNetwork, int numNetworkAdditions) throws Exception {
         if (haveExistingNetwork) {
             verify(mISupplicantStaIfaceMock).removeNetwork(anyInt());
         }
@@ -1447,16 +1434,14 @@ public class SupplicantStaIfaceHalTest {
      *
      * @param newFrameworkNetworkId Framework Network Id of the new network to connect.
      * @param haveExistingNetwork Removes the existing network.
-     * @param shouldDisconnect Should trigger disconnect before connecting.
      */
     private void executeAndValidateConnectSequence(
-            final int newFrameworkNetworkId, final boolean haveExistingNetwork,
-            boolean shouldDisconnect) throws Exception {
+            final int newFrameworkNetworkId, final boolean haveExistingNetwork) throws Exception {
         setupMocksForConnectSequence(haveExistingNetwork);
         WifiConfiguration config = new WifiConfiguration();
         config.networkId = newFrameworkNetworkId;
-        assertTrue(mDut.connectToNetwork(config, shouldDisconnect));
-        validateConnectSequence(haveExistingNetwork, shouldDisconnect, 1);
+        assertTrue(mDut.connectToNetwork(config));
+        validateConnectSequence(haveExistingNetwork, 1);
     }
 
     /**
@@ -1485,7 +1470,7 @@ public class SupplicantStaIfaceHalTest {
         } else {
             roamNetworkId = connectedNetworkId + 1;
         }
-        executeAndValidateConnectSequence(connectedNetworkId, false, true);
+        executeAndValidateConnectSequence(connectedNetworkId, false);
         setupMocksForRoamSequence(roamBssid);
 
         WifiConfiguration roamingConfig = new WifiConfiguration();
@@ -1494,7 +1479,7 @@ public class SupplicantStaIfaceHalTest {
         assertTrue(mDut.roamToNetwork(roamingConfig));
 
         if (!sameNetwork) {
-            validateConnectSequence(false, false, 2);
+            validateConnectSequence(false, 2);
             verify(mSupplicantStaNetworkMock, never()).setBssid(anyString());
             verify(mISupplicantStaIfaceMock, never()).reassociate();
         } else {