Merge "Fix an infinite loop with network offers"
This commit is contained in:
@@ -4622,9 +4622,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<NetworkOfferInfo> offersToUpdate = new ArrayList<>(mNetworkOffers);
|
||||
for (final NetworkOfferInfo noi : offersToUpdate) {
|
||||
updateOfferScore(noi.offer);
|
||||
}
|
||||
rematchAllNetworksAndRequests();
|
||||
}
|
||||
|
||||
@@ -6413,11 +6420,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));
|
||||
@@ -6838,6 +6857,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)) {
|
||||
@@ -6852,6 +6872,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 {
|
||||
@@ -6866,6 +6894,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 */);
|
||||
}
|
||||
|
||||
@@ -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.
|
||||
*/
|
||||
|
||||
@@ -1187,6 +1187,7 @@ public class NetworkAgentInfo implements Comparable<NetworkAgentInfo>, NetworkRa
|
||||
? " underlying{" + Arrays.toString(declaredUnderlyingNetworks) + "}" : "")
|
||||
+ " lp{" + linkProperties + "}"
|
||||
+ " nc{" + networkCapabilities + "}"
|
||||
+ " factorySerialNumber=" + factorySerialNumber
|
||||
+ "}";
|
||||
}
|
||||
|
||||
|
||||
@@ -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();
|
||||
}
|
||||
|
||||
@@ -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();
|
||||
@@ -4797,6 +4811,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();
|
||||
|
||||
Reference in New Issue
Block a user