diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index c0071385ff..e39c36aec0 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -4630,9 +4630,16 @@ public class ConnectivityService extends IConnectivityManager.Stub } 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 offersToUpdate = new ArrayList<>(mNetworkOffers); + for (final NetworkOfferInfo noi : offersToUpdate) { + updateOfferScore(noi.offer); + } rematchAllNetworksAndRequests(); } @@ -6421,11 +6428,23 @@ public class ConnectivityService extends IConnectivityManager.Stub 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)); @@ -6846,6 +6865,7 @@ public class ConnectivityService extends IConnectivityManager.Stub * @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)) { @@ -6859,6 +6879,14 @@ public class ConnectivityService extends IConnectivityManager.Stub 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 { @@ -6873,6 +6901,9 @@ public class ConnectivityService extends IConnectivityManager.Stub 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 14cec09560..aebb80df38 100644 --- a/service/src/com/android/server/connectivity/FullScore.java +++ b/service/src/com/android/server/connectivity/FullScore.java @@ -183,7 +183,7 @@ public class FullScore { * @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 @@ public class FullScore { 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; @@ -258,6 +256,16 @@ public class FullScore { keepConnectedReason); } + /** + * 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. */ diff --git a/service/src/com/android/server/connectivity/NetworkAgentInfo.java b/service/src/com/android/server/connectivity/NetworkAgentInfo.java index bbf523a29d..6426f86070 100644 --- a/service/src/com/android/server/connectivity/NetworkAgentInfo.java +++ b/service/src/com/android/server/connectivity/NetworkAgentInfo.java @@ -1187,6 +1187,7 @@ public class NetworkAgentInfo implements Comparable, NetworkRa ? " 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 95ea401acd..970b7d2e58 100644 --- a/tests/integration/util/com/android/server/NetworkAgentWrapper.java +++ b/tests/integration/util/com/android/server/NetworkAgentWrapper.java @@ -83,6 +83,12 @@ public class NetworkAgentWrapper implements TestableNetworkCallback.HasNetwork { 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 @@ public class NetworkAgentWrapper implements TestableNetworkCallback.HasNetwork { .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 class NetworkAgentWrapper implements TestableNetworkCallback.HasNetwork { 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 d0cdf20386..3d45c896c1 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; 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.ExceptionUtils; 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 @@ public class ConnectivityServiceTest { 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 @@ public class ConnectivityServiceTest { 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 @@ public class ConnectivityServiceTest { 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(); @@ -4915,6 +4929,124 @@ public class ConnectivityServiceTest { testAvoidBadWifiConfig_controlledBySettings(); } + @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();