From 5178a9d07a501262f92ef6e37227ada57ab32954 Mon Sep 17 00:00:00 2001 From: Adam Newman Date: Mon, 19 Mar 2018 21:09:52 -0700 Subject: [PATCH] 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 --- .../wifi/CaptivePortalNetworkCallback.java | 74 ++++++++++++++ .../wifi/ConnectedAccessPointPreference.java | 26 ++--- .../android/settings/wifi/WifiSettings.java | 98 +++++++++++-------- .../ConnectedAccessPointPreferenceTest.java | 12 +-- 4 files changed, 149 insertions(+), 61 deletions(-) create mode 100644 src/com/android/settings/wifi/CaptivePortalNetworkCallback.java diff --git a/src/com/android/settings/wifi/CaptivePortalNetworkCallback.java b/src/com/android/settings/wifi/CaptivePortalNetworkCallback.java new file mode 100644 index 00000000000..9bcfba021e3 --- /dev/null +++ b/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; + } +} diff --git a/src/com/android/settings/wifi/ConnectedAccessPointPreference.java b/src/com/android/settings/wifi/ConnectedAccessPointPreference.java index e9e1560cfbb..def48e00e54 100644 --- a/src/com/android/settings/wifi/ConnectedAccessPointPreference.java +++ b/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, - CaptivePortalStatus captivePortalStatus) { + UserBadgeCache cache, @DrawableRes int iconResId, boolean forSavedNetworks) { 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(mCaptivePortalNetwork); - if (mCaptivePortalNetwork) { + setShowDivider(mIsCaptivePortal); + if (mIsCaptivePortal) { 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); - gear.setVisibility(mCaptivePortalNetwork ? View.VISIBLE : View.INVISIBLE); + gearNoBg.setVisibility(mIsCaptivePortal ? View.INVISIBLE : View.VISIBLE); + 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(); - } } diff --git a/src/com/android/settings/wifi/WifiSettings.java b/src/com/android/settings/wifi/WifiSettings.java index 7c1b725fac8..0f6a0bbb7ba 100644 --- a/src/com/android/settings/wifi/WifiSettings.java +++ b/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, - ConnectedAccessPointPreference.CaptivePortalStatus captivePortalStatus) { + AccessPoint accessPoint) { return new ConnectedAccessPointPreference(accessPoint, getPrefContext(), mUserBadgeCache, - R.drawable.ic_wifi_signal_0, false /* forSavedNetworks */, - captivePortalStatus); + R.drawable.ic_wifi_signal_0, false /* forSavedNetworks */); } /** @@ -817,8 +819,9 @@ public class WifiSettings extends RestrictedSettingsFragment } // Is the previous currently connected SSID different from the new one? - AccessPointPreference preference = (AccessPointPreference) - (mConnectedAccessPointPreferenceCategory.getPreference(0)); + ConnectedAccessPointPreference preference = + (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)) - .refresh(); + preference.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( - connectedAp, this::isConnectedToCaptivePortalNetwork); + createConnectedAccessPointPreference(connectedAp); + registerCaptivePortalNetworkCallback(getCurrentWifiNetwork(), pref); // Launch details page or captive portal on click. pref.setOnPreferenceClickListener( preference -> { pref.getAccessPoint().saveWifiState(pref.getExtras()); - Network network = getConnectedWifiNetwork(); - if (isConnectedToCaptivePortalNetwork(network)) { + if (mCaptivePortalNetworkCallback != null + && 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() { - return isConnectedToCaptivePortalNetwork(getConnectedWifiNetwork()); - } - - 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; + private Network getCurrentWifiNetwork() { + return mWifiManager != null ? mWifiManager.getCurrentNetwork() : null; } /** Removes all preferences and hide the {@link #mConnectedAccessPointPreferenceCategory}. */ private void removeConnectedAccessPointPreference() { mConnectedAccessPointPreferenceCategory.removeAll(); mConnectedAccessPointPreferenceCategory.setVisible(false); + unregisterCaptivePortalNetworkCallback(); } private void setAdditionalSettingsSummaries() { diff --git a/tests/robotests/src/com/android/settings/wifi/ConnectedAccessPointPreferenceTest.java b/tests/robotests/src/com/android/settings/wifi/ConnectedAccessPointPreferenceTest.java index b9b29d27133..452fe0346da 100644 --- a/tests/robotests/src/com/android/settings/wifi/ConnectedAccessPointPreferenceTest.java +++ b/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.refresh(); + mConnectedAccessPointPreference.setCaptivePortal(true); assertThat(mConnectedAccessPointPreference.shouldShowDivider()).isTrue(); } @Test public void testCaptivePortalStatus_isNotCaptivePortal_dividerNotDrawn() { - Mockito.when(mCaptivePortalStatus.isCaptivePortalNetwork()).thenReturn(false); - mConnectedAccessPointPreference.refresh(); + mConnectedAccessPointPreference.setCaptivePortal(false); assertThat(mConnectedAccessPointPreference.shouldShowDivider()).isFalse(); }