Merge "Fix an infinite loop with network offers"
diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java
index 47bde72..1d5d8af 100644
--- a/service/src/com/android/server/ConnectivityService.java
+++ b/service/src/com/android/server/ConnectivityService.java
@@ -4622,9 +4622,16 @@
}
private void updateAvoidBadWifi() {
+ ensureRunningOnConnectivityServiceThread();
+ // Agent info scores and offer scores depend on whether cells yields to bad wifi.
for (final NetworkAgentInfo nai : mNetworkAgentInfos) {
nai.updateScoreForNetworkAgentUpdate();
}
+ // UpdateOfferScore will update mNetworkOffers inline, so make a copy first.
+ final ArrayList<NetworkOfferInfo> offersToUpdate = new ArrayList<>(mNetworkOffers);
+ for (final NetworkOfferInfo noi : offersToUpdate) {
+ updateOfferScore(noi.offer);
+ }
rematchAllNetworksAndRequests();
}
@@ -6413,11 +6420,23 @@
Objects.requireNonNull(score);
Objects.requireNonNull(caps);
Objects.requireNonNull(callback);
+ final boolean yieldToBadWiFi = caps.hasTransport(TRANSPORT_CELLULAR) && !avoidBadWifi();
final NetworkOffer offer = new NetworkOffer(
- FullScore.makeProspectiveScore(score, caps), caps, callback, providerId);
+ FullScore.makeProspectiveScore(score, caps, yieldToBadWiFi),
+ caps, callback, providerId);
mHandler.sendMessage(mHandler.obtainMessage(EVENT_REGISTER_NETWORK_OFFER, offer));
}
+ private void updateOfferScore(final NetworkOffer offer) {
+ final boolean yieldToBadWiFi =
+ offer.caps.hasTransport(TRANSPORT_CELLULAR) && !avoidBadWifi();
+ final NetworkOffer newOffer = new NetworkOffer(
+ offer.score.withYieldToBadWiFi(yieldToBadWiFi),
+ offer.caps, offer.callback, offer.providerId);
+ if (offer.equals(newOffer)) return;
+ handleRegisterNetworkOffer(newOffer);
+ }
+
@Override
public void unofferNetwork(@NonNull final INetworkOfferCallback callback) {
mHandler.sendMessage(mHandler.obtainMessage(EVENT_UNREGISTER_NETWORK_OFFER, callback));
@@ -6838,6 +6857,7 @@
* @param newOffer The new offer. If the callback member is the same as an existing
* offer, it is an update of that offer.
*/
+ // TODO : rename this to handleRegisterOrUpdateNetworkOffer
private void handleRegisterNetworkOffer(@NonNull final NetworkOffer newOffer) {
ensureRunningOnConnectivityServiceThread();
if (!isNetworkProviderWithIdRegistered(newOffer.providerId)) {
@@ -6852,6 +6872,14 @@
if (null != existingOffer) {
handleUnregisterNetworkOffer(existingOffer);
newOffer.migrateFrom(existingOffer.offer);
+ if (DBG) {
+ // handleUnregisterNetworkOffer has already logged the old offer
+ log("update offer from providerId " + newOffer.providerId + " new : " + newOffer);
+ }
+ } else {
+ if (DBG) {
+ log("register offer from providerId " + newOffer.providerId + " : " + newOffer);
+ }
}
final NetworkOfferInfo noi = new NetworkOfferInfo(newOffer);
try {
@@ -6866,6 +6894,9 @@
private void handleUnregisterNetworkOffer(@NonNull final NetworkOfferInfo noi) {
ensureRunningOnConnectivityServiceThread();
+ if (DBG) {
+ log("unregister offer from providerId " + noi.offer.providerId + " : " + noi.offer);
+ }
mNetworkOffers.remove(noi);
noi.offer.callback.asBinder().unlinkToDeath(noi, 0 /* flags */);
}
diff --git a/service/src/com/android/server/connectivity/FullScore.java b/service/src/com/android/server/connectivity/FullScore.java
index 14cec09..aebb80d 100644
--- a/service/src/com/android/server/connectivity/FullScore.java
+++ b/service/src/com/android/server/connectivity/FullScore.java
@@ -183,7 +183,7 @@
* @return a FullScore appropriate for comparing to actual network's scores.
*/
public static FullScore makeProspectiveScore(@NonNull final NetworkScore score,
- @NonNull final NetworkCapabilities caps) {
+ @NonNull final NetworkCapabilities caps, final boolean yieldToBadWiFi) {
// If the network offers Internet access, it may validate.
final boolean mayValidate = caps.hasCapability(NET_CAPABILITY_INTERNET);
// VPN transports are known in advance.
@@ -197,8 +197,6 @@
final boolean everUserSelected = false;
// Don't assume the user will accept unvalidated connectivity.
final boolean acceptUnvalidated = false;
- // Don't assume clinging to bad wifi
- final boolean yieldToBadWiFi = false;
// A prospective score is invincible if the legacy int in the filter is over the maximum
// score.
final boolean invincible = score.getLegacyInt() > NetworkRanker.LEGACY_INT_MAX;
@@ -259,6 +257,16 @@
}
/**
+ * Returns this score but with the specified yield to bad wifi policy.
+ */
+ public FullScore withYieldToBadWiFi(final boolean newYield) {
+ return new FullScore(mLegacyInt,
+ newYield ? mPolicies | (1L << POLICY_YIELD_TO_BAD_WIFI)
+ : mPolicies & ~(1L << POLICY_YIELD_TO_BAD_WIFI),
+ mKeepConnectedReason);
+ }
+
+ /**
* Returns this score but validated.
*/
public FullScore asValidated() {
diff --git a/service/src/com/android/server/connectivity/NetworkAgentInfo.java b/service/src/com/android/server/connectivity/NetworkAgentInfo.java
index bbf523a..6426f86 100644
--- a/service/src/com/android/server/connectivity/NetworkAgentInfo.java
+++ b/service/src/com/android/server/connectivity/NetworkAgentInfo.java
@@ -1187,6 +1187,7 @@
? " underlying{" + Arrays.toString(declaredUnderlyingNetworks) + "}" : "")
+ " lp{" + linkProperties + "}"
+ " nc{" + networkCapabilities + "}"
+ + " factorySerialNumber=" + factorySerialNumber
+ "}";
}
diff --git a/tests/integration/util/com/android/server/NetworkAgentWrapper.java b/tests/integration/util/com/android/server/NetworkAgentWrapper.java
index 95ea401..970b7d2 100644
--- a/tests/integration/util/com/android/server/NetworkAgentWrapper.java
+++ b/tests/integration/util/com/android/server/NetworkAgentWrapper.java
@@ -83,6 +83,12 @@
public NetworkAgentWrapper(int transport, LinkProperties linkProperties,
NetworkCapabilities ncTemplate, Context context) throws Exception {
+ this(transport, linkProperties, ncTemplate, null /* provider */, context);
+ }
+
+ public NetworkAgentWrapper(int transport, LinkProperties linkProperties,
+ NetworkCapabilities ncTemplate, NetworkProvider provider,
+ Context context) throws Exception {
final int type = transportToLegacyType(transport);
final String typeName = ConnectivityManager.getNetworkTypeName(type);
mNetworkCapabilities = (ncTemplate != null) ? ncTemplate : new NetworkCapabilities();
@@ -124,12 +130,12 @@
.setLegacyTypeName(typeName)
.setLegacyExtraInfo(extraInfo)
.build();
- mNetworkAgent = makeNetworkAgent(linkProperties, mNetworkAgentConfig);
+ mNetworkAgent = makeNetworkAgent(linkProperties, mNetworkAgentConfig, provider);
}
protected InstrumentedNetworkAgent makeNetworkAgent(LinkProperties linkProperties,
- final NetworkAgentConfig nac) throws Exception {
- return new InstrumentedNetworkAgent(this, linkProperties, nac);
+ final NetworkAgentConfig nac, NetworkProvider provider) throws Exception {
+ return new InstrumentedNetworkAgent(this, linkProperties, nac, provider);
}
public static class InstrumentedNetworkAgent extends NetworkAgent {
@@ -138,10 +144,15 @@
public InstrumentedNetworkAgent(NetworkAgentWrapper wrapper, LinkProperties lp,
NetworkAgentConfig nac) {
+ this(wrapper, lp, nac, null /* provider */);
+ }
+
+ public InstrumentedNetworkAgent(NetworkAgentWrapper wrapper, LinkProperties lp,
+ NetworkAgentConfig nac, NetworkProvider provider) {
super(wrapper.mContext, wrapper.mHandlerThread.getLooper(), wrapper.mLogTag,
wrapper.mNetworkCapabilities, lp, wrapper.mScore, nac,
- new NetworkProvider(wrapper.mContext, wrapper.mHandlerThread.getLooper(),
- PROVIDER_NAME));
+ null != provider ? provider : new NetworkProvider(wrapper.mContext,
+ wrapper.mHandlerThread.getLooper(), PROVIDER_NAME));
mWrapper = wrapper;
register();
}
diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java
index 2370b8d..814d799 100644
--- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java
@@ -241,6 +241,7 @@
import android.net.NetworkInfo.DetailedState;
import android.net.NetworkPolicyManager;
import android.net.NetworkPolicyManager.NetworkPolicyCallback;
+import android.net.NetworkProvider;
import android.net.NetworkRequest;
import android.net.NetworkScore;
import android.net.NetworkSpecifier;
@@ -338,6 +339,7 @@
import com.android.testutils.HandlerUtils;
import com.android.testutils.RecorderCallback.CallbackEntry;
import com.android.testutils.TestableNetworkCallback;
+import com.android.testutils.TestableNetworkOfferCallback;
import org.junit.After;
import org.junit.Before;
@@ -816,17 +818,22 @@
private String mRedirectUrl;
TestNetworkAgentWrapper(int transport) throws Exception {
- this(transport, new LinkProperties(), null);
+ this(transport, new LinkProperties(), null /* ncTemplate */, null /* provider */);
}
TestNetworkAgentWrapper(int transport, LinkProperties linkProperties)
throws Exception {
- this(transport, linkProperties, null);
+ this(transport, linkProperties, null /* ncTemplate */, null /* provider */);
}
private TestNetworkAgentWrapper(int transport, LinkProperties linkProperties,
NetworkCapabilities ncTemplate) throws Exception {
- super(transport, linkProperties, ncTemplate, mServiceContext);
+ this(transport, linkProperties, ncTemplate, null /* provider */);
+ }
+
+ private TestNetworkAgentWrapper(int transport, LinkProperties linkProperties,
+ NetworkCapabilities ncTemplate, NetworkProvider provider) throws Exception {
+ super(transport, linkProperties, ncTemplate, provider, mServiceContext);
// Waits for the NetworkAgent to be registered, which includes the creation of the
// NetworkMonitor.
@@ -835,9 +842,40 @@
HandlerUtils.waitForIdle(ConnectivityThread.get(), TIMEOUT_MS);
}
+ class TestInstrumentedNetworkAgent extends InstrumentedNetworkAgent {
+ TestInstrumentedNetworkAgent(NetworkAgentWrapper wrapper, LinkProperties lp,
+ NetworkAgentConfig nac, NetworkProvider provider) {
+ super(wrapper, lp, nac, provider);
+ }
+
+ @Override
+ public void networkStatus(int status, String redirectUrl) {
+ mRedirectUrl = redirectUrl;
+ mNetworkStatusReceived.open();
+ }
+
+ @Override
+ public void onNetworkCreated() {
+ super.onNetworkCreated();
+ if (mCreatedCallback != null) mCreatedCallback.run();
+ }
+
+ @Override
+ public void onNetworkUnwanted() {
+ super.onNetworkUnwanted();
+ if (mUnwantedCallback != null) mUnwantedCallback.run();
+ }
+
+ @Override
+ public void onNetworkDestroyed() {
+ super.onNetworkDestroyed();
+ if (mDisconnectedCallback != null) mDisconnectedCallback.run();
+ }
+ }
+
@Override
protected InstrumentedNetworkAgent makeNetworkAgent(LinkProperties linkProperties,
- NetworkAgentConfig nac) throws Exception {
+ NetworkAgentConfig nac, NetworkProvider provider) throws Exception {
mNetworkMonitor = mock(INetworkMonitor.class);
final Answer validateAnswer = inv -> {
@@ -857,31 +895,7 @@
nmCbCaptor.capture());
final InstrumentedNetworkAgent na =
- new InstrumentedNetworkAgent(this, linkProperties, nac) {
- @Override
- public void networkStatus(int status, String redirectUrl) {
- mRedirectUrl = redirectUrl;
- mNetworkStatusReceived.open();
- }
-
- @Override
- public void onNetworkCreated() {
- super.onNetworkCreated();
- if (mCreatedCallback != null) mCreatedCallback.run();
- }
-
- @Override
- public void onNetworkUnwanted() {
- super.onNetworkUnwanted();
- if (mUnwantedCallback != null) mUnwantedCallback.run();
- }
-
- @Override
- public void onNetworkDestroyed() {
- super.onNetworkDestroyed();
- if (mDisconnectedCallback != null) mDisconnectedCallback.run();
- }
- };
+ new TestInstrumentedNetworkAgent(this, linkProperties, nac, provider);
assertEquals(na.getNetwork().netId, nmNetworkCaptor.getValue().netId);
mNmCallbacks = nmCbCaptor.getValue();
@@ -4798,6 +4812,124 @@
}
@Test
+ public void testOffersAvoidsBadWifi() throws Exception {
+ // Normal mode : the carrier doesn't restrict moving away from bad wifi.
+ // This has getAvoidBadWifi return true.
+ doReturn(1).when(mResources).getInteger(R.integer.config_networkAvoidBadWifi);
+ // Don't request cell separately for the purposes of this test.
+ setAlwaysOnNetworks(false);
+
+ final NetworkProvider cellProvider = new NetworkProvider(mServiceContext,
+ mCsHandlerThread.getLooper(), "Cell provider");
+ final NetworkProvider wifiProvider = new NetworkProvider(mServiceContext,
+ mCsHandlerThread.getLooper(), "Wifi provider");
+
+ mCm.registerNetworkProvider(cellProvider);
+ mCm.registerNetworkProvider(wifiProvider);
+
+ final NetworkScore cellScore = new NetworkScore.Builder().build();
+ final NetworkScore wifiScore = new NetworkScore.Builder().build();
+ final NetworkCapabilities defaultCaps = new NetworkCapabilities.Builder()
+ .addCapability(NET_CAPABILITY_INTERNET)
+ .addCapability(NET_CAPABILITY_NOT_VCN_MANAGED)
+ .build();
+ final NetworkCapabilities cellCaps = new NetworkCapabilities.Builder()
+ .addTransportType(TRANSPORT_CELLULAR)
+ .addCapability(NET_CAPABILITY_INTERNET)
+ .addCapability(NET_CAPABILITY_NOT_VCN_MANAGED)
+ .build();
+ final NetworkCapabilities wifiCaps = new NetworkCapabilities.Builder()
+ .addTransportType(TRANSPORT_WIFI)
+ .addCapability(NET_CAPABILITY_INTERNET)
+ .addCapability(NET_CAPABILITY_NOT_VCN_MANAGED)
+ .build();
+ final TestableNetworkOfferCallback cellCallback = new TestableNetworkOfferCallback(
+ TIMEOUT_MS /* timeout */, TEST_CALLBACK_TIMEOUT_MS /* noCallbackTimeout */);
+ final TestableNetworkOfferCallback wifiCallback = new TestableNetworkOfferCallback(
+ TIMEOUT_MS /* timeout */, TEST_CALLBACK_TIMEOUT_MS /* noCallbackTimeout */);
+
+ Log.e("ConnectivityService", "test registering " + cellProvider);
+ // Offer callbacks will run on the CS handler thread in this test.
+ cellProvider.registerNetworkOffer(cellScore, cellCaps, r -> r.run(), cellCallback);
+ wifiProvider.registerNetworkOffer(wifiScore, wifiCaps, r -> r.run(), wifiCallback);
+
+ // Both providers see the default request.
+ cellCallback.expectOnNetworkNeeded(defaultCaps);
+ wifiCallback.expectOnNetworkNeeded(defaultCaps);
+
+ // Listen to cell and wifi to know when agents are finished processing
+ final TestNetworkCallback cellNetworkCallback = new TestNetworkCallback();
+ final NetworkRequest cellRequest = new NetworkRequest.Builder()
+ .addTransportType(TRANSPORT_CELLULAR).build();
+ mCm.registerNetworkCallback(cellRequest, cellNetworkCallback);
+ final TestNetworkCallback wifiNetworkCallback = new TestNetworkCallback();
+ final NetworkRequest wifiRequest = new NetworkRequest.Builder()
+ .addTransportType(TRANSPORT_WIFI).build();
+ mCm.registerNetworkCallback(wifiRequest, wifiNetworkCallback);
+
+ // Cell connects and validates.
+ mCellNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR,
+ new LinkProperties(), null /* ncTemplate */, cellProvider);
+ mCellNetworkAgent.connect(true);
+ cellNetworkCallback.expectAvailableThenValidatedCallbacks(mCellNetworkAgent);
+ cellCallback.assertNoCallback();
+ wifiCallback.assertNoCallback();
+
+ // Bring up wifi. At first it's invalidated, so cell is still needed.
+ mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI,
+ new LinkProperties(), null /* ncTemplate */, wifiProvider);
+ mWiFiNetworkAgent.connect(false);
+ wifiNetworkCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent);
+ cellCallback.assertNoCallback();
+ wifiCallback.assertNoCallback();
+
+ // Wifi validates. Cell is no longer needed, because it's outscored.
+ mWiFiNetworkAgent.setNetworkValid(true /* isStrictMode */);
+ // Have CS reconsider the network (see testPartialConnectivity)
+ mCm.reportNetworkConnectivity(mWiFiNetworkAgent.getNetwork(), true);
+ wifiNetworkCallback.expectCapabilitiesWith(NET_CAPABILITY_VALIDATED, mWiFiNetworkAgent);
+ cellCallback.expectOnNetworkUnneeded(defaultCaps);
+ wifiCallback.assertNoCallback();
+
+ // Wifi is no longer validated. Cell is needed again.
+ mWiFiNetworkAgent.setNetworkInvalid(true /* isStrictMode */);
+ mCm.reportNetworkConnectivity(mWiFiNetworkAgent.getNetwork(), false);
+ wifiNetworkCallback.expectCapabilitiesWithout(NET_CAPABILITY_VALIDATED, mWiFiNetworkAgent);
+ cellCallback.expectOnNetworkNeeded(defaultCaps);
+ wifiCallback.assertNoCallback();
+
+ // Disconnect wifi and pretend the carrier restricts moving away from bad wifi.
+ mWiFiNetworkAgent.disconnect();
+ wifiNetworkCallback.expectCallback(CallbackEntry.LOST, mWiFiNetworkAgent);
+ // This has getAvoidBadWifi return false. This test doesn't change the value of the
+ // associated setting.
+ doReturn(0).when(mResources).getInteger(R.integer.config_networkAvoidBadWifi);
+ mPolicyTracker.reevaluate();
+ waitForIdle();
+
+ // Connect wifi again, cell is needed until wifi validates.
+ mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI,
+ new LinkProperties(), null /* ncTemplate */, wifiProvider);
+ mWiFiNetworkAgent.connect(false);
+ wifiNetworkCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent);
+ cellCallback.assertNoCallback();
+ wifiCallback.assertNoCallback();
+ mWiFiNetworkAgent.setNetworkValid(true /* isStrictMode */);
+ mCm.reportNetworkConnectivity(mWiFiNetworkAgent.getNetwork(), true);
+ wifiNetworkCallback.expectCapabilitiesWith(NET_CAPABILITY_VALIDATED, mWiFiNetworkAgent);
+ cellCallback.expectOnNetworkUnneeded(defaultCaps);
+ wifiCallback.assertNoCallback();
+
+ // Wifi loses validation. Because the device doesn't avoid bad wifis, cell is
+ // not needed.
+ mWiFiNetworkAgent.setNetworkInvalid(true /* isStrictMode */);
+ mCm.reportNetworkConnectivity(mWiFiNetworkAgent.getNetwork(), false);
+ wifiNetworkCallback.expectCapabilitiesWithout(NET_CAPABILITY_VALIDATED, mWiFiNetworkAgent);
+ cellCallback.assertNoCallback();
+ wifiCallback.assertNoCallback();
+ }
+
+ @Test
public void testAvoidBadWifi() throws Exception {
final ContentResolver cr = mServiceContext.getContentResolver();