SoftApManager should listen for interface up/down

Defer reporting AP bring up as complete until we see the interface
going up.  Since wificond takes down the interface at each tear
down, the interface should always be initially down, so we should
always see this event.

Bug: 31337216
Test: unittests pass

Change-Id: If5d5e3908887df33f7da9a61af1bd648d8b8cc5a

3 files changed, 103 insertions, 7 deletions

diff --git a/service/java/com/android/server/wifi/SoftApManager.java b/service/java/com/android/server/wifi/SoftApManager.java
index 99a6923d0..b1955654b 100644
--- a/service/java/com/android/server/wifi/SoftApManager.java
+++ b/service/java/com/android/server/wifi/SoftApManager.java
@@ -20,11 +20,12 @@ import static com.android.server.wifi.util.ApConfigUtil.ERROR_GENERIC;
 import static com.android.server.wifi.util.ApConfigUtil.ERROR_NO_CHANNEL;
 import static com.android.server.wifi.util.ApConfigUtil.SUCCESS;
 
+import android.net.InterfaceConfiguration;
 import android.net.wifi.IApInterface;
 import android.net.wifi.WifiConfiguration;
-import android.net.wifi.WifiManager;
 import android.net.wifi.WifiConfiguration.KeyMgmt;
-import android.os.IBinder.DeathRecipient;
+import android.net.wifi.WifiManager;
+import android.os.INetworkManagementService;
 import android.os.Looper;
 import android.os.Message;
 import android.os.RemoteException;
@@ -32,6 +33,7 @@ import android.util.Log;
 
 import com.android.internal.util.State;
 import com.android.internal.util.StateMachine;
+import com.android.server.net.BaseNetworkObserver;
 import com.android.server.wifi.util.ApConfigUtil;
 
 import java.nio.charset.StandardCharsets;
@@ -56,6 +58,8 @@ public class SoftApManager {
 
     private final IApInterface mApInterface;
 
+    private final INetworkManagementService mNwService;
+
     /**
      * Listener for soft AP state changes.
      */
@@ -73,7 +77,8 @@ public class SoftApManager {
                          String countryCode,
                          ArrayList<Integer> allowed2GChannels,
                          Listener listener,
-                         IApInterface apInterface) {
+                         IApInterface apInterface,
+                         INetworkManagementService nms) {
         mStateMachine = new SoftApStateMachine(looper);
 
         mWifiNative = wifiNative;
@@ -81,6 +86,7 @@ public class SoftApManager {
         mAllowed2GChannels = allowed2GChannels;
         mListener = listener;
         mApInterface = apInterface;
+        mNwService = nms;
     }
 
     /**
@@ -214,6 +220,7 @@ public class SoftApManager {
         public static final int CMD_START = 0;
         public static final int CMD_STOP = 1;
         public static final int CMD_AP_INTERFACE_BINDER_DEATH = 2;
+        public static final int CMD_INTERFACE_STATUS_CHANGED = 3;
 
         private final State mIdleState = new IdleState();
         private final State mStartedState = new StartedState();
@@ -221,6 +228,24 @@ public class SoftApManager {
         private final StateMachineDeathRecipient mDeathRecipient =
                 new StateMachineDeathRecipient(this, CMD_AP_INTERFACE_BINDER_DEATH);
 
+        private NetworkObserver mNetworkObserver;
+
+        private class NetworkObserver extends BaseNetworkObserver {
+            private final String mIfaceName;
+
+            NetworkObserver(String ifaceName) {
+                mIfaceName = ifaceName;
+            }
+
+            @Override
+            public void interfaceLinkStateChanged(String iface, boolean up) {
+                if (mIfaceName.equals(iface)) {
+                    SoftApStateMachine.this.sendMessage(
+                            CMD_INTERFACE_STATUS_CHANGED, up ? 1 : 0, 0, this);
+                }
+            }
+        }
+
         SoftApStateMachine(Looper looper) {
             super(TAG, looper);
 
@@ -235,6 +260,7 @@ public class SoftApManager {
             @Override
             public void enter() {
                 mDeathRecipient.unlinkToDeath();
+                unregisterObserver();
             }
 
             @Override
@@ -249,6 +275,17 @@ public class SoftApManager {
                             break;
                         }
 
+                        try {
+                            mNetworkObserver = new NetworkObserver(mApInterface.getInterfaceName());
+                            mNwService.registerObserver(mNetworkObserver);
+                        } catch (RemoteException e) {
+                            mDeathRecipient.unlinkToDeath();
+                            unregisterObserver();
+                            updateApState(WifiManager.WIFI_AP_STATE_FAILED,
+                                          WifiManager.SAP_START_FAILURE_GENERAL);
+                            break;
+                        }
+
                         int result = startSoftAp((WifiConfiguration) message.obj);
                         if (result != SUCCESS) {
                             int failureReason = WifiManager.SAP_START_FAILURE_GENERAL;
@@ -256,11 +293,11 @@ public class SoftApManager {
                                 failureReason = WifiManager.SAP_START_FAILURE_NO_CHANNEL;
                             }
                             mDeathRecipient.unlinkToDeath();
+                            unregisterObserver();
                             updateApState(WifiManager.WIFI_AP_STATE_FAILED, failureReason);
                             break;
                         }
 
-                        updateApState(WifiManager.WIFI_AP_STATE_ENABLED, 0);
                         transitionTo(mStartedState);
                         break;
                     default:
@@ -270,12 +307,58 @@ public class SoftApManager {
 
                 return HANDLED;
             }
+
+            private void unregisterObserver() {
+                if (mNetworkObserver == null) {
+                    return;
+                }
+                try {
+                    mNwService.unregisterObserver(mNetworkObserver);
+                } catch (RemoteException e) { }
+                mNetworkObserver = null;
+            }
         }
 
         private class StartedState extends State {
+            private boolean mIfaceIsUp;
+
+            private void onUpChanged(boolean isUp) {
+                if (isUp == mIfaceIsUp) {
+                    return;  // no change
+                }
+                mIfaceIsUp = isUp;
+                if (isUp) {
+                    Log.d(TAG, "SoftAp is ready for use");
+                    updateApState(WifiManager.WIFI_AP_STATE_ENABLED, 0);
+                } else {
+                    // TODO: handle the case where the interface was up, but goes down
+                }
+            }
+
+            @Override
+            public void enter() {
+                mIfaceIsUp = false;
+                InterfaceConfiguration config = null;
+                try {
+                    config = mNwService.getInterfaceConfig(mApInterface.getInterfaceName());
+                } catch (RemoteException e) {
+                }
+                if (config != null) {
+                    onUpChanged(config.isUp());
+                }
+            }
+
             @Override
             public boolean processMessage(Message message) {
                 switch (message.what) {
+                    case CMD_INTERFACE_STATUS_CHANGED:
+                        if (message.obj != mNetworkObserver) {
+                            // This is from some time before the most recent configuration.
+                            break;
+                        }
+                        boolean isUp = message.arg1 == 1;
+                        onUpChanged(isUp);
+                        break;
                     case CMD_START:
                         /* Already started, ignore this command. */
                         break;
diff --git a/service/java/com/android/server/wifi/WifiInjector.java b/service/java/com/android/server/wifi/WifiInjector.java
index 711c03b54..732047624 100644
--- a/service/java/com/android/server/wifi/WifiInjector.java
+++ b/service/java/com/android/server/wifi/WifiInjector.java
@@ -290,7 +290,8 @@ public class WifiInjector {
         return new SoftApManager(
                 mWifiServiceHandlerThread.getLooper(),
                 wifiNative, countryCode,
-                allowed2GChannels, listener, apInterface);
+                allowed2GChannels, listener, apInterface,
+                nmService);
     }
 
     /**
diff --git a/tests/wifitests/src/com/android/server/wifi/SoftApManagerTest.java b/tests/wifitests/src/com/android/server/wifi/SoftApManagerTest.java
index 2cf2398bf..25fbb4210 100644
--- a/tests/wifitests/src/com/android/server/wifi/SoftApManagerTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/SoftApManagerTest.java
@@ -31,9 +31,12 @@ import android.net.wifi.WifiConfiguration;
 import android.net.wifi.WifiManager;
 import android.os.IBinder;
 import android.os.IBinder.DeathRecipient;
+import android.os.INetworkManagementService;
 import android.os.test.TestLooper;
 import android.test.suitebuilder.annotation.SmallTest;
 
+import com.android.server.net.BaseNetworkObserver;
+
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.ArgumentCaptor;
@@ -54,6 +57,7 @@ public class SoftApManagerTest {
     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};
+    private static final String TEST_INTERFACE_NAME = "testif0";
 
     private final ArrayList<Integer> mAllowed2GChannels =
             new ArrayList<>(Arrays.asList(ALLOWED_2G_CHANNELS));
@@ -64,8 +68,11 @@ public class SoftApManagerTest {
     @Mock InterfaceConfiguration mInterfaceConfiguration;
     @Mock IBinder mApInterfaceBinder;
     @Mock IApInterface mApInterface;
+    @Mock INetworkManagementService mNmService;
     final ArgumentCaptor<DeathRecipient> mDeathListenerCaptor =
             ArgumentCaptor.forClass(DeathRecipient.class);
+    final ArgumentCaptor<BaseNetworkObserver> mNetworkObserverCaptor =
+            ArgumentCaptor.forClass(BaseNetworkObserver.class);
 
     SoftApManager mSoftApManager;
 
@@ -80,13 +87,15 @@ public class SoftApManagerTest {
         when(mApInterface.stopHostapd()).thenReturn(true);
         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);
+                                           mApInterface,
+                                           mNmService);
 
         mLooper.dispatchAll();
     }
@@ -142,7 +151,7 @@ public class SoftApManagerTest {
 
     /** Starts soft AP and verifies that it is enabled successfully. */
     protected void startSoftApAndVerifyEnabled() throws Exception {
-        InOrder order = inOrder(mListener, mApInterfaceBinder, mApInterface);
+        InOrder order = inOrder(mListener, mApInterfaceBinder, mApInterface, mNmService);
 
         /**
          *  Only test the default configuration. Testing for different configurations
@@ -158,9 +167,12 @@ public class SoftApManagerTest {
         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());
         order.verify(mApInterface).startHostapd();
+        mNetworkObserverCaptor.getValue().interfaceLinkStateChanged(TEST_INTERFACE_NAME, true);
+        mLooper.dispatchAll();
         order.verify(mListener).onStateChanged(WifiManager.WIFI_AP_STATE_ENABLED, 0);
     }