Switch to the two-argument version of explicitlySelected.

am: 7cbe8435dc

Change-Id: I6ecc3c5d0f2f8b45bb8d74e9bc0c7f7b6015dc53

2 files changed, 140 insertions, 52 deletions

diff --git a/service/java/com/android/server/wifi/ClientModeImpl.java b/service/java/com/android/server/wifi/ClientModeImpl.java
index bb1230554..ca00b572e 100644
--- a/service/java/com/android/server/wifi/ClientModeImpl.java
+++ b/service/java/com/android/server/wifi/ClientModeImpl.java
@@ -673,7 +673,7 @@ public class ClientModeImpl extends StateMachine {
 
     /**
      * Time window in milliseconds for which we send
-     * {@link NetworkAgent#explicitlySelected(boolean)}
+     * {@link NetworkAgent#explicitlySelected(boolean, boolean)}
      * after connecting to the network which the user last selected.
      */
     @VisibleForTesting
@@ -5391,8 +5391,8 @@ public class ClientModeImpl extends StateMachine {
 
     /**
      * Helper function to check if we need to invoke
-     * {@link NetworkAgent#explicitlySelected(boolean)} to indicate that we connected to a network
-     * which the user just chose
+     * {@link NetworkAgent#explicitlySelected(boolean, boolean)} to indicate that we connected to a
+     * network which the user just chose
      * (i.e less than {@link #LAST_SELECTED_NETWORK_EXPIRATION_AGE_MILLIS) before).
      */
     @VisibleForTesting
@@ -5408,26 +5408,29 @@ public class ClientModeImpl extends StateMachine {
     }
 
     private void sendConnectedState() {
-        // If this network was explicitly selected by the user, evaluate whether to call
-        // explicitlySelected() so the system can treat it appropriately.
+        // If this network was explicitly selected by the user, evaluate whether to inform
+        // ConnectivityService of that fact so the system can treat it appropriately.
         WifiConfiguration config = getCurrentWifiConfiguration();
+
+        boolean prompt = false;
         if (shouldEvaluateWhetherToSendExplicitlySelected(config)) {
-            boolean prompt =
-                    mWifiPermissionsUtil.checkNetworkSettingsPermission(config.lastConnectUid);
+            // If prompt is true, the network was selected by the user via Settings or
+            // QuickSettings. If this network has Internet access, switch to it. Otherwise, switch
+            // to it only if the user confirms that they really want to switch, or has already
+            // confirmed and selected "Don't ask again".
+            prompt = mWifiPermissionsUtil.checkNetworkSettingsPermission(config.lastConnectUid);
             if (mVerboseLoggingEnabled) {
                 log("Network selected by UID " + config.lastConnectUid + " prompt=" + prompt);
             }
-            if (prompt) {
-                // Selected by the user via Settings or QuickSettings. If this network has Internet
-                // access, switch to it. Otherwise, switch to it only if the user confirms that they
-                // really want to switch, or has already confirmed and selected "Don't ask again".
-                if (mVerboseLoggingEnabled) {
-                    log("explictlySelected acceptUnvalidated=" + config.noInternetAccessExpected);
-                }
-                if (mNetworkAgent != null) {
-                    mNetworkAgent.explicitlySelected(config.noInternetAccessExpected);
-                }
-            }
+        }
+
+        if (mVerboseLoggingEnabled) {
+            log("explictlySelected=" + prompt + " acceptUnvalidated="
+                    + config.noInternetAccessExpected);
+        }
+
+        if (mNetworkAgent != null) {
+            mNetworkAgent.explicitlySelected(prompt, config.noInternetAccessExpected);
         }
 
         setNetworkDetailedState(DetailedState.CONNECTED);
diff --git a/tests/wifitests/src/com/android/server/wifi/ClientModeImplTest.java b/tests/wifitests/src/com/android/server/wifi/ClientModeImplTest.java
index e839796c9..8dfd3e99f 100644
--- a/tests/wifitests/src/com/android/server/wifi/ClientModeImplTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/ClientModeImplTest.java
@@ -123,6 +123,7 @@ import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Objects;
 import java.util.concurrent.CountDownLatch;
 import java.util.function.Consumer;
 
@@ -328,6 +329,7 @@ public class ClientModeImplTest {
     HandlerThread mP2pThread;
     HandlerThread mSyncThread;
     AsyncChannel  mCmiAsyncChannel;
+    AsyncChannel  mNetworkAgentAsyncChannel;
     TestAlarmManager mAlarmManager;
     MockWifiMonitor mWifiMonitor;
     TestLooper mLooper;
@@ -381,6 +383,8 @@ public class ClientModeImplTest {
     @Mock WifiLockManager mWifiLockManager;
     @Mock AsyncChannel mNullAsyncChannel;
     @Mock CarrierNetworkConfig mCarrierNetworkConfig;
+    @Mock Handler mNetworkAgentHandler;
+
 
     final ArgumentCaptor<WifiNative.InterfaceCallback> mInterfaceCallbackCaptor =
             ArgumentCaptor.forClass(WifiNative.InterfaceCallback.class);
@@ -500,7 +504,8 @@ public class ClientModeImplTest {
         when(mWifiScoreCard.getL2KeyAndGroupHint(any())).thenReturn(new Pair<>(null, null));
     }
 
-    private void registerAsyncChannel(Consumer<AsyncChannel> consumer, Messenger messenger) {
+    private void registerAsyncChannel(Consumer<AsyncChannel> consumer, Messenger messenger,
+            Handler wrappedHandler) {
         final AsyncChannel channel = new AsyncChannel();
         Handler handler = new Handler(mLooper.getLooper()) {
             @Override
@@ -514,7 +519,15 @@ public class ClientModeImplTest {
                         }
                         break;
                     case AsyncChannel.CMD_CHANNEL_DISCONNECTED:
-                        Log.d(TAG, "Command channel disconnected" + this);
+                        Log.d(TAG, "Command channel disconnected " + this);
+                        break;
+                    case AsyncChannel.CMD_CHANNEL_FULLY_CONNECTED:
+                        Log.d(TAG, "Command channel fully connected " + this);
+                        break;
+                    default:
+                        if (wrappedHandler != null) {
+                            wrappedHandler.handleMessage(msg);
+                        }
                         break;
                 }
             }
@@ -524,6 +537,10 @@ public class ClientModeImplTest {
         mLooper.dispatchAll();
     }
 
+    private void registerAsyncChannel(Consumer<AsyncChannel> consumer, Messenger messenger) {
+        registerAsyncChannel(consumer, messenger, null /* wrappedHandler */);
+    }
+
     private void initializeCmi() throws Exception {
         mCmi = new ClientModeImpl(mContext, mFrameworkFacade, mLooper.getLooper(),
                 mUserManager, mWifiInjector, mBackupManagerProxy, mCountryCode, mWifiNative,
@@ -561,6 +578,8 @@ public class ClientModeImplTest {
         mP2pThread = null;
         mSyncThread = null;
         mCmiAsyncChannel = null;
+        mNetworkAgentAsyncChannel = null;
+        mNetworkAgentHandler = null;
         mCmi = null;
     }
 
@@ -2237,25 +2256,30 @@ public class ClientModeImplTest {
         verify(mWifiConfigManager, never()).setRecentFailureAssociationStatus(anyInt(), anyInt());
     }
 
-    /**
-     * Test that the helper method
-     * {@link ClientModeImpl#shouldEvaluateWhetherToSendExplicitlySelected(WifiConfiguration)}
-     * returns true when we connect to the last selected network before expiration of
-     * {@link ClientModeImpl#LAST_SELECTED_NETWORK_EXPIRATION_AGE_MILLIS}.
-     */
-    @Test
-    public void testShouldEvaluateWhetherToSendExplicitlySelected_SameNetworkNotExpired() {
+    private WifiConfiguration makeLastSelectedWifiConfiguration(int lastSelectedNetworkId,
+            long timeSinceLastSelected) {
         long lastSelectedTimestamp = 45666743454L;
-        int lastSelectedNetworkId = 5;
 
         when(mClock.getElapsedSinceBootMillis()).thenReturn(
-                lastSelectedTimestamp
-                        + ClientModeImpl.LAST_SELECTED_NETWORK_EXPIRATION_AGE_MILLIS - 1);
+                lastSelectedTimestamp + timeSinceLastSelected);
         when(mWifiConfigManager.getLastSelectedTimeStamp()).thenReturn(lastSelectedTimestamp);
         when(mWifiConfigManager.getLastSelectedNetwork()).thenReturn(lastSelectedNetworkId);
 
         WifiConfiguration currentConfig = new WifiConfiguration();
         currentConfig.networkId = lastSelectedNetworkId;
+        return currentConfig;
+    }
+
+    /**
+     * Test that the helper method
+     * {@link ClientModeImpl#shouldEvaluateWhetherToSendExplicitlySelected(WifiConfiguration)}
+     * returns true when we connect to the last selected network before expiration of
+     * {@link ClientModeImpl#LAST_SELECTED_NETWORK_EXPIRATION_AGE_MILLIS}.
+     */
+    @Test
+    public void testShouldEvaluateWhetherToSendExplicitlySelected_SameNetworkNotExpired() {
+        WifiConfiguration currentConfig = makeLastSelectedWifiConfiguration(5,
+                ClientModeImpl.LAST_SELECTED_NETWORK_EXPIRATION_AGE_MILLIS - 1);
         assertTrue(mCmi.shouldEvaluateWhetherToSendExplicitlySelected(currentConfig));
     }
 
@@ -2267,17 +2291,8 @@ public class ClientModeImplTest {
      */
     @Test
     public void testShouldEvaluateWhetherToSendExplicitlySelected_SameNetworkExpired() {
-        long lastSelectedTimestamp = 45666743454L;
-        int lastSelectedNetworkId = 5;
-
-        when(mClock.getElapsedSinceBootMillis()).thenReturn(
-                lastSelectedTimestamp
-                        + ClientModeImpl.LAST_SELECTED_NETWORK_EXPIRATION_AGE_MILLIS + 1);
-        when(mWifiConfigManager.getLastSelectedTimeStamp()).thenReturn(lastSelectedTimestamp);
-        when(mWifiConfigManager.getLastSelectedNetwork()).thenReturn(lastSelectedNetworkId);
-
-        WifiConfiguration currentConfig = new WifiConfiguration();
-        currentConfig.networkId = lastSelectedNetworkId;
+        WifiConfiguration currentConfig = makeLastSelectedWifiConfiguration(5,
+                ClientModeImpl.LAST_SELECTED_NETWORK_EXPIRATION_AGE_MILLIS + 1);
         assertFalse(mCmi.shouldEvaluateWhetherToSendExplicitlySelected(currentConfig));
     }
 
@@ -2288,18 +2303,88 @@ public class ClientModeImplTest {
      */
     @Test
     public void testShouldEvaluateWhetherToSendExplicitlySelected_DifferentNetwork() {
-        long lastSelectedTimestamp = 45666743454L;
-        int lastSelectedNetworkId = 5;
+        WifiConfiguration currentConfig = makeLastSelectedWifiConfiguration(5,
+                ClientModeImpl.LAST_SELECTED_NETWORK_EXPIRATION_AGE_MILLIS - 1);
+        currentConfig.networkId = 4;
+        assertFalse(mCmi.shouldEvaluateWhetherToSendExplicitlySelected(currentConfig));
+    }
 
-        when(mClock.getElapsedSinceBootMillis()).thenReturn(
-                lastSelectedTimestamp
-                        + ClientModeImpl.LAST_SELECTED_NETWORK_EXPIRATION_AGE_MILLIS - 1);
-        when(mWifiConfigManager.getLastSelectedTimeStamp()).thenReturn(lastSelectedTimestamp);
-        when(mWifiConfigManager.getLastSelectedNetwork()).thenReturn(lastSelectedNetworkId);
+    private void expectRegisterNetworkAgent() {
+        // Expects that the code calls registerNetworkAgent and provides a way for the test to
+        // verify the messages sent through the NetworkAgent to ConnectivityService.
+        // We cannot just use a mock object here because mWifiNetworkAgent is private to CMI.
+        // TODO: consider exposing WifiNetworkAgent and using mocks.
+        ArgumentCaptor<Messenger> messengerCaptor = ArgumentCaptor.forClass(Messenger.class);
+        verify(mConnectivityManager).registerNetworkAgent(messengerCaptor.capture(),
+                any(NetworkInfo.class), any(LinkProperties.class), any(NetworkCapabilities.class),
+                anyInt(), any(NetworkMisc.class), anyInt());
 
-        WifiConfiguration currentConfig = new WifiConfiguration();
-        currentConfig.networkId = lastSelectedNetworkId - 1;
-        assertFalse(mCmi.shouldEvaluateWhetherToSendExplicitlySelected(currentConfig));
+        registerAsyncChannel((x) -> {
+            mNetworkAgentAsyncChannel = x;
+        }, messengerCaptor.getValue(), mNetworkAgentHandler);
+
+        mNetworkAgentAsyncChannel.sendMessage(AsyncChannel.CMD_CHANNEL_FULL_CONNECTION);
+        mLooper.dispatchAll();
+    }
+
+    private void expectNetworkAgentMessage(int what, int arg1, int arg2, Object obj) {
+        verify(mNetworkAgentHandler).handleMessage(argThat(msg ->
+                what == msg.what && arg1 == msg.arg1 && arg2 == msg.arg2
+                && Objects.equals(obj, msg.obj)));
+    }
+
+    /**
+     * Verify that when a network is explicitly selected, but noInternetAccessExpected is false,
+     * {@link NetworkAgent#explicitlySelected} is called with explicitlySelected=false and
+     * acceptUnvalidated=false.
+     */
+    @Test
+    public void testExplicitlySelected_ExplicitInternetExpected() throws Exception {
+        // Network is explicitly selected.
+        WifiConfiguration config = makeLastSelectedWifiConfiguration(FRAMEWORK_NETWORK_ID,
+                ClientModeImpl.LAST_SELECTED_NETWORK_EXPIRATION_AGE_MILLIS - 1);
+        mConnectedNetwork.noInternetAccessExpected = false;
+
+        connect();
+        expectRegisterNetworkAgent();
+        expectNetworkAgentMessage(NetworkAgent.EVENT_SET_EXPLICITLY_SELECTED,
+                1 /* explicitlySelected */, 0 /* acceptUnvalidated */, null);
+    }
+
+    /**
+     * Verify that when a network is not explicitly selected, but noInternetAccessExpected is true,
+     * {@link NetworkAgent#explicitlySelected} is called with explicitlySelected=false and
+     * acceptUnvalidated=true.
+     */
+    @Test
+    public void testExplicitlySelected_NotExplicitNoInternetExpected() throws Exception {
+        // Network is no longer explicitly selected.
+        WifiConfiguration config = makeLastSelectedWifiConfiguration(FRAMEWORK_NETWORK_ID,
+                ClientModeImpl.LAST_SELECTED_NETWORK_EXPIRATION_AGE_MILLIS + 1);
+        mConnectedNetwork.noInternetAccessExpected = true;
+
+        connect();
+        expectRegisterNetworkAgent();
+        expectNetworkAgentMessage(NetworkAgent.EVENT_SET_EXPLICITLY_SELECTED,
+                0 /* explicitlySelected */, 1 /* acceptUnvalidated */, null);
+    }
+
+    /**
+     * Verify that when a network is explicitly selected, and noInternetAccessExpected is true,
+     * {@link NetworkAgent#explicitlySelected} is called with explicitlySelected=true and
+     * acceptUnvalidated=true.
+     */
+    @Test
+    public void testExplicitlySelected_ExplicitNoInternetExpected() throws Exception {
+        // Network is explicitly selected.
+        WifiConfiguration config = makeLastSelectedWifiConfiguration(FRAMEWORK_NETWORK_ID,
+                ClientModeImpl.LAST_SELECTED_NETWORK_EXPIRATION_AGE_MILLIS - 1);
+        mConnectedNetwork.noInternetAccessExpected = true;
+
+        connect();
+        expectRegisterNetworkAgent();
+        expectNetworkAgentMessage(NetworkAgent.EVENT_SET_EXPLICITLY_SELECTED,
+                1 /* explicitlySelected */, 1 /* acceptUnvalidated */, null);
     }
 
     /**