SoftApManager: move config argument to constructor

This CL moves the softap configuration argument from the start command
to the constructor.  This allows a generic start() method to be used
instead of sending the config with the CMD_START message.  Additionally,
the default config will be used if a config is not provided.  An error
will be triggered if the default config is not loaded properly.

Existing unit tests were updated to reflect the changes above and
additional tests were added to cover a failure loading the default
config.  The tests were also modified to check the SSID name from the
config when start is called.  This allows the test to distinguish the
default from provided configs in the constructor.

Bug: 31631674
Test: manual testing and runtests.sh
Change-Id: I3be2ee36eb8119647fc42623785e1bbd4c7ad9b1

4 files changed, 95 insertions, 60 deletions

diff --git a/service/java/com/android/server/wifi/SoftApManager.java b/service/java/com/android/server/wifi/SoftApManager.java
index b1955654b..bb18a8476 100644
--- a/service/java/com/android/server/wifi/SoftApManager.java
+++ b/service/java/com/android/server/wifi/SoftApManager.java
@@ -37,7 +37,6 @@ import com.android.server.net.BaseNetworkObserver;
 import com.android.server.wifi.util.ApConfigUtil;
 
 import java.nio.charset.StandardCharsets;
-import java.util.ArrayList;
 import java.util.Locale;
 
 /**
@@ -48,7 +47,6 @@ public class SoftApManager {
     private static final String TAG = "SoftApManager";
 
     private final WifiNative mWifiNative;
-    private final ArrayList<Integer> mAllowed2GChannels;
 
     private final String mCountryCode;
 
@@ -59,6 +57,7 @@ public class SoftApManager {
     private final IApInterface mApInterface;
 
     private final INetworkManagementService mNwService;
+    private final WifiApConfigStore mWifiApConfigStore;
 
     /**
      * Listener for soft AP state changes.
@@ -75,26 +74,30 @@ public class SoftApManager {
     public SoftApManager(Looper looper,
                          WifiNative wifiNative,
                          String countryCode,
-                         ArrayList<Integer> allowed2GChannels,
                          Listener listener,
                          IApInterface apInterface,
-                         INetworkManagementService nms) {
+                         INetworkManagementService nms,
+                         WifiApConfigStore wifiApConfigStore,
+                         WifiConfiguration config) {
         mStateMachine = new SoftApStateMachine(looper);
 
         mWifiNative = wifiNative;
         mCountryCode = countryCode;
-        mAllowed2GChannels = allowed2GChannels;
         mListener = listener;
         mApInterface = apInterface;
         mNwService = nms;
+        mWifiApConfigStore = wifiApConfigStore;
+        if (config != null) {
+            mWifiApConfigStore.setApConfiguration(config);
+        }
     }
 
     /**
-     * Start soft AP with given configuration.
-     * @param config AP configuration
+     * Start soft AP with the current saved config.
      */
-    public void start(WifiConfiguration config) {
-        mStateMachine.sendMessage(SoftApStateMachine.CMD_START, config);
+    public void start() {
+        mStateMachine.sendMessage(SoftApStateMachine.CMD_START,
+                                  mWifiApConfigStore.getApConfiguration());
     }
 
     /**
@@ -126,22 +129,21 @@ public class SoftApManager {
             return ERROR_GENERIC;
         }
 
-        /* Make a copy of configuration for updating AP band and channel. */
+        // Make a copy of configuration for updating AP band and channel.
         WifiConfiguration localConfig = new WifiConfiguration(config);
 
         int result = ApConfigUtil.updateApChannelConfig(
-                mWifiNative, mCountryCode, mAllowed2GChannels, localConfig);
+                mWifiNative, mCountryCode,
+                mWifiApConfigStore.getAllowed2GChannel(), localConfig);
         if (result != SUCCESS) {
             Log.e(TAG, "Failed to update AP band and channel");
             return result;
         }
 
-        /* Setup country code if it is provide. */
+        // Setup country code if it is provided.
         if (mCountryCode != null) {
-            /**
-             * Country code is mandatory for 5GHz band, return an error if failed to set
-             * country code when AP is configured for 5GHz band.
-             */
+            // Country code is mandatory for 5GHz band, return an error if failed to set
+            // country code when AP is configured for 5GHz band.
             if (!mWifiNative.setCountryCodeHal(mCountryCode.toUpperCase(Locale.ROOT))
                     && config.apBand == WifiConfiguration.AP_BAND_5GHZ) {
                 Log.e(TAG, "Failed to set country code, required for setting up "
@@ -216,7 +218,7 @@ public class SoftApManager {
     }
 
     private class SoftApStateMachine extends StateMachine {
-        /* Commands for the state machine. */
+        // Commands for the state machine.
         public static final int CMD_START = 0;
         public static final int CMD_STOP = 1;
         public static final int CMD_AP_INTERFACE_BINDER_DEATH = 2;
@@ -301,7 +303,7 @@ public class SoftApManager {
                         transitionTo(mStartedState);
                         break;
                     default:
-                        /* Ignore all other commands. */
+                        // Ignore all other commands.
                         break;
                 }
 
@@ -360,7 +362,7 @@ public class SoftApManager {
                         onUpChanged(isUp);
                         break;
                     case CMD_START:
-                        /* Already started, ignore this command. */
+                        // Already started, ignore this command.
                         break;
                     case CMD_AP_INTERFACE_BINDER_DEATH:
                     case CMD_STOP:
diff --git a/service/java/com/android/server/wifi/WifiInjector.java b/service/java/com/android/server/wifi/WifiInjector.java
index 28c56b5bd..11d10f5f7 100644
--- a/service/java/com/android/server/wifi/WifiInjector.java
+++ b/service/java/com/android/server/wifi/WifiInjector.java
@@ -20,6 +20,7 @@ import android.content.Context;
 import android.net.wifi.IApInterface;
 import android.net.wifi.IWifiScanner;
 import android.net.wifi.IWificond;
+import android.net.wifi.WifiConfiguration;
 import android.net.wifi.WifiScanner;
 import android.os.HandlerThread;
 import android.os.IBinder;
@@ -287,15 +288,17 @@ public class WifiInjector {
      * changes
      * @param listener listener for SoftApManager
      * @param apInterface network interface to start hostapd against
+     * @param config softAp WifiConfiguration
      * @return an instance of SoftApManager
      */
     public SoftApManager makeSoftApManager(INetworkManagementService nmService,
                                            SoftApManager.Listener listener,
-                                           IApInterface apInterface) {
+                                           IApInterface apInterface,
+                                           WifiConfiguration config) {
         return new SoftApManager(mWifiServiceHandlerThread.getLooper(),
                                  mWifiNative, mCountryCode.getCountryCode(),
-                                 mWifiApConfigStore.getAllowed2GChannel(),
-                                 listener, apInterface, nmService);
+                                 listener, apInterface, nmService,
+                                 mWifiApConfigStore, config);
     }
 
     /**
diff --git a/service/java/com/android/server/wifi/WifiStateMachine.java b/service/java/com/android/server/wifi/WifiStateMachine.java
index ff8e6e3b4..859bbb241 100644
--- a/service/java/com/android/server/wifi/WifiStateMachine.java
+++ b/service/java/com/android/server/wifi/WifiStateMachine.java
@@ -6641,22 +6641,13 @@ public class WifiStateMachine extends StateMachine implements WifiNative.WifiRss
             }
 
             WifiConfiguration config = (WifiConfiguration) message.obj;
-            if (config == null) {
-                /**
-                 * Configuration not provided in the command, fallback to use the current
-                 * configuration.
-                 */
-                config = mWifiApConfigStore.getApConfiguration();
-            } else {
-                /* Update AP configuration. */
-                mWifiApConfigStore.setApConfiguration(config);
-            }
 
             checkAndSetConnectivityInstance();
             mSoftApManager = mWifiInjector.makeSoftApManager(mNwService,
                                                              new SoftApListener(),
-                                                             apInterface);
-            mSoftApManager.start(config);
+                                                             apInterface,
+                                                             config);
+            mSoftApManager.start();
         }
 
         @Override
diff --git a/tests/wifitests/src/com/android/server/wifi/SoftApManagerTest.java b/tests/wifitests/src/com/android/server/wifi/SoftApManagerTest.java
index 25fbb4210..12370b0f8 100644
--- a/tests/wifitests/src/com/android/server/wifi/SoftApManagerTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/SoftApManagerTest.java
@@ -44,6 +44,7 @@ import org.mockito.InOrder;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 
+import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Locale;
@@ -54,6 +55,7 @@ public class SoftApManagerTest {
 
     private static final String TAG = "SoftApManagerTest";
 
+    private static final String DEFAULT_SSID = "DefaultTestSSID";
     private static final String TEST_SSID = "TestSSID";
     private static final String TEST_COUNTRY_CODE = "TestCountry";
     private static final Integer[] ALLOWED_2G_CHANNELS = {1, 2, 3, 4};
@@ -62,6 +64,8 @@ public class SoftApManagerTest {
     private final ArrayList<Integer> mAllowed2GChannels =
             new ArrayList<>(Arrays.asList(ALLOWED_2G_CHANNELS));
 
+    private final WifiConfiguration mDefaultApConfig = createDefaultApConfig();
+
     TestLooper mLooper;
     @Mock WifiNative mWifiNative;
     @Mock SoftApManager.Listener mListener;
@@ -69,6 +73,7 @@ public class SoftApManagerTest {
     @Mock IBinder mApInterfaceBinder;
     @Mock IApInterface mApInterface;
     @Mock INetworkManagementService mNmService;
+    @Mock WifiApConfigStore mWifiApConfigStore;
     final ArgumentCaptor<DeathRecipient> mDeathListenerCaptor =
             ArgumentCaptor.forClass(DeathRecipient.class);
     final ArgumentCaptor<BaseNetworkObserver> mNetworkObserverCaptor =
@@ -88,34 +93,64 @@ public class SoftApManagerTest {
         when(mApInterface.writeHostapdConfig(
                 any(), anyBoolean(), anyInt(), anyInt(), any())).thenReturn(true);
         when(mApInterface.getInterfaceName()).thenReturn(TEST_INTERFACE_NAME);
+    }
 
-        mSoftApManager = new SoftApManager(mLooper.getLooper(),
-                                           mWifiNative,
-                                           TEST_COUNTRY_CODE,
-                                           mAllowed2GChannels,
-                                           mListener,
-                                           mApInterface,
-                                           mNmService);
+    private WifiConfiguration createDefaultApConfig() {
+        WifiConfiguration defaultConfig = new WifiConfiguration();
+        defaultConfig.SSID = DEFAULT_SSID;
+        return defaultConfig;
+    }
 
+    private SoftApManager createSoftApManager(WifiConfiguration config) throws Exception {
+        when(mApInterface.asBinder()).thenReturn(mApInterfaceBinder);
+        when(mApInterface.startHostapd()).thenReturn(true);
+        when(mApInterface.stopHostapd()).thenReturn(true);
+        SoftApManager newSoftApManager = new SoftApManager(mLooper.getLooper(),
+                                                           mWifiNative,
+                                                           TEST_COUNTRY_CODE,
+                                                           mListener,
+                                                           mApInterface,
+                                                           mNmService,
+                                                           mWifiApConfigStore,
+                                                           config);
         mLooper.dispatchAll();
+        if (config != null) {
+            verify(mWifiApConfigStore).setApConfiguration(config);
+        }
+        return newSoftApManager;
     }
 
-    /** Verifies startSoftAp will fail if AP configuration is not provided. */
+    /** Verifies startSoftAp will use default config if AP configuration is not provided. */
     @Test
     public void startSoftApWithoutConfig() throws Exception {
-        InOrder order = inOrder(mListener);
+        startSoftApAndVerifyEnabled(null);
+    }
 
-        mSoftApManager.start(null);
-        mLooper.dispatchAll();
+    /** Verifies startSoftAp will use provided config and start AP. */
+    @Test
+    public void startSoftApWithConfig() throws Exception {
+        WifiConfiguration config = new WifiConfiguration();
+        config.apBand = WifiConfiguration.AP_BAND_2GHZ;
+        config.SSID = TEST_SSID;
+        startSoftApAndVerifyEnabled(config);
+    }
 
-        order.verify(mListener).onStateChanged(WifiManager.WIFI_AP_STATE_ENABLING, 0);
-        order.verify(mListener).onStateChanged(
-                WifiManager.WIFI_AP_STATE_FAILED, WifiManager.SAP_START_FAILURE_GENERAL);
+    /** Tests softap startup if default config fails to load. **/
+    @Test
+    public void startSoftApDefaultConfigFailedToLoad() throws Exception {
+        InOrder order = inOrder(mListener);
+        mSoftApManager = createSoftApManager(null);
+        when(mWifiApConfigStore.getApConfiguration()).thenReturn(null);
+        mSoftApManager.start();
+        mLooper.dispatchAll();
+        verify(mListener).onStateChanged(WifiManager.WIFI_AP_STATE_FAILED,
+                WifiManager.SAP_START_FAILURE_GENERAL);
     }
 
     /** Tests the handling of stop command when soft AP is not started. */
     @Test
     public void stopWhenNotStarted() throws Exception {
+        mSoftApManager = createSoftApManager(null);
         mSoftApManager.stop();
         mLooper.dispatchAll();
         /* Verify no state changes. */
@@ -125,7 +160,7 @@ public class SoftApManagerTest {
     /** Tests the handling of stop command when soft AP is started. */
     @Test
     public void stopWhenStarted() throws Exception {
-        startSoftApAndVerifyEnabled();
+        startSoftApAndVerifyEnabled(null);
 
         InOrder order = inOrder(mListener);
 
@@ -139,7 +174,7 @@ public class SoftApManagerTest {
 
     @Test
     public void handlesWificondInterfaceDeath() throws Exception {
-        startSoftApAndVerifyEnabled();
+        startSoftApAndVerifyEnabled(null);
 
         mDeathListenerCaptor.getValue().binderDied();
         mLooper.dispatchAll();
@@ -150,26 +185,30 @@ public class SoftApManagerTest {
     }
 
     /** Starts soft AP and verifies that it is enabled successfully. */
-    protected void startSoftApAndVerifyEnabled() throws Exception {
+    protected void startSoftApAndVerifyEnabled(WifiConfiguration config) throws Exception {
+        String expectedSSID;
         InOrder order = inOrder(mListener, mApInterfaceBinder, mApInterface, mNmService);
 
-        /**
-         *  Only test the default configuration. Testing for different configurations
-         *  are taken care of by ApConfigUtilTest.
-         */
-        WifiConfiguration config = new WifiConfiguration();
-        config.apBand = WifiConfiguration.AP_BAND_2GHZ;
-        config.SSID = TEST_SSID;
         when(mWifiNative.isHalStarted()).thenReturn(false);
         when(mWifiNative.setCountryCodeHal(TEST_COUNTRY_CODE.toUpperCase(Locale.ROOT)))
                 .thenReturn(true);
-        mSoftApManager.start(config);
+
+        mSoftApManager = createSoftApManager(config);
+        if (config == null) {
+            when(mWifiApConfigStore.getApConfiguration()).thenReturn(mDefaultApConfig);
+            expectedSSID = mDefaultApConfig.SSID;
+        } else {
+            when(mWifiApConfigStore.getApConfiguration()).thenReturn(config);
+            expectedSSID = config.SSID;
+        }
+        mSoftApManager.start();
         mLooper.dispatchAll();
         order.verify(mListener).onStateChanged(WifiManager.WIFI_AP_STATE_ENABLING, 0);
         order.verify(mApInterfaceBinder).linkToDeath(mDeathListenerCaptor.capture(), eq(0));
         order.verify(mNmService).registerObserver(mNetworkObserverCaptor.capture());
         order.verify(mApInterface).writeHostapdConfig(
-                any(), anyBoolean(), anyInt(), anyInt(), any());
+                eq(expectedSSID.getBytes(StandardCharsets.UTF_8)), anyBoolean(),
+                anyInt(), anyInt(), any());
         order.verify(mApInterface).startHostapd();
         mNetworkObserverCaptor.getValue().interfaceLinkStateChanged(TEST_INTERFACE_NAME, true);
         mLooper.dispatchAll();