Merge "Observe NetworkCallback instead of polling" into pi-dev

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();
     }