From 065ab6ee407dc8b3b5b477e483b6f48e50720113 Mon Sep 17 00:00:00 2001 From: Erik Kline Date: Sun, 2 Oct 2016 18:02:14 +0900 Subject: [PATCH] Refactor "avoid bad wifi" logic into a utility class Additionally, add this utility class to IpManager for compatibility verification. A follow-on CL will make use of IpManager's local AvoidBadWifiTracker. Bug: 31827713 Change-Id: If8c56c3f8076d6a5157ea180e361bbdadc2bc1dd --- .../com/android/server/ConnectivityService.java | 89 ++++--------- services/net/java/android/net/ip/IpManager.java | 4 + .../java/android/net/util/AvoidBadWifiTracker.java | 139 +++++++++++++++++++++ .../android/server/ConnectivityServiceTest.java | 74 +++++++---- 4 files changed, 216 insertions(+), 90 deletions(-) create mode 100644 services/net/java/android/net/util/AvoidBadWifiTracker.java diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 9c3c537be654..989892f1296e 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -77,6 +77,7 @@ import android.net.Uri; import android.net.metrics.DefaultNetworkEvent; import android.net.metrics.IpConnectivityLog; import android.net.metrics.NetworkEvent; +import android.net.util.AvoidBadWifiTracker; import android.os.Binder; import android.os.Build; import android.os.Bundle; @@ -397,11 +398,6 @@ public class ConnectivityService extends IConnectivityManager.Stub private static final int EVENT_REQUEST_LINKPROPERTIES = 32; private static final int EVENT_REQUEST_NETCAPABILITIES = 33; - /** - * Used internally to (re)configure avoid bad wifi setting. - */ - private static final int EVENT_CONFIGURE_NETWORK_AVOID_BAD_WIFI = 34; - /** Handler thread used for both of the handlers below. */ @VisibleForTesting protected final HandlerThread mHandlerThread; @@ -495,6 +491,9 @@ public class ConnectivityService extends IConnectivityManager.Stub private final IpConnectivityLog mMetricsLog; + @VisibleForTesting + final AvoidBadWifiTracker mAvoidBadWifiTracker; + /** * Implements support for the legacy "one network per network type" model. * @@ -857,14 +856,8 @@ public class ConnectivityService extends IConnectivityManager.Stub LingerMonitor.DEFAULT_NOTIFICATION_RATE_LIMIT_MILLIS); mLingerMonitor = new LingerMonitor(mContext, mNotifier, dailyLimit, rateLimit); - intentFilter = new IntentFilter(); - intentFilter.addAction(Intent.ACTION_CONFIGURATION_CHANGED); - mContext.registerReceiverAsUser(new BroadcastReceiver() { - public void onReceive(Context context, Intent intent) { - mHandler.sendEmptyMessage(EVENT_CONFIGURE_NETWORK_AVOID_BAD_WIFI); - } - }, UserHandle.ALL, intentFilter, null, null); - updateAvoidBadWifi(); + mAvoidBadWifiTracker = createAvoidBadWifiTracker( + mContext, mHandler, () -> rematchForAvoidBadWifiUpdate()); } private NetworkRequest createInternetRequestForTransport(int transportType) { @@ -915,12 +908,6 @@ public class ConnectivityService extends IConnectivityManager.Stub mSettingsObserver.observe( Settings.Global.getUriFor(Settings.Global.MOBILE_DATA_ALWAYS_ON), EVENT_CONFIGURE_MOBILE_DATA_ALWAYS_ON); - - // Watch for whether to automatically switch away from wifi networks that lose Internet - // access. - mSettingsObserver.observe( - Settings.Global.getUriFor(Settings.Global.NETWORK_AVOID_BAD_WIFI), - EVENT_CONFIGURE_NETWORK_AVOID_BAD_WIFI); } private synchronized int nextNetworkRequestId() { @@ -2755,36 +2742,23 @@ public class ConnectivityService extends IConnectivityManager.Stub PROMPT_UNVALIDATED_DELAY_MS); } - private boolean mAvoidBadWifi = true; - public boolean avoidBadWifi() { - return mAvoidBadWifi; - } - - @VisibleForTesting - /** Whether the device or carrier configuration disables avoiding bad wifi by default. */ - public boolean configRestrictsAvoidBadWifi() { - return mContext.getResources().getInteger(R.integer.config_networkAvoidBadWifi) == 0; + return mAvoidBadWifiTracker.currentValue(); } - /** Whether we should display a notification when wifi becomes unvalidated. */ - public boolean shouldNotifyWifiUnvalidated() { - return configRestrictsAvoidBadWifi() && - Settings.Global.getString(mContext.getContentResolver(), - Settings.Global.NETWORK_AVOID_BAD_WIFI) == null; - } - - private boolean updateAvoidBadWifi() { - boolean settingAvoidBadWifi = "1".equals(Settings.Global.getString( - mContext.getContentResolver(), Settings.Global.NETWORK_AVOID_BAD_WIFI)); - - boolean prev = mAvoidBadWifi; - mAvoidBadWifi = settingAvoidBadWifi || !configRestrictsAvoidBadWifi(); - return mAvoidBadWifi != prev; + private void rematchForAvoidBadWifiUpdate() { + rematchAllNetworksAndRequests(null, 0); + for (NetworkAgentInfo nai: mNetworkAgentInfos.values()) { + if (nai.networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI)) { + sendUpdatedScoreToFactories(nai); + } + } } + // TODO: Evaluate whether this is of interest to other consumers of + // AvoidBadWifiTracker and worth moving out of here. private void dumpAvoidBadWifiSettings(IndentingPrintWriter pw) { - boolean configRestrict = configRestrictsAvoidBadWifi(); + final boolean configRestrict = mAvoidBadWifiTracker.configRestrictsAvoidBadWifi(); if (!configRestrict) { pw.println("Bad Wi-Fi avoidance: unrestricted"); return; @@ -2794,8 +2768,7 @@ public class ConnectivityService extends IConnectivityManager.Stub pw.increaseIndent(); pw.println("Config restrict: " + configRestrict); - String value = Settings.Global.getString( - mContext.getContentResolver(), Settings.Global.NETWORK_AVOID_BAD_WIFI); + final String value = mAvoidBadWifiTracker.getSettingsValue(); String description; // Can't use a switch statement because strings are legal case labels, but null is not. if ("0".equals(value)) { @@ -2858,17 +2831,12 @@ public class ConnectivityService extends IConnectivityManager.Stub showValidationNotification(nai, NotificationType.NO_INTERNET); } - // TODO: Delete this like updateMobileDataAlwaysOn above. - @VisibleForTesting - void updateNetworkAvoidBadWifi() { - mHandler.sendEmptyMessage(EVENT_CONFIGURE_NETWORK_AVOID_BAD_WIFI); - } - private void handleNetworkUnvalidated(NetworkAgentInfo nai) { NetworkCapabilities nc = nai.networkCapabilities; if (DBG) log("handleNetworkUnvalidated " + nai.name() + " cap=" + nc); - if (nc.hasTransport(NetworkCapabilities.TRANSPORT_WIFI) && shouldNotifyWifiUnvalidated()) { + if (nc.hasTransport(NetworkCapabilities.TRANSPORT_WIFI) && + mAvoidBadWifiTracker.shouldNotifyWifiUnvalidated()) { showValidationNotification(nai, NotificationType.LOST_INTERNET); } } @@ -2958,18 +2926,6 @@ public class ConnectivityService extends IConnectivityManager.Stub handleMobileDataAlwaysOn(); break; } - case EVENT_CONFIGURE_NETWORK_AVOID_BAD_WIFI: { - if (updateAvoidBadWifi()) { - rematchAllNetworksAndRequests(null, 0); - for (NetworkAgentInfo nai: mNetworkAgentInfos.values()) { - if (nai.networkCapabilities.hasTransport( - NetworkCapabilities.TRANSPORT_WIFI)) { - sendUpdatedScoreToFactories(nai); - } - } - } - break; - } case EVENT_REQUEST_LINKPROPERTIES: handleRequestLinkProperties((NetworkRequest) msg.obj, msg.arg1); break; @@ -5443,6 +5399,11 @@ public class ConnectivityService extends IConnectivityManager.Stub } @VisibleForTesting + AvoidBadWifiTracker createAvoidBadWifiTracker(Context c, Handler h, Runnable r) { + return new AvoidBadWifiTracker(c, h, r); + } + + @VisibleForTesting public WakeupMessage makeWakeupMessage(Context c, Handler h, String s, int cmd, Object obj) { return new WakeupMessage(c, h, s, cmd, 0, 0, obj); } diff --git a/services/net/java/android/net/ip/IpManager.java b/services/net/java/android/net/ip/IpManager.java index 654ef18f9608..fc91f512c21f 100644 --- a/services/net/java/android/net/ip/IpManager.java +++ b/services/net/java/android/net/ip/IpManager.java @@ -33,6 +33,7 @@ import android.net.StaticIpConfiguration; import android.net.dhcp.DhcpClient; import android.net.metrics.IpConnectivityLog; import android.net.metrics.IpManagerEvent; +import android.net.util.AvoidBadWifiTracker; import android.os.INetworkManagementService; import android.os.Message; import android.os.RemoteException; @@ -393,6 +394,7 @@ public class IpManager extends StateMachine { private final NetlinkTracker mNetlinkTracker; private final WakeupMessage mProvisioningTimeoutAlarm; private final WakeupMessage mDhcpActionTimeoutAlarm; + private final AvoidBadWifiTracker mAvoidBadWifiTracker; private final LocalLog mLocalLog; private final IpConnectivityLog mMetricsLog = new IpConnectivityLog(); @@ -466,6 +468,8 @@ public class IpManager extends StateMachine { Log.e(mTag, "Couldn't register NetlinkTracker: " + e.toString()); } + mAvoidBadWifiTracker = new AvoidBadWifiTracker(mContext, getHandler()); + resetLinkProperties(); mProvisioningTimeoutAlarm = new WakeupMessage(mContext, getHandler(), diff --git a/services/net/java/android/net/util/AvoidBadWifiTracker.java b/services/net/java/android/net/util/AvoidBadWifiTracker.java new file mode 100644 index 000000000000..c14e811e0584 --- /dev/null +++ b/services/net/java/android/net/util/AvoidBadWifiTracker.java @@ -0,0 +1,139 @@ +/* + * Copyright (C) 2016 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 android.net.util; + +import android.content.BroadcastReceiver; +import android.content.ContentResolver; +import android.content.Context; +import android.content.Intent; +import android.content.IntentFilter; +import android.database.ContentObserver; +import android.net.Uri; +import android.os.Handler; +import android.os.Message; +import android.os.UserHandle; +import android.provider.Settings; +import android.util.Slog; + +import com.android.internal.annotations.VisibleForTesting; +import com.android.internal.R; + +import static android.provider.Settings.Global.NETWORK_AVOID_BAD_WIFI; + +/** + * A class to encapsulate management of the "Smart Networking" capability of + * avoiding bad Wi-Fi when, for example upstream connectivity is lost or + * certain critical link failures occur. + * + * This enables the device to switch to another form of connectivity, like + * mobile, if it's available and working. + * + * The Runnable |cb|, if given, is called on the supplied Handler's thread + * whether the computed "avoid bad wifi" value changes. + * + * Disabling this reverts the device to a level of networking sophistication + * circa 2012-13 by disabling disparate code paths each of which contribute to + * maintaining continuous, working Internet connectivity. + * + * @hide + */ +public class AvoidBadWifiTracker { + private static String TAG = AvoidBadWifiTracker.class.getSimpleName(); + + private final Context mContext; + private final Handler mHandler; + private final Runnable mReevaluateRunnable; + private final SettingObserver mSettingObserver; + private volatile boolean mAvoidBadWifi = true; + + public AvoidBadWifiTracker(Context ctx, Handler handler) { + this(ctx, handler, null); + } + + public AvoidBadWifiTracker(Context ctx, Handler handler, Runnable cb) { + mContext = ctx; + mHandler = handler; + mReevaluateRunnable = () -> { if (update() && cb != null) cb.run(); }; + mSettingObserver = new SettingObserver(); + + final IntentFilter intentFilter = new IntentFilter(); + intentFilter.addAction(Intent.ACTION_CONFIGURATION_CHANGED); + mContext.registerReceiverAsUser(new BroadcastReceiver() { + public void onReceive(Context context, Intent intent) { + reevaluate(); + } + }, UserHandle.ALL, intentFilter, null, null); + + update(); + } + + public boolean currentValue() { + return mAvoidBadWifi; + } + + /** + * Whether the device or carrier configuration disables avoiding bad wifi by default. + */ + public boolean configRestrictsAvoidBadWifi() { + return (mContext.getResources().getInteger(R.integer.config_networkAvoidBadWifi) == 0); + } + + /** + * Whether we should display a notification when wifi becomes unvalidated. + */ + public boolean shouldNotifyWifiUnvalidated() { + return configRestrictsAvoidBadWifi() && getSettingsValue() == null; + } + + public String getSettingsValue() { + final ContentResolver resolver = mContext.getContentResolver(); + return Settings.Global.getString(resolver, NETWORK_AVOID_BAD_WIFI); + } + + @VisibleForTesting + public void reevaluate() { + mHandler.post(mReevaluateRunnable); + } + + public boolean update() { + final boolean settingAvoidBadWifi = "1".equals(getSettingsValue()); + final boolean prev = mAvoidBadWifi; + mAvoidBadWifi = settingAvoidBadWifi || !configRestrictsAvoidBadWifi(); + return mAvoidBadWifi != prev; + } + + private class SettingObserver extends ContentObserver { + private final Uri mUri = Settings.Global.getUriFor(NETWORK_AVOID_BAD_WIFI); + + public SettingObserver() { + super(null); + final ContentResolver resolver = mContext.getContentResolver(); + resolver.registerContentObserver(mUri, false, this); + } + + @Override + public void onChange(boolean selfChange) { + Slog.wtf(TAG, "Should never be reached."); + } + + @Override + public void onChange(boolean selfChange, Uri uri) { + if (!mUri.equals(uri)) return; + reevaluate(); + } + } +} diff --git a/services/tests/servicestests/src/com/android/server/ConnectivityServiceTest.java b/services/tests/servicestests/src/com/android/server/ConnectivityServiceTest.java index 2055d16f3286..a656accb9f7e 100644 --- a/services/tests/servicestests/src/com/android/server/ConnectivityServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/ConnectivityServiceTest.java @@ -53,6 +53,7 @@ import android.net.NetworkMisc; import android.net.NetworkRequest; import android.net.RouteInfo; import android.net.metrics.IpConnectivityLog; +import android.net.util.AvoidBadWifiTracker; import android.os.ConditionVariable; import android.os.Handler; import android.os.HandlerThread; @@ -600,9 +601,22 @@ public class ConnectivityServiceTest extends AndroidTestCase { } } + private class WrappedAvoidBadWifiTracker extends AvoidBadWifiTracker { + public boolean configRestrictsAvoidBadWifi; + + public WrappedAvoidBadWifiTracker(Context c, Handler h, Runnable r) { + super(c, h, r); + } + + @Override + public boolean configRestrictsAvoidBadWifi() { + return configRestrictsAvoidBadWifi; + } + } + private class WrappedConnectivityService extends ConnectivityService { + public WrappedAvoidBadWifiTracker wrappedAvoidBadWifiTracker; private WrappedNetworkMonitor mLastCreatedNetworkMonitor; - public boolean configRestrictsAvoidBadWifi; public WrappedConnectivityService(Context context, INetworkManagementService netManager, INetworkStatsService statsService, INetworkPolicyManager policyManager, @@ -653,14 +667,20 @@ public class ConnectivityServiceTest extends AndroidTestCase { } @Override - public WakeupMessage makeWakeupMessage( - Context context, Handler handler, String cmdName, int cmd, Object obj) { - return new FakeWakeupMessage(context, handler, cmdName, cmd, 0, 0, obj); + public AvoidBadWifiTracker createAvoidBadWifiTracker( + Context c, Handler h, Runnable r) { + final WrappedAvoidBadWifiTracker tracker = new WrappedAvoidBadWifiTracker(c, h, r); + return tracker; + } + + public WrappedAvoidBadWifiTracker getAvoidBadWifiTracker() { + return (WrappedAvoidBadWifiTracker) mAvoidBadWifiTracker; } @Override - public boolean configRestrictsAvoidBadWifi() { - return configRestrictsAvoidBadWifi; + public WakeupMessage makeWakeupMessage( + Context context, Handler handler, String cmdName, int cmd, Object obj) { + return new FakeWakeupMessage(context, handler, cmdName, cmd, 0, 0, obj); } public WrappedNetworkMonitor getLastCreatedWrappedNetworkMonitor() { @@ -1965,46 +1985,48 @@ public class ConnectivityServiceTest extends AndroidTestCase { @SmallTest public void testAvoidBadWifiSetting() throws Exception { final ContentResolver cr = mServiceContext.getContentResolver(); + final WrappedAvoidBadWifiTracker tracker = mService.getAvoidBadWifiTracker(); final String settingName = Settings.Global.NETWORK_AVOID_BAD_WIFI; - mService.configRestrictsAvoidBadWifi = false; + tracker.configRestrictsAvoidBadWifi = false; String[] values = new String[] {null, "0", "1"}; for (int i = 0; i < values.length; i++) { Settings.Global.putInt(cr, settingName, 1); - mService.updateNetworkAvoidBadWifi(); + tracker.reevaluate(); mService.waitForIdle(); String msg = String.format("config=false, setting=%s", values[i]); assertTrue(msg, mService.avoidBadWifi()); - assertFalse(msg, mService.shouldNotifyWifiUnvalidated()); + assertFalse(msg, tracker.shouldNotifyWifiUnvalidated()); } - mService.configRestrictsAvoidBadWifi = true; + tracker.configRestrictsAvoidBadWifi = true; Settings.Global.putInt(cr, settingName, 0); - mService.updateNetworkAvoidBadWifi(); + tracker.reevaluate(); mService.waitForIdle(); assertFalse(mService.avoidBadWifi()); - assertFalse(mService.shouldNotifyWifiUnvalidated()); + assertFalse(tracker.shouldNotifyWifiUnvalidated()); Settings.Global.putInt(cr, settingName, 1); - mService.updateNetworkAvoidBadWifi(); + tracker.reevaluate(); mService.waitForIdle(); assertTrue(mService.avoidBadWifi()); - assertFalse(mService.shouldNotifyWifiUnvalidated()); + assertFalse(tracker.shouldNotifyWifiUnvalidated()); Settings.Global.putString(cr, settingName, null); - mService.updateNetworkAvoidBadWifi(); + tracker.reevaluate(); mService.waitForIdle(); assertFalse(mService.avoidBadWifi()); - assertTrue(mService.shouldNotifyWifiUnvalidated()); + assertTrue(tracker.shouldNotifyWifiUnvalidated()); } @SmallTest public void testAvoidBadWifi() throws Exception { - ContentResolver cr = mServiceContext.getContentResolver(); + final ContentResolver cr = mServiceContext.getContentResolver(); + final WrappedAvoidBadWifiTracker tracker = mService.getAvoidBadWifiTracker(); // Pretend we're on a carrier that restricts switching away from bad wifi. - mService.configRestrictsAvoidBadWifi = true; + tracker.configRestrictsAvoidBadWifi = true; // File a request for cell to ensure it doesn't go down. final TestNetworkCallback cellNetworkCallback = new TestNetworkCallback(); @@ -2023,7 +2045,7 @@ public class ConnectivityServiceTest extends AndroidTestCase { mCm.registerNetworkCallback(validatedWifiRequest, validatedWifiCallback); Settings.Global.putInt(cr, Settings.Global.NETWORK_AVOID_BAD_WIFI, 0); - mService.updateNetworkAvoidBadWifi(); + tracker.reevaluate(); // Bring up validated cell. mCellNetworkAgent = new MockNetworkAgent(TRANSPORT_CELLULAR); @@ -2054,14 +2076,14 @@ public class ConnectivityServiceTest extends AndroidTestCase { // Simulate switching to a carrier that does not restrict avoiding bad wifi, and expect // that we switch back to cell. - mService.configRestrictsAvoidBadWifi = false; - mService.updateNetworkAvoidBadWifi(); + tracker.configRestrictsAvoidBadWifi = false; + tracker.reevaluate(); defaultCallback.expectCallback(CallbackState.AVAILABLE, mCellNetworkAgent); assertEquals(mCm.getActiveNetwork(), cellNetwork); // Switch back to a restrictive carrier. - mService.configRestrictsAvoidBadWifi = true; - mService.updateNetworkAvoidBadWifi(); + tracker.configRestrictsAvoidBadWifi = true; + tracker.reevaluate(); defaultCallback.expectCallback(CallbackState.AVAILABLE, mWiFiNetworkAgent); assertEquals(mCm.getActiveNetwork(), wifiNetwork); @@ -2089,7 +2111,7 @@ public class ConnectivityServiceTest extends AndroidTestCase { // Simulate the user selecting "switch" and checking the don't ask again checkbox. Settings.Global.putInt(cr, Settings.Global.NETWORK_AVOID_BAD_WIFI, 1); - mService.updateNetworkAvoidBadWifi(); + tracker.reevaluate(); // We now switch to cell. defaultCallback.expectCallback(CallbackState.AVAILABLE, mCellNetworkAgent); @@ -2102,11 +2124,11 @@ public class ConnectivityServiceTest extends AndroidTestCase { // Simulate the user turning the cellular fallback setting off and then on. // We switch to wifi and then to cell. Settings.Global.putString(cr, Settings.Global.NETWORK_AVOID_BAD_WIFI, null); - mService.updateNetworkAvoidBadWifi(); + tracker.reevaluate(); defaultCallback.expectCallback(CallbackState.AVAILABLE, mWiFiNetworkAgent); assertEquals(mCm.getActiveNetwork(), wifiNetwork); Settings.Global.putInt(cr, Settings.Global.NETWORK_AVOID_BAD_WIFI, 1); - mService.updateNetworkAvoidBadWifi(); + tracker.reevaluate(); defaultCallback.expectCallback(CallbackState.AVAILABLE, mCellNetworkAgent); assertEquals(mCm.getActiveNetwork(), cellNetwork); -- 2.11.0