Merge changes If420bd66,Icbb161ca,Ida190e5a,Iae75781f

* changes:
  ConnectivityServiceTest: remove remaining poll loops.
  Remove assertEventuallyTrue as it's unnecessary.
  Rename AvoidBadWifiTracker to MultinetworkPolicyTracker
  Attempt to make waitForIdleHandler reliable.
This commit is contained in:
Lorenzo Colitti
2017-02-01 02:25:42 +00:00
committed by Gerrit Code Review
5 changed files with 69 additions and 104 deletions

View File

@@ -78,7 +78,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.net.util.MultinetworkPolicyTracker;
import android.os.Binder;
import android.os.Build;
import android.os.Bundle;
@@ -499,7 +499,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
private final IpConnectivityLog mMetricsLog;
@VisibleForTesting
final AvoidBadWifiTracker mAvoidBadWifiTracker;
final MultinetworkPolicyTracker mMultinetworkPolicyTracker;
/**
* Implements support for the legacy "one network per network type" model.
@@ -849,9 +849,9 @@ public class ConnectivityService extends IConnectivityManager.Stub
LingerMonitor.DEFAULT_NOTIFICATION_RATE_LIMIT_MILLIS);
mLingerMonitor = new LingerMonitor(mContext, mNotifier, dailyLimit, rateLimit);
mAvoidBadWifiTracker = createAvoidBadWifiTracker(
mMultinetworkPolicyTracker = createMultinetworkPolicyTracker(
mContext, mHandler, () -> rematchForAvoidBadWifiUpdate());
mAvoidBadWifiTracker.start();
mMultinetworkPolicyTracker.start();
}
private NetworkRequest createInternetRequestForTransport(
@@ -2793,7 +2793,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
}
public boolean avoidBadWifi() {
return mAvoidBadWifiTracker.currentValue();
return mMultinetworkPolicyTracker.getAvoidBadWifi();
}
private void rematchForAvoidBadWifiUpdate() {
@@ -2806,9 +2806,9 @@ public class ConnectivityService extends IConnectivityManager.Stub
}
// TODO: Evaluate whether this is of interest to other consumers of
// AvoidBadWifiTracker and worth moving out of here.
// MultinetworkPolicyTracker and worth moving out of here.
private void dumpAvoidBadWifiSettings(IndentingPrintWriter pw) {
final boolean configRestrict = mAvoidBadWifiTracker.configRestrictsAvoidBadWifi();
final boolean configRestrict = mMultinetworkPolicyTracker.configRestrictsAvoidBadWifi();
if (!configRestrict) {
pw.println("Bad Wi-Fi avoidance: unrestricted");
return;
@@ -2818,7 +2818,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
pw.increaseIndent();
pw.println("Config restrict: " + configRestrict);
final String value = mAvoidBadWifiTracker.getSettingsValue();
final String value = mMultinetworkPolicyTracker.getAvoidBadWifiSetting();
String description;
// Can't use a switch statement because strings are legal case labels, but null is not.
if ("0".equals(value)) {
@@ -2886,7 +2886,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
if (DBG) log("handleNetworkUnvalidated " + nai.name() + " cap=" + nc);
if (nc.hasTransport(NetworkCapabilities.TRANSPORT_WIFI) &&
mAvoidBadWifiTracker.shouldNotifyWifiUnvalidated()) {
mMultinetworkPolicyTracker.shouldNotifyWifiUnvalidated()) {
showValidationNotification(nai, NotificationType.LOST_INTERNET);
}
}
@@ -5534,8 +5534,8 @@ public class ConnectivityService extends IConnectivityManager.Stub
}
@VisibleForTesting
AvoidBadWifiTracker createAvoidBadWifiTracker(Context c, Handler h, Runnable r) {
return new AvoidBadWifiTracker(c, h, r);
MultinetworkPolicyTracker createMultinetworkPolicyTracker(Context c, Handler h, Runnable r) {
return new MultinetworkPolicyTracker(c, h, r);
}
@VisibleForTesting

View File

@@ -33,7 +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.net.util.MultinetworkPolicyTracker;
import android.os.INetworkManagementService;
import android.os.Message;
import android.os.RemoteException;
@@ -398,7 +398,7 @@ public class IpManager extends StateMachine {
private final NetlinkTracker mNetlinkTracker;
private final WakeupMessage mProvisioningTimeoutAlarm;
private final WakeupMessage mDhcpActionTimeoutAlarm;
private final AvoidBadWifiTracker mAvoidBadWifiTracker;
private final MultinetworkPolicyTracker mMultinetworkPolicyTracker;
private final LocalLog mLocalLog;
private final LocalLog mConnectivityPacketLog;
private final MessageHandlingLogger mMsgStateLogger;
@@ -492,7 +492,7 @@ public class IpManager extends StateMachine {
mLinkProperties = new LinkProperties();
mLinkProperties.setInterfaceName(mInterfaceName);
mAvoidBadWifiTracker = new AvoidBadWifiTracker(mContext, getHandler(),
mMultinetworkPolicyTracker = new MultinetworkPolicyTracker(mContext, getHandler(),
() -> { mLocalLog.log("OBSERVED AvoidBadWifi changed"); });
mProvisioningTimeoutAlarm = new WakeupMessage(mContext, getHandler(),
@@ -527,7 +527,7 @@ public class IpManager extends StateMachine {
Log.e(mTag, "Couldn't register NetlinkTracker: " + e.toString());
}
mAvoidBadWifiTracker.start();
mMultinetworkPolicyTracker.start();
}
@Override
@@ -538,7 +538,7 @@ public class IpManager extends StateMachine {
// Shut down this IpManager instance altogether.
public void shutdown() {
stop();
mAvoidBadWifiTracker.shutdown();
mMultinetworkPolicyTracker.shutdown();
quit();
}
@@ -767,7 +767,7 @@ public class IpManager extends StateMachine {
// Note that we can still be disconnected by IpReachabilityMonitor
// if the IPv6 default gateway (but not the IPv6 DNS servers; see
// accompanying code in IpReachabilityMonitor) is unreachable.
final boolean ignoreIPv6ProvisioningLoss = !mAvoidBadWifiTracker.currentValue();
final boolean ignoreIPv6ProvisioningLoss = !mMultinetworkPolicyTracker.getAvoidBadWifi();
// Additionally:
//
@@ -1045,7 +1045,7 @@ public class IpManager extends StateMachine {
mCallback.onReachabilityLost(logMsg);
}
},
mAvoidBadWifiTracker);
mMultinetworkPolicyTracker);
} catch (IllegalArgumentException iae) {
// Failed to start IpReachabilityMonitor. Log it and call
// onProvisioningFailure() immediately.

View File

@@ -34,7 +34,7 @@ import android.net.netlink.RtNetlinkNeighborMessage;
import android.net.netlink.StructNdaCacheInfo;
import android.net.netlink.StructNdMsg;
import android.net.netlink.StructNlMsgHdr;
import android.net.util.AvoidBadWifiTracker;
import android.net.util.MultinetworkPolicyTracker;
import android.os.PowerManager;
import android.os.SystemClock;
import android.system.ErrnoException;
@@ -151,7 +151,7 @@ public class IpReachabilityMonitor {
private final String mInterfaceName;
private final int mInterfaceIndex;
private final Callback mCallback;
private final AvoidBadWifiTracker mAvoidBadWifiTracker;
private final MultinetworkPolicyTracker mMultinetworkPolicyTracker;
private final NetlinkSocketObserver mNetlinkSocketObserver;
private final Thread mObserverThread;
private final IpConnectivityLog mMetricsLog = new IpConnectivityLog();
@@ -226,7 +226,7 @@ public class IpReachabilityMonitor {
}
public IpReachabilityMonitor(Context context, String ifName, Callback callback,
AvoidBadWifiTracker tracker) throws IllegalArgumentException {
MultinetworkPolicyTracker tracker) throws IllegalArgumentException {
mInterfaceName = ifName;
int ifIndex = -1;
try {
@@ -238,7 +238,7 @@ public class IpReachabilityMonitor {
mWakeLock = ((PowerManager) context.getSystemService(Context.POWER_SERVICE)).newWakeLock(
PowerManager.PARTIAL_WAKE_LOCK, TAG + "." + mInterfaceName);
mCallback = callback;
mAvoidBadWifiTracker = tracker;
mMultinetworkPolicyTracker = tracker;
mNetlinkSocketObserver = new NetlinkSocketObserver();
mObserverThread = new Thread(mNetlinkSocketObserver);
mObserverThread.start();
@@ -379,7 +379,7 @@ public class IpReachabilityMonitor {
}
private boolean avoidingBadLinks() {
return (mAvoidBadWifiTracker != null) ? mAvoidBadWifiTracker.currentValue() : true;
return (mMultinetworkPolicyTracker == null) || mMultinetworkPolicyTracker.getAvoidBadWifi();
}
public void probeAll() {

View File

@@ -42,8 +42,8 @@ import static android.provider.Settings.Global.NETWORK_AVOID_BAD_WIFI;
* 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.
* The Runnable |avoidBadWifiCallback|, if given, is posted to the supplied
* Handler' whenever 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
@@ -51,28 +51,30 @@ import static android.provider.Settings.Global.NETWORK_AVOID_BAD_WIFI;
*
* @hide
*/
public class AvoidBadWifiTracker {
private static String TAG = AvoidBadWifiTracker.class.getSimpleName();
public class MultinetworkPolicyTracker {
private static String TAG = MultinetworkPolicyTracker.class.getSimpleName();
private final Context mContext;
private final Handler mHandler;
private final Runnable mReevaluateRunnable;
private final Uri mUri;
private final Uri mAvoidBadWifiUri;
private final ContentResolver mResolver;
private final SettingObserver mSettingObserver;
private final BroadcastReceiver mBroadcastReceiver;
private volatile boolean mAvoidBadWifi = true;
public AvoidBadWifiTracker(Context ctx, Handler handler) {
public MultinetworkPolicyTracker(Context ctx, Handler handler) {
this(ctx, handler, null);
}
public AvoidBadWifiTracker(Context ctx, Handler handler, Runnable cb) {
public MultinetworkPolicyTracker(Context ctx, Handler handler, Runnable avoidBadWifiCallback) {
mContext = ctx;
mHandler = handler;
mReevaluateRunnable = () -> { if (update() && cb != null) cb.run(); };
mUri = Settings.Global.getUriFor(NETWORK_AVOID_BAD_WIFI);
mReevaluateRunnable = () -> {
if (updateAvoidBadWifi() && avoidBadWifiCallback != null) avoidBadWifiCallback.run();
};
mAvoidBadWifiUri = Settings.Global.getUriFor(NETWORK_AVOID_BAD_WIFI);
mResolver = mContext.getContentResolver();
mSettingObserver = new SettingObserver();
mBroadcastReceiver = new BroadcastReceiver() {
@@ -82,11 +84,11 @@ public class AvoidBadWifiTracker {
}
};
update();
updateAvoidBadWifi();
}
public void start() {
mResolver.registerContentObserver(mUri, false, mSettingObserver);
mResolver.registerContentObserver(mAvoidBadWifiUri, false, mSettingObserver);
final IntentFilter intentFilter = new IntentFilter();
intentFilter.addAction(Intent.ACTION_CONFIGURATION_CHANGED);
@@ -102,7 +104,7 @@ public class AvoidBadWifiTracker {
mContext.unregisterReceiver(mBroadcastReceiver);
}
public boolean currentValue() {
public boolean getAvoidBadWifi() {
return mAvoidBadWifi;
}
@@ -117,10 +119,10 @@ public class AvoidBadWifiTracker {
* Whether we should display a notification when wifi becomes unvalidated.
*/
public boolean shouldNotifyWifiUnvalidated() {
return configRestrictsAvoidBadWifi() && getSettingsValue() == null;
return configRestrictsAvoidBadWifi() && getAvoidBadWifiSetting() == null;
}
public String getSettingsValue() {
public String getAvoidBadWifiSetting() {
return Settings.Global.getString(mResolver, NETWORK_AVOID_BAD_WIFI);
}
@@ -129,8 +131,8 @@ public class AvoidBadWifiTracker {
mHandler.post(mReevaluateRunnable);
}
public boolean update() {
final boolean settingAvoidBadWifi = "1".equals(getSettingsValue());
public boolean updateAvoidBadWifi() {
final boolean settingAvoidBadWifi = "1".equals(getAvoidBadWifiSetting());
final boolean prev = mAvoidBadWifi;
mAvoidBadWifi = settingAvoidBadWifi || !configRestrictsAvoidBadWifi();
return mAvoidBadWifi != prev;
@@ -148,7 +150,7 @@ public class AvoidBadWifiTracker {
@Override
public void onChange(boolean selfChange, Uri uri) {
if (!mUri.equals(uri)) return;
if (!mAvoidBadWifiUri.equals(uri)) return;
reevaluate();
}
}

View File

@@ -53,7 +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.net.util.MultinetworkPolicyTracker;
import android.os.ConditionVariable;
import android.os.Handler;
import android.os.HandlerThread;
@@ -155,25 +155,13 @@ public class ConnectivityServiceTest extends AndroidTestCase {
/**
* Block until the given handler becomes idle, or until timeoutMs has passed.
*/
private static void waitForIdleHandler(HandlerThread handler, int timeoutMs) {
private static void waitForIdleHandler(HandlerThread handlerThread, int timeoutMs) {
final ConditionVariable cv = new ConditionVariable();
final MessageQueue queue = handler.getLooper().getQueue();
final IdleHandler idleHandler = () -> {
synchronized (queue) {
cv.open();
return false; // Remove the idleHandler.
}
};
synchronized (queue) {
if (queue.isIdle()) {
return;
}
queue.addIdleHandler(idleHandler);
}
final Handler handler = new Handler(handlerThread.getLooper());
handler.post(() -> cv.open());
if (!cv.block(timeoutMs)) {
fail("HandlerThread " + handler.getName() +
fail("HandlerThread " + handlerThread.getName() +
" did not become idle after " + timeoutMs + " ms");
queue.removeIdleHandler(idleHandler);
}
}
@@ -605,10 +593,10 @@ public class ConnectivityServiceTest extends AndroidTestCase {
}
}
private class WrappedAvoidBadWifiTracker extends AvoidBadWifiTracker {
private class WrappedMultinetworkPolicyTracker extends MultinetworkPolicyTracker {
public volatile boolean configRestrictsAvoidBadWifi;
public WrappedAvoidBadWifiTracker(Context c, Handler h, Runnable r) {
public WrappedMultinetworkPolicyTracker(Context c, Handler h, Runnable r) {
super(c, h, r);
}
@@ -619,7 +607,7 @@ public class ConnectivityServiceTest extends AndroidTestCase {
}
private class WrappedConnectivityService extends ConnectivityService {
public WrappedAvoidBadWifiTracker wrappedAvoidBadWifiTracker;
public WrappedMultinetworkPolicyTracker wrappedMultinetworkPolicyTracker;
private WrappedNetworkMonitor mLastCreatedNetworkMonitor;
public WrappedConnectivityService(Context context, INetworkManagementService netManager,
@@ -666,14 +654,14 @@ public class ConnectivityServiceTest extends AndroidTestCase {
}
@Override
public AvoidBadWifiTracker createAvoidBadWifiTracker(
public MultinetworkPolicyTracker createMultinetworkPolicyTracker(
Context c, Handler h, Runnable r) {
final WrappedAvoidBadWifiTracker tracker = new WrappedAvoidBadWifiTracker(c, h, r);
final WrappedMultinetworkPolicyTracker tracker = new WrappedMultinetworkPolicyTracker(c, h, r);
return tracker;
}
public WrappedAvoidBadWifiTracker getAvoidBadWifiTracker() {
return (WrappedAvoidBadWifiTracker) mAvoidBadWifiTracker;
public WrappedMultinetworkPolicyTracker getMultinetworkPolicyTracker() {
return (WrappedMultinetworkPolicyTracker) mMultinetworkPolicyTracker;
}
@Override
@@ -695,22 +683,6 @@ public class ConnectivityServiceTest extends AndroidTestCase {
}
}
private interface Criteria {
public boolean get();
}
/**
* Wait up to 500ms for {@code criteria.get()} to become true, polling.
* Fails if 500ms goes by before {@code criteria.get()} to become true.
*/
static private void waitFor(Criteria criteria) {
int delays = 0;
while (!criteria.get()) {
sleepFor(50);
if (++delays == 10) fail();
}
}
/**
* Wait up to TIMEOUT_MS for {@code conditionVariable} to open.
* Fails if TIMEOUT_MS goes by before {@code conditionVariable} opens.
@@ -846,8 +818,9 @@ public class ConnectivityServiceTest extends AndroidTestCase {
assertTrue(mCm.getAllNetworks()[0].equals(mCellNetworkAgent.getNetwork()) ||
mCm.getAllNetworks()[1].equals(mCellNetworkAgent.getNetwork()));
// Test cellular linger timeout.
waitFor(new Criteria() {
public boolean get() { return mCm.getAllNetworks().length == 1; } });
waitFor(mCellNetworkAgent.getDisconnectedCV());
mService.waitForIdle();
assertEquals(1, mCm.getAllNetworks().length);
verifyActiveNetwork(TRANSPORT_WIFI);
assertEquals(1, mCm.getAllNetworks().length);
assertEquals(mCm.getAllNetworks()[0], mCm.getActiveNetwork());
@@ -1622,8 +1595,8 @@ public class ConnectivityServiceTest extends AndroidTestCase {
ConditionVariable cv = mCellNetworkAgent.getDisconnectedCV();
mCellNetworkAgent.connectWithoutInternet();
waitFor(cv);
waitFor(new Criteria() {
public boolean get() { return mCm.getAllNetworks().length == 0; } });
mService.waitForIdle();
assertEquals(0, mCm.getAllNetworks().length);
verifyNoNetwork();
// Test bringing up validated WiFi.
mWiFiNetworkAgent = new MockNetworkAgent(TRANSPORT_WIFI);
@@ -1982,7 +1955,6 @@ public class ConnectivityServiceTest extends AndroidTestCase {
// Disconnect wifi and check that cell is foreground again.
mWiFiNetworkAgent.disconnect();
mService.waitForIdle();
callback.expectCallback(CallbackState.LOST, mWiFiNetworkAgent);
fgCallback.expectCallback(CallbackState.LOST, mWiFiNetworkAgent);
fgCallback.expectCallback(CallbackState.AVAILABLE, mCellNetworkAgent);
@@ -2152,7 +2124,7 @@ public class ConnectivityServiceTest extends AndroidTestCase {
@SmallTest
public void testAvoidBadWifiSetting() throws Exception {
final ContentResolver cr = mServiceContext.getContentResolver();
final WrappedAvoidBadWifiTracker tracker = mService.getAvoidBadWifiTracker();
final WrappedMultinetworkPolicyTracker tracker = mService.getMultinetworkPolicyTracker();
final String settingName = Settings.Global.NETWORK_AVOID_BAD_WIFI;
tracker.configRestrictsAvoidBadWifi = false;
@@ -2162,7 +2134,7 @@ public class ConnectivityServiceTest extends AndroidTestCase {
tracker.reevaluate();
mService.waitForIdle();
String msg = String.format("config=false, setting=%s", values[i]);
assertEventuallyTrue(() -> mService.avoidBadWifi(), 50);
assertTrue(mService.avoidBadWifi());
assertFalse(msg, tracker.shouldNotifyWifiUnvalidated());
}
@@ -2171,26 +2143,26 @@ public class ConnectivityServiceTest extends AndroidTestCase {
Settings.Global.putInt(cr, settingName, 0);
tracker.reevaluate();
mService.waitForIdle();
assertEventuallyTrue(() -> !mService.avoidBadWifi(), 50);
assertFalse(mService.avoidBadWifi());
assertFalse(tracker.shouldNotifyWifiUnvalidated());
Settings.Global.putInt(cr, settingName, 1);
tracker.reevaluate();
mService.waitForIdle();
assertEventuallyTrue(() -> mService.avoidBadWifi(), 50);
assertTrue(mService.avoidBadWifi());
assertFalse(tracker.shouldNotifyWifiUnvalidated());
Settings.Global.putString(cr, settingName, null);
tracker.reevaluate();
mService.waitForIdle();
assertEventuallyTrue(() -> !mService.avoidBadWifi(), 50);
assertFalse(mService.avoidBadWifi());
assertTrue(tracker.shouldNotifyWifiUnvalidated());
}
@SmallTest
public void testAvoidBadWifi() throws Exception {
final ContentResolver cr = mServiceContext.getContentResolver();
final WrappedAvoidBadWifiTracker tracker = mService.getAvoidBadWifiTracker();
final WrappedMultinetworkPolicyTracker tracker = mService.getMultinetworkPolicyTracker();
// Pretend we're on a carrier that restricts switching away from bad wifi.
tracker.configRestrictsAvoidBadWifi = true;
@@ -2404,17 +2376,6 @@ public class ConnectivityServiceTest extends AndroidTestCase {
networkCallback.assertNoCallback();
}
public void assertEventuallyTrue(BooleanSupplier fn, long maxWaitingTimeMs) {
long start = SystemClock.elapsedRealtime();
while (SystemClock.elapsedRealtime() <= start + maxWaitingTimeMs) {
if (fn.getAsBoolean()) {
return;
}
sleepFor(15);
}
assertTrue(fn.getAsBoolean());
}
private static class TestKeepaliveCallback extends PacketKeepaliveCallback {
public static enum CallbackType { ON_STARTED, ON_STOPPED, ON_ERROR };
@@ -2575,12 +2536,13 @@ public class ConnectivityServiceTest extends AndroidTestCase {
ka = mCm.startNattKeepalive(myNet, 25, callback, myIPv4, 12345, dstIPv4);
callback.expectStarted();
mWiFiNetworkAgent.disconnect();
waitFor(mWiFiNetworkAgent.getDisconnectedCV());
callback.expectError(PacketKeepalive.ERROR_INVALID_NETWORK);
// ... and that stopping it after that has no adverse effects.
// TODO: investigate assertEventuallyTrue is needed and waitForIdle() is not enough
mService.waitForIdle();
final Network myNetAlias = myNet;
assertEventuallyTrue(() -> mCm.getNetworkCapabilities(myNetAlias) == null, 100);
assertNull(mCm.getNetworkCapabilities(myNetAlias));
ka.stop();
// Reconnect.
@@ -2592,6 +2554,7 @@ public class ConnectivityServiceTest extends AndroidTestCase {
callback.expectStarted();
ka.stop();
mWiFiNetworkAgent.disconnect();
waitFor(mWiFiNetworkAgent.getDisconnectedCV());
mService.waitForIdle();
callback.expectStopped();