Auto-open Captive Portal when user clicks on open network.

Rationale: many users are confused about what to do after tapping the captive
portal network in Settings, and since there is explicit user action to
connect to that network at that point, auto-opening the portal is
natural and makes it simpler.

Bug: 148538768
Test: manually with local AP with Captive Portal and:
> make RunSettingsRoboTests -j40
> atest com.android.server.ConnectivityServiceTest

Change-Id: I29573132cd3e46ff22e6f67bb3678516fabac47d

src/com/android/settings/wifi/CaptivePortalNetworkCallback.java

@@ -22,7 +22,7 @@ import android.net.NetworkCapabilities;
 import com.android.internal.util.Preconditions;
 
 /** Listens for changes to NetworkCapabilities to update the ConnectedAccessPointPreference. */
-final class CaptivePortalNetworkCallback extends NetworkCallback {
+class CaptivePortalNetworkCallback extends NetworkCallback {
 
     private final ConnectedAccessPointPreference mConnectedApPreference;
     private final Network mNetwork;
@@ -36,25 +36,42 @@ final class CaptivePortalNetworkCallback extends NetworkCallback {
     }
 
     @Override
-    public void onLost(Network network) {
+    public final void onLost(Network network) {
         if (mNetwork.equals(network)) {
-            mIsCaptivePortal = false;
+            setIsCaptivePortal(false);
         }
     }
 
     @Override
-    public void onCapabilitiesChanged(Network network, NetworkCapabilities networkCapabilities) {
+    public final void onCapabilitiesChanged(Network network,
+            NetworkCapabilities networkCapabilities) {
         if (mNetwork.equals(network)) {
-            mIsCaptivePortal = WifiUtils.canSignIntoNetwork(networkCapabilities);
+            boolean isCaptivePortal = WifiUtils.canSignIntoNetwork(networkCapabilities);
-            mConnectedApPreference.setCaptivePortal(mIsCaptivePortal);
+            setIsCaptivePortal(isCaptivePortal);
+            mConnectedApPreference.setCaptivePortal(isCaptivePortal);
         }
     }
 
+    /**
+     * Called when captive portal capability changes for the current network. Default implementation
+     * is a no-op. Use {@link CaptivePortalNetworkCallback#isCaptivePortal()} to read new
+     * capability.
+     */
+    public void onCaptivePortalCapabilityChanged() {}
+
+    private void setIsCaptivePortal(boolean isCaptivePortal) {
+        if (isCaptivePortal == mIsCaptivePortal) {
+            return;
+        }
+        mIsCaptivePortal = isCaptivePortal;
+        onCaptivePortalCapabilityChanged();
+    }
+
     /**
      * Returns true if the supplied network and preference are not null and are the same as the
      * originally supplied values.
      */
-    public boolean isSameNetworkAndPreference(
+    public final boolean isSameNetworkAndPreference(
             Network network, ConnectedAccessPointPreference connectedApPreference) {
         return mNetwork.equals(network) && mConnectedApPreference == connectedApPreference;
     }
@@ -63,12 +80,12 @@ final class CaptivePortalNetworkCallback extends NetworkCallback {
      * Returns true if the most recent update to the NetworkCapabilities indicates a captive portal
      * network and the Network was not lost in the interim.
      */
-    public boolean isCaptivePortal() {
+    public final boolean isCaptivePortal() {
         return mIsCaptivePortal;
     }
 
     /** Returns the currently associated network. */
-    public Network getNetwork() {
+    public final Network getNetwork() {
         return mNetwork;
     }
 }

src/com/android/settings/wifi/WifiSettings.java

@@ -50,6 +50,7 @@ import android.view.MenuItem;
 import android.view.View;
 import android.widget.Toast;
 
+import androidx.annotation.IntDef;
 import androidx.annotation.VisibleForTesting;
 import androidx.preference.Preference;
 import androidx.preference.PreferenceCategory;
@@ -135,12 +136,16 @@ public class WifiSettings extends RestrictedSettingsFragment
         setProgressBarVisible(false);
     };
 
-    protected WifiManager mWifiManager;
+    @VisibleForTesting
-    private ConnectivityManager mConnectivityManager;
+    WifiManager mWifiManager;
+    @VisibleForTesting
+    ConnectivityManager mConnectivityManager;
     private WifiManager.ActionListener mConnectListener;
     private WifiManager.ActionListener mSaveListener;
     private WifiManager.ActionListener mForgetListener;
-    private CaptivePortalNetworkCallback mCaptivePortalNetworkCallback;
+    @VisibleForTesting
+    CaptivePortalNetworkCallback mCaptivePortalNetworkCallback;
+    private Network mLastNetworkCaptivePortalAppStarted;
 
     /**
      * The state of {@link #isUiRestricted()} at {@link #onCreate(Bundle)}}. This is neccesary to
@@ -196,6 +201,15 @@ public class WifiSettings extends RestrictedSettingsFragment
      * network once connected.
      */
     private boolean mClickedConnect;
+    @ConnectSource int mConnectSource = CONNECT_SOURCE_UNSPECIFIED;
+
+    private static final int CONNECT_SOURCE_UNSPECIFIED = 0;
+    private static final int CONNECT_SOURCE_NETWORK_MENU_ITEM_CLICK = 1;
+    private static final int CONNECT_SOURCE_NETWORK_LIST_ITEM_CLICK = 2;
+
+    @IntDef({CONNECT_SOURCE_UNSPECIFIED, CONNECT_SOURCE_NETWORK_MENU_ITEM_CLICK,
+        CONNECT_SOURCE_NETWORK_LIST_ITEM_CLICK})
+    private @interface ConnectSource {}
 
     /* End of "used in Wifi Setup context" */
 
@@ -512,12 +526,14 @@ public class WifiSettings extends RestrictedSettingsFragment
             case MENU_ID_CONNECT: {
                 boolean isSavedNetwork = mSelectedAccessPoint.isSaved();
                 if (isSavedNetwork) {
-                    connect(mSelectedAccessPoint.getConfig(), isSavedNetwork);
+                    connect(mSelectedAccessPoint.getConfig(), isSavedNetwork,
+                            CONNECT_SOURCE_NETWORK_MENU_ITEM_CLICK);
                 } else if ((mSelectedAccessPoint.getSecurity() == AccessPoint.SECURITY_NONE) ||
                         (mSelectedAccessPoint.getSecurity() == AccessPoint.SECURITY_OWE)) {
                     /** Bypass dialog for unsecured networks */
                     mSelectedAccessPoint.generateOpenNetworkConfig();
-                    connect(mSelectedAccessPoint.getConfig(), isSavedNetwork);
+                    connect(mSelectedAccessPoint.getConfig(), isSavedNetwork,
+                            CONNECT_SOURCE_NETWORK_MENU_ITEM_CLICK);
                 } else {
                     showDialog(mSelectedAccessPoint, WifiConfigUiBase.MODE_CONNECT);
                 }
@@ -563,11 +579,15 @@ public class WifiSettings extends RestrictedSettingsFragment
 
                 case WifiUtils.CONNECT_TYPE_OPEN_NETWORK:
                     mSelectedAccessPoint.generateOpenNetworkConfig();
-                    connect(mSelectedAccessPoint.getConfig(), mSelectedAccessPoint.isSaved());
+                    connect(mSelectedAccessPoint.getConfig(),
+                            mSelectedAccessPoint.isSaved(),
+                            CONNECT_SOURCE_NETWORK_LIST_ITEM_CLICK);
                     break;
 
                 case WifiUtils.CONNECT_TYPE_SAVED_NETWORK:
-                    connect(mSelectedAccessPoint.getConfig(), true /* isSavedNetwork */);
+                    connect(mSelectedAccessPoint.getConfig(),
+                            true /* isSavedNetwork */,
+                            CONNECT_SOURCE_NETWORK_LIST_ITEM_CLICK);
                     break;
 
                 default:
@@ -705,6 +725,8 @@ public class WifiSettings extends RestrictedSettingsFragment
                 setOffMessage();
                 setAdditionalSettingsSummaries();
                 setProgressBarVisible(false);
+                mConnectSource = CONNECT_SOURCE_UNSPECIFIED;
+                mClickedConnect = false;
                 break;
         }
     }
@@ -876,7 +898,7 @@ public class WifiSettings extends RestrictedSettingsFragment
                     pref.getAccessPoint().saveWifiState(pref.getExtras());
                     if (mCaptivePortalNetworkCallback != null
                             && mCaptivePortalNetworkCallback.isCaptivePortal()) {
-                        mConnectivityManager.startCaptivePortalApp(
+                        startCaptivePortalApp(
                                 mCaptivePortalNetworkCallback.getNetwork());
                     } else {
                         launchNetworkDetailsFragment(pref);
@@ -914,7 +936,12 @@ public class WifiSettings extends RestrictedSettingsFragment
 
         unregisterCaptivePortalNetworkCallback();
 
-        mCaptivePortalNetworkCallback = new CaptivePortalNetworkCallback(wifiNetwork, pref);
+        mCaptivePortalNetworkCallback = new CaptivePortalNetworkCallback(wifiNetwork, pref) {
+            @Override
+            public void onCaptivePortalCapabilityChanged() {
+                checkStartCaptivePortalApp();
+            }
+        };
         mConnectivityManager.registerNetworkCallback(
                 new NetworkRequest.Builder()
                         .clearCapabilities()
@@ -1099,14 +1126,17 @@ public class WifiSettings extends RestrictedSettingsFragment
         if (config == null) {
             if (mSelectedAccessPoint != null
                     && mSelectedAccessPoint.isSaved()) {
-                connect(mSelectedAccessPoint.getConfig(), true /* isSavedNetwork */);
+                connect(mSelectedAccessPoint.getConfig(),
+                        true /* isSavedNetwork */,
+                        CONNECT_SOURCE_UNSPECIFIED);
             }
         } else if (configController.getMode() == WifiConfigUiBase.MODE_MODIFY) {
             mWifiManager.save(config, mSaveListener);
         } else {
             mWifiManager.save(config, mSaveListener);
             if (mSelectedAccessPoint != null) { // Not an "Add network"
-                connect(config, false /* isSavedNetwork */);
+                connect(config, false /* isSavedNetwork */,
+                        CONNECT_SOURCE_UNSPECIFIED);
             }
         }
 
@@ -1143,21 +1173,16 @@ public class WifiSettings extends RestrictedSettingsFragment
         changeNextButtonState(false);
     }
 
-    protected void connect(final WifiConfiguration config, boolean isSavedNetwork) {
+    protected void connect(final WifiConfiguration config,
+            boolean isSavedNetwork, @ConnectSource int connectSource) {
         // Log subtype if configuration is a saved network.
         mMetricsFeatureProvider.action(getContext(), SettingsEnums.ACTION_WIFI_CONNECT,
                 isSavedNetwork);
+        mConnectSource = connectSource;
         mWifiManager.connect(config, mConnectListener);
         mClickedConnect = true;
     }
 
-    protected void connect(final int networkId, boolean isSavedNetwork) {
-        // Log subtype if configuration is a saved network.
-        mMetricsFeatureProvider.action(getActivity(), SettingsEnums.ACTION_WIFI_CONNECT,
-                isSavedNetwork);
-        mWifiManager.connect(networkId, mConnectListener);
-    }
-
     @VisibleForTesting
     void handleAddNetworkRequest(int result, Intent data) {
         if (result == Activity.RESULT_OK) {
@@ -1217,7 +1242,8 @@ public class WifiSettings extends RestrictedSettingsFragment
             mWifiManager.save(wifiConfiguration, mSaveListener);
 
             if (mSelectedAccessPoint != null) {
-                connect(wifiConfiguration, false /*isSavedNetwork*/);
+                connect(wifiConfiguration, false /*isSavedNetwork*/,
+                        CONNECT_SOURCE_UNSPECIFIED);
             }
             mWifiTracker.resumeScanning();
         }
@@ -1236,6 +1262,42 @@ public class WifiSettings extends RestrictedSettingsFragment
                 .launch();
     }
 
+    /**
+     * Starts the captive portal for current network if it's been clicked from the available
+     * networks (or contextual menu). We only do it *once* for a picked network, to avoid connecting
+     * again on bg/fg or if user dismisses Captive Portal before connecting (otherwise, coming back
+     * to this screen while connected to the same network but not signed in would open CP again).
+     */
+    private void checkStartCaptivePortalApp() {
+        Network currentNetwork = getCurrentWifiNetwork();
+        if (mCaptivePortalNetworkCallback == null || currentNetwork == null
+                || !currentNetwork.equals(mCaptivePortalNetworkCallback.getNetwork())
+                || !mCaptivePortalNetworkCallback.isCaptivePortal()) {
+            return;
+        }
+
+        if (mConnectSource != CONNECT_SOURCE_NETWORK_LIST_ITEM_CLICK
+                && mConnectSource != CONNECT_SOURCE_NETWORK_MENU_ITEM_CLICK) {
+            return;
+        }
+
+        if (mLastNetworkCaptivePortalAppStarted != null
+                && mLastNetworkCaptivePortalAppStarted.equals(currentNetwork)) {
+            // We already auto-opened CP for same network
+            return;
+        }
+
+        startCaptivePortalApp(currentNetwork);
+    }
+
+    private void startCaptivePortalApp(Network network) {
+        if (mConnectivityManager == null || network == null) {
+            return;
+        }
+        mLastNetworkCaptivePortalAppStarted = network;
+        mConnectivityManager.startCaptivePortalApp(network);
+    }
+
     public static final BaseSearchIndexProvider SEARCH_INDEX_DATA_PROVIDER =
             new BaseSearchIndexProvider(R.xml.wifi_settings) {
                 @Override

tests/robotests/src/com/android/settings/wifi/WifiSettingsTest.java

@@ -15,6 +15,8 @@
  */
 package com.android.settings.wifi;
 
+import static android.net.wifi.WifiManager.WIFI_STATE_ENABLED;
+
 import static com.google.common.truth.Truth.assertThat;
 
 import static org.mockito.ArgumentMatchers.any;
@@ -31,9 +33,14 @@ import android.app.Activity;
 import android.content.ContentResolver;
 import android.content.Context;
 import android.content.Intent;
+import android.content.pm.PackageManager;
 import android.content.res.Resources;
+import android.net.ConnectivityManager;
+import android.net.Network;
+import android.net.NetworkCapabilities;
 import android.net.wifi.EAPConstants;
 import android.net.wifi.WifiConfiguration;
+import android.net.wifi.WifiInfo;
 import android.net.wifi.WifiManager;
 import android.net.wifi.hotspot2.PasspointConfiguration;
 import android.net.wifi.hotspot2.pps.Credential;
@@ -51,22 +58,30 @@ import androidx.preference.PreferenceScreen;
 import androidx.recyclerview.widget.RecyclerView;
 
 import com.android.settings.R;
+import com.android.settings.SettingsActivity;
 import com.android.settings.datausage.DataUsagePreference;
+import com.android.settings.testutils.FakeFeatureFactory;
 import com.android.settings.testutils.shadow.ShadowDataUsageUtils;
 import com.android.settings.testutils.shadow.ShadowFragment;
+import com.android.settings.widget.SwitchBar;
+import com.android.settingslib.core.instrumentation.MetricsFeatureProvider;
 import com.android.settingslib.wifi.AccessPoint;
 import com.android.settingslib.wifi.WifiTracker;
+import com.android.settingslib.wifi.WifiTrackerFactory;
 
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.ArgumentCaptor;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 import org.robolectric.RobolectricTestRunner;
 import org.robolectric.RuntimeEnvironment;
 import org.robolectric.annotation.Config;
+import org.robolectric.util.ReflectionHelpers;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 
 @RunWith(RobolectricTestRunner.class)
@@ -81,9 +96,27 @@ public class WifiSettingsTest {
     @Mock
     private DataUsagePreference mDataUsagePreference;
     @Mock
+    private RecyclerView mRecyclerView;
+    @Mock
+    private RecyclerView.Adapter mRecyclerViewAdapter;
+    @Mock
+    private View mHeaderView;
+    @Mock
     private WifiManager mWifiManager;
+    @Mock
+    private ConnectivityManager mConnectivityManager;
+    @Mock
+    private Intent mActivityIntent;
+    @Mock
+    private SwitchBar mSwitchBar;
+    @Mock
+    private WifiInfo mWifiInfo;
+    @Mock
+    private PackageManager mPackageManager;
     private Context mContext;
     private WifiSettings mWifiSettings;
+    private FakeFeatureFactory mFakeFeatureFactory;
+    private MetricsFeatureProvider mMetricsFeatureProvider;
 
     @Before
     public void setUp() {
@@ -92,12 +125,23 @@ public class WifiSettingsTest {
 
         mWifiSettings = spy(new WifiSettings());
         doReturn(mContext).when(mWifiSettings).getContext();
+        doReturn(mRecyclerViewAdapter).when(mRecyclerView).getAdapter();
+        doReturn(mRecyclerView).when(mWifiSettings).getListView();
         doReturn(mPowerManager).when(mContext).getSystemService(PowerManager.class);
+        doReturn(mHeaderView).when(mWifiSettings).setPinnedHeaderView(anyInt());
+        doReturn(mWifiInfo).when(mWifiManager).getConnectionInfo();
+        doReturn(mWifiManager).when(mWifiTracker).getManager();
         mWifiSettings.mAddWifiNetworkPreference = new AddWifiNetworkPreference(mContext);
         mWifiSettings.mSavedNetworksPreference = new Preference(mContext);
         mWifiSettings.mConfigureWifiSettingsPreference = new Preference(mContext);
         mWifiSettings.mWifiTracker = mWifiTracker;
         mWifiSettings.mWifiManager = mWifiManager;
+        mWifiSettings.mConnectivityManager = mConnectivityManager;
+        mFakeFeatureFactory = FakeFeatureFactory.setupForTest();
+        mMetricsFeatureProvider = mFakeFeatureFactory.getMetricsFeatureProvider();
+        ReflectionHelpers.setField(mWifiSettings, "mMetricsFeatureProvider",
+                mMetricsFeatureProvider);
+        WifiTrackerFactory.setTestingWifiTracker(mWifiTracker);
     }
 
     @Test
@@ -138,6 +182,14 @@ public class WifiSettingsTest {
         return mockConfigs;
     }
 
+    static NetworkCapabilities makeCaptivePortalNetworkCapabilities() {
+        final NetworkCapabilities capabilities = new NetworkCapabilities();
+        capabilities.clearAll();
+        capabilities.addTransportType(NetworkCapabilities.TRANSPORT_WIFI);
+        capabilities.addCapability(NetworkCapabilities.NET_CAPABILITY_CAPTIVE_PORTAL);
+        return capabilities;
+    }
+
     @Test
     public void setAdditionalSettingsSummaries_hasSavedNetwork_preferenceVisible() {
         when(mWifiManager.getConfiguredNetworks())
@@ -225,16 +277,20 @@ public class WifiSettingsTest {
     }
 
     private void setUpForOnCreate() {
-        final FragmentActivity activity = mock(FragmentActivity.class);
+        final SettingsActivity activity = mock(SettingsActivity.class);
+        when(activity.getSwitchBar()).thenReturn(mSwitchBar);
         when(mWifiSettings.getActivity()).thenReturn(activity);
         final Resources.Theme theme = mContext.getTheme();
         when(activity.getTheme()).thenReturn(theme);
+        when(activity.getIntent()).thenReturn(mActivityIntent);
         UserManager userManager = mock(UserManager.class);
         when(activity.getSystemService(Context.USER_SERVICE))
                 .thenReturn(userManager);
-
         when(mWifiSettings.findPreference(WifiSettings.PREF_KEY_DATA_USAGE))
                 .thenReturn(mDataUsagePreference);
+        when(activity.getSystemService(Context.WIFI_SERVICE)).thenReturn(mWifiManager);
+        when(activity.getSystemService(ConnectivityManager.class)).thenReturn(mConnectivityManager);
+        when(activity.getPackageManager()).thenReturn(mPackageManager);
     }
 
     @Test
@@ -291,4 +347,58 @@ public class WifiSettingsTest {
 
         assertThat(adapter.hasStableIds()).isTrue();
     }
+
+    @Test
+    @Config(shadows = {ShadowDataUsageUtils.class, ShadowFragment.class})
+    public void clickOnWifiNetworkWith_shouldStartCaptivePortalApp() {
+        when(mWifiManager.getConfiguredNetworks()).thenReturn(createMockWifiConfigurations(
+                NUM_NETWORKS));
+        when(mWifiTracker.isConnected()).thenReturn(true);
+
+        final AccessPoint accessPointActive = mock(AccessPoint.class);
+        when(accessPointActive.isActive()).thenReturn(true);
+        when(accessPointActive.isSaved()).thenReturn(false);
+        when(accessPointActive.getConfig()).thenReturn(mock(WifiConfiguration.class));
+
+        final AccessPoint accessPointInactive = mock(AccessPoint.class);
+        when(accessPointInactive.isActive()).thenReturn(false);
+        when(accessPointInactive.isSaved()).thenReturn(false);
+        when(accessPointInactive.getConfig()).thenReturn(mock(WifiConfiguration.class));
+
+        when(mWifiTracker.getAccessPoints()).thenReturn(Arrays.asList(accessPointActive,
+                accessPointInactive));
+        when(mWifiManager.getWifiState()).thenReturn(WIFI_STATE_ENABLED);
+        when(mWifiManager.isWifiEnabled()).thenReturn(true);
+
+        final Network network = mock(Network.class);
+        when(mWifiManager.getCurrentNetwork()).thenReturn(network);
+
+        // Simulate activity creation cycle
+        setUpForOnCreate();
+        ShadowDataUsageUtils.IS_WIFI_SUPPORTED = true;
+        mWifiSettings.onCreate(Bundle.EMPTY);
+        mWifiSettings.onActivityCreated(null);
+        mWifiSettings.onViewCreated(new View(mContext), new Bundle());
+        mWifiSettings.onStart();
+
+        // Click on open network
+        final Preference openWifiPref = new LongPressAccessPointPreference(accessPointInactive,
+                mContext, null,
+                false /* forSavedNetworks */, R.drawable.ic_wifi_signal_0,
+                null);
+        mWifiSettings.onPreferenceTreeClick(openWifiPref);
+
+        // Ensure connect() was called, and fake success.
+        ArgumentCaptor<WifiManager.ActionListener> wifiCallbackCaptor = ArgumentCaptor.forClass(
+                WifiManager.ActionListener.class);
+        verify(mWifiManager).connect(any(WifiConfiguration.class), wifiCallbackCaptor.capture());
+        wifiCallbackCaptor.getValue().onSuccess();
+
+        // Simulate capability change
+        mWifiSettings.mCaptivePortalNetworkCallback.onCapabilitiesChanged(network,
+                makeCaptivePortalNetworkCapabilities());
+
+        // Ensure CP was called
+        verify(mConnectivityManager).startCaptivePortalApp(eq(network));
+    }
 }