Observe NetworkCallback instead of polling

Switch from polling for the first connected WiFi network to observing
the currently connected WiFi network and associating that with the
preference at the time the preference is created or updated.

Whenever the ConnectedAccessPointPreference is removed or UI is stopped
unregister the callback.

Bug:68031656
Test: make RunSettingsRoboTests
Change-Id: I5d1ed83b6a13e8a83fae04bfdce8d0f13c2ba0ac

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

@@ -0,0 +1,74 @@
+/*
+ * Copyright (C) 2018 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.android.settings.wifi;
+
+import android.net.ConnectivityManager.NetworkCallback;
+import android.net.Network;
+import android.net.NetworkCapabilities;
+
+import com.android.internal.util.Preconditions;
+
+/** Listens for changes to NetworkCapabilities to update the ConnectedAccessPointPreference. */
+final class CaptivePortalNetworkCallback extends NetworkCallback {
+
+    private final ConnectedAccessPointPreference mConnectedApPreference;
+    private final Network mNetwork;
+
+    private boolean mIsCaptivePortal;
+
+    CaptivePortalNetworkCallback(
+            Network network, ConnectedAccessPointPreference connectedApPreference) {
+        mNetwork = Preconditions.checkNotNull(network);
+        mConnectedApPreference = Preconditions.checkNotNull(connectedApPreference);
+    }
+
+    @Override
+    public void onLost(Network network) {
+        if (mNetwork.equals(network)) {
+            mIsCaptivePortal = false;
+        }
+    }
+
+    @Override
+    public void onCapabilitiesChanged(Network network, NetworkCapabilities networkCapabilities) {
+        if (mNetwork.equals(network)) {
+            mIsCaptivePortal = WifiUtils.canSignIntoNetwork(networkCapabilities);
+            mConnectedApPreference.setCaptivePortal(mIsCaptivePortal);
+        }
+    }
+
+    /**
+     * Returns true if the supplied network and preference are not null and are the same as the
+     * originally supplied values.
+     */
+    public boolean isSameNetworkAndPreference(
+            Network network, ConnectedAccessPointPreference connectedApPreference) {
+        return mNetwork.equals(network) && mConnectedApPreference == connectedApPreference;
+    }
+
+    /**
+     * 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() {
+        return mIsCaptivePortal;
+    }
+
+    /** Returns the currently associated network. */
+    public Network getNetwork() {
+        return mNetwork;
+    }
+}

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

@@ -31,15 +31,12 @@ import com.android.settingslib.wifi.AccessPointPreference;
 public class ConnectedAccessPointPreference extends AccessPointPreference implements
         View.OnClickListener {
 
-    private final CaptivePortalStatus mCaptivePortalStatus;
     private OnGearClickListener mOnGearClickListener;
-    private boolean mCaptivePortalNetwork;
+    private boolean mIsCaptivePortal;
 
     public ConnectedAccessPointPreference(AccessPoint accessPoint, Context context,
-            UserBadgeCache cache, @DrawableRes int iconResId, boolean forSavedNetworks,
+            UserBadgeCache cache, @DrawableRes int iconResId, boolean forSavedNetworks) {
-            CaptivePortalStatus captivePortalStatus) {
         super(accessPoint, context, cache, iconResId, forSavedNetworks);
-        mCaptivePortalStatus = captivePortalStatus;
     }
 
     @Override
@@ -51,9 +48,8 @@ public class ConnectedAccessPointPreference extends AccessPointPreference implem
     public void refresh() {
         super.refresh();
 
-        mCaptivePortalNetwork = mCaptivePortalStatus.isCaptivePortalNetwork();
+        setShowDivider(mIsCaptivePortal);
-        setShowDivider(mCaptivePortalNetwork);
+        if (mIsCaptivePortal) {
-        if (mCaptivePortalNetwork) {
             setSummary(R.string.wifi_tap_to_sign_in);
         }
     }
@@ -71,8 +67,8 @@ public class ConnectedAccessPointPreference extends AccessPointPreference implem
         gear.setOnClickListener(this);
 
         final View gearNoBg = holder.findViewById(R.id.settings_button_no_background);
-        gearNoBg.setVisibility(mCaptivePortalNetwork ? View.INVISIBLE : View.VISIBLE);
+        gearNoBg.setVisibility(mIsCaptivePortal ? View.INVISIBLE : View.VISIBLE);
-        gear.setVisibility(mCaptivePortalNetwork ? View.VISIBLE : View.INVISIBLE);
+        gear.setVisibility(mIsCaptivePortal ? View.VISIBLE : View.INVISIBLE);
     }
 
     @Override
@@ -84,11 +80,15 @@ public class ConnectedAccessPointPreference extends AccessPointPreference implem
         }
     }
 
+    public void setCaptivePortal(boolean isCaptivePortal) {
+        if (mIsCaptivePortal != isCaptivePortal) {
+            mIsCaptivePortal = isCaptivePortal;
+            refresh();
+        }
+    }
+
     public interface OnGearClickListener {
         void onGearClick(ConnectedAccessPointPreference p);
     }
 
-    public interface CaptivePortalStatus {
-        boolean isCaptivePortalNetwork();
-    }
 }

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

@@ -16,6 +16,7 @@
 
 package com.android.settings.wifi;
 
+import static android.net.NetworkCapabilities.TRANSPORT_WIFI;
 import static android.os.UserManager.DISALLOW_CONFIG_WIFI;
 
 import android.annotation.NonNull;
@@ -27,14 +28,15 @@ import android.content.Intent;
 import android.content.res.Resources;
 import android.net.ConnectivityManager;
 import android.net.Network;
-import android.net.NetworkCapabilities;
 import android.net.NetworkInfo;
 import android.net.NetworkInfo.State;
+import android.net.NetworkRequest;
 import android.net.wifi.WifiConfiguration;
 import android.net.wifi.WifiManager;
 import android.nfc.NfcAdapter;
 import android.os.Bundle;
 import android.os.Handler;
+import android.os.Looper;
 import android.os.PowerManager;
 import android.provider.Settings;
 import android.support.annotation.VisibleForTesting;
@@ -124,6 +126,7 @@ public class WifiSettings extends RestrictedSettingsFragment
     private WifiManager.ActionListener mConnectListener;
     private WifiManager.ActionListener mSaveListener;
     private WifiManager.ActionListener mForgetListener;
+    private CaptivePortalNetworkCallback mCaptivePortalNetworkCallback;
 
     /**
      * The state of {@link #isUiRestricted()} at {@link #onCreate(Bundle)}}. This is neccesary to
@@ -400,6 +403,7 @@ public class WifiSettings extends RestrictedSettingsFragment
     public void onStop() {
         getView().removeCallbacks(mUpdateAccessPointsRunnable);
         getView().removeCallbacks(mHideProgressBarRunnable);
+        unregisterCaptivePortalNetworkCallback();
         super.onStop();
     }
 
@@ -786,11 +790,9 @@ public class WifiSettings extends RestrictedSettingsFragment
 
     @NonNull
     private ConnectedAccessPointPreference createConnectedAccessPointPreference(
-            AccessPoint accessPoint,
+            AccessPoint accessPoint) {
-            ConnectedAccessPointPreference.CaptivePortalStatus captivePortalStatus) {
         return new ConnectedAccessPointPreference(accessPoint, getPrefContext(), mUserBadgeCache,
-                R.drawable.ic_wifi_signal_0, false /* forSavedNetworks */,
+                R.drawable.ic_wifi_signal_0, false /* forSavedNetworks */);
-                captivePortalStatus);
     }
 
     /**
@@ -817,8 +819,9 @@ public class WifiSettings extends RestrictedSettingsFragment
         }
 
         // Is the previous currently connected SSID different from the new one?
-        AccessPointPreference preference = (AccessPointPreference)
+        ConnectedAccessPointPreference preference =
-            (mConnectedAccessPointPreferenceCategory.getPreference(0));
+                (ConnectedAccessPointPreference)
+                        (mConnectedAccessPointPreferenceCategory.getPreference(0));
         // The AccessPoints need to be the same reference to ensure that updates are reflected
         // in the UI.
         if (preference.getAccessPoint() != connectedAp) {
@@ -829,8 +832,10 @@ public class WifiSettings extends RestrictedSettingsFragment
 
         // Else same AP is connected, simply refresh the connected access point preference
         // (first and only access point in this category).
-        ((AccessPointPreference) mConnectedAccessPointPreferenceCategory.getPreference(0))
+        preference.refresh();
-                .refresh();
+        // Update any potential changes to the connected network and ensure that the callback is
+        // registered after an onStop lifecycle event.
+        registerCaptivePortalNetworkCallback(getCurrentWifiNetwork(), preference);
         return true;
     }
 
@@ -840,18 +845,19 @@ public class WifiSettings extends RestrictedSettingsFragment
      */
     private void addConnectedAccessPointPreference(AccessPoint connectedAp) {
         final ConnectedAccessPointPreference pref =
-                createConnectedAccessPointPreference(
+                createConnectedAccessPointPreference(connectedAp);
-                        connectedAp, this::isConnectedToCaptivePortalNetwork);
+        registerCaptivePortalNetworkCallback(getCurrentWifiNetwork(), pref);
 
         // Launch details page or captive portal on click.
         pref.setOnPreferenceClickListener(
                 preference -> {
                     pref.getAccessPoint().saveWifiState(pref.getExtras());
-                    Network network = getConnectedWifiNetwork();
+                    if (mCaptivePortalNetworkCallback != null
-                    if (isConnectedToCaptivePortalNetwork(network)) {
+                            && mCaptivePortalNetworkCallback.isCaptivePortal()) {
                         ConnectivityManagerWrapper connectivityManagerWrapper =
                                 new ConnectivityManagerWrapper(mConnectivityManager);
-                        connectivityManagerWrapper.startCaptivePortalApp(network);
+                        connectivityManagerWrapper.startCaptivePortalApp(
+                                mCaptivePortalNetworkCallback.getNetwork());
                     } else {
                         launchNetworkDetailsFragment(pref);
                     }
@@ -874,6 +880,41 @@ public class WifiSettings extends RestrictedSettingsFragment
         }
     }
 
+    private void registerCaptivePortalNetworkCallback(
+            Network wifiNetwork, ConnectedAccessPointPreference pref) {
+        if (wifiNetwork == null || pref == null) {
+            Log.w(TAG, "Network or Preference were null when registering callback.");
+            return;
+        }
+
+        if (mCaptivePortalNetworkCallback != null
+                && mCaptivePortalNetworkCallback.isSameNetworkAndPreference(wifiNetwork, pref)) {
+            return;
+        }
+
+        unregisterCaptivePortalNetworkCallback();
+
+        mCaptivePortalNetworkCallback = new CaptivePortalNetworkCallback(wifiNetwork, pref);
+        mConnectivityManager.registerNetworkCallback(
+                new NetworkRequest.Builder()
+                        .clearCapabilities()
+                        .addTransportType(TRANSPORT_WIFI)
+                        .build(),
+                mCaptivePortalNetworkCallback,
+                new Handler(Looper.getMainLooper()));
+    }
+
+    private void unregisterCaptivePortalNetworkCallback() {
+        if (mCaptivePortalNetworkCallback != null) {
+            try {
+                mConnectivityManager.unregisterNetworkCallback(mCaptivePortalNetworkCallback);
+            } catch (RuntimeException e) {
+                Log.e(TAG, "Unregistering CaptivePortalNetworkCallback failed.", e);
+            }
+            mCaptivePortalNetworkCallback = null;
+        }
+    }
+
     private void launchNetworkDetailsFragment(ConnectedAccessPointPreference pref) {
         new SubSettingLauncher(getContext())
                 .setTitle(getContext().getString(R.string.pref_title_network_details))
@@ -883,38 +924,15 @@ public class WifiSettings extends RestrictedSettingsFragment
                 .launch();
     }
 
-    private boolean isConnectedToCaptivePortalNetwork() {
+    private Network getCurrentWifiNetwork() {
-        return isConnectedToCaptivePortalNetwork(getConnectedWifiNetwork());
+        return mWifiManager != null ? mWifiManager.getCurrentNetwork() : null;
-    }
-
-    private boolean isConnectedToCaptivePortalNetwork(Network network) {
-        if (mConnectivityManager == null || network == null) {
-            return false;
-        }
-        return WifiUtils.canSignIntoNetwork(mConnectivityManager.getNetworkCapabilities(network));
-    }
-
-    private Network getConnectedWifiNetwork() {
-        if (mConnectivityManager != null) {
-            Network networks[] = mConnectivityManager.getAllNetworks();
-            if (networks != null) {
-                for (Network network : networks) {
-                    NetworkCapabilities capabilities =
-                            mConnectivityManager.getNetworkCapabilities(network);
-                    if (capabilities != null
-                            && capabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI)) {
-                        return network;
-                    }
-                }
-            }
-        }
-        return null;
     }
 
     /** Removes all preferences and hide the {@link #mConnectedAccessPointPreferenceCategory}. */
     private void removeConnectedAccessPointPreference() {
         mConnectedAccessPointPreferenceCategory.removeAll();
         mConnectedAccessPointPreferenceCategory.setVisible(false);
+        unregisterCaptivePortalNetworkCallback();
     }
 
     private void setAdditionalSettingsSummaries() {

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

@@ -17,6 +17,7 @@
 package com.android.settings.wifi;
 
 import static com.google.common.truth.Truth.assertThat;
+
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -32,7 +33,6 @@ import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
-import org.mockito.Mockito;
 import org.mockito.MockitoAnnotations;
 import org.robolectric.RuntimeEnvironment;
 
@@ -45,8 +45,6 @@ public class ConnectedAccessPointPreferenceTest {
     private View mView;
     @Mock
     private ConnectedAccessPointPreference.OnGearClickListener mOnGearClickListener;
-    @Mock
-    private ConnectedAccessPointPreference.CaptivePortalStatus mCaptivePortalStatus;
     private Context mContext;
     private ConnectedAccessPointPreference mConnectedAccessPointPreference;
 
@@ -56,7 +54,7 @@ public class ConnectedAccessPointPreferenceTest {
 
         mContext = RuntimeEnvironment.application;
         mConnectedAccessPointPreference = new ConnectedAccessPointPreference(mAccessPoint, mContext,
-                null, 0 /* iconResId */, false /* forSavedNetworks */, mCaptivePortalStatus);
+                null, 0 /* iconResId */, false /* forSavedNetworks */);
         mConnectedAccessPointPreference.setOnGearClickListener(mOnGearClickListener);
     }
 
@@ -78,15 +76,13 @@ public class ConnectedAccessPointPreferenceTest {
 
     @Test
     public void testCaptivePortalStatus_isCaptivePortal_dividerDrawn() {
-        Mockito.when(mCaptivePortalStatus.isCaptivePortalNetwork()).thenReturn(true);
+        mConnectedAccessPointPreference.setCaptivePortal(true);
-        mConnectedAccessPointPreference.refresh();
         assertThat(mConnectedAccessPointPreference.shouldShowDivider()).isTrue();
     }
 
     @Test
     public void testCaptivePortalStatus_isNotCaptivePortal_dividerNotDrawn() {
-        Mockito.when(mCaptivePortalStatus.isCaptivePortalNetwork()).thenReturn(false);
+        mConnectedAccessPointPreference.setCaptivePortal(false);
-        mConnectedAccessPointPreference.refresh();
         assertThat(mConnectedAccessPointPreference.shouldShowDivider()).isFalse();
     }