Allow any unvalidated Network to satisfy NetworkRequests.
Explicitly selected Networks may never be validated (e.g. Chromecast)
but are still given a high score so they can explicitly become the
default Network. Without this fix they do not become the default
Network if another Network is present. This was an artifact of how
unvalidated Networks were handled, but now that unvalidated Networks
are properly handled, ala 50807d, we can freely rematch even
unvalidated Networks and NetworkRequests.
Also, never linger and teardown unvalidated Networks as the user
might be in the process of signing in. This better matches prior
behavior when unvalidated networks didn't match NetworkRequests,
and thus were never lingered.
Also, don't disconnect networks that may be lingering. The
disconnect logic in rematchNetworkAndReqeuests() is adjusted to only
fire when a network is newly validated.
It is incorrect to consider rematching uncreated Networks and
explicitly selecting created Networks, so this change logs error
messages in those cases.
bug:17647968
bug:17396616
Change-Id: Id6b8a350b8200f484d5bfd14ca0a8f64f08846a0
diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index 85ab249..3e9bb2e 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -1888,6 +1888,9 @@
loge("EVENT_SET_EXPLICITLY_SELECTED from unknown NetworkAgent");
break;
}
+ if (nai.created && !nai.networkMisc.explicitlySelected) {
+ loge("ERROR: created network explicitly selected.");
+ }
nai.networkMisc.explicitlySelected = true;
break;
}
@@ -1897,8 +1900,9 @@
boolean valid = (msg.arg1 == NetworkMonitor.NETWORK_TEST_RESULT_VALID);
if (valid) {
if (DBG) log("Validated " + nai.name());
+ final boolean previouslyValidated = nai.validated;
nai.validated = true;
- rematchNetworkAndRequests(nai);
+ rematchNetworkAndRequests(nai, !previouslyValidated);
}
updateInetCondition(nai, valid);
// Let the NetworkAgent know the state of its network
@@ -2071,6 +2075,7 @@
}
// Since we've lost the network, go through all the requests that
// it was satisfying and see if any other factory can satisfy them.
+ // TODO: This logic may be better replaced with a call to rematchAllNetworksAndRequests
final ArrayList<NetworkAgentInfo> toActivate = new ArrayList<NetworkAgentInfo>();
for (int i = 0; i < nai.networkRequests.size(); i++) {
NetworkRequest request = nai.networkRequests.valueAt(i);
@@ -2106,7 +2111,9 @@
requestNetworkTransitionWakelock(nai.name());
}
for (NetworkAgentInfo networkToActivate : toActivate) {
+ networkToActivate.networkLingered.clear();
networkToActivate.networkMonitor.sendMessage(NetworkMonitor.CMD_NETWORK_CONNECTED);
+ rematchNetworkAndRequests(networkToActivate, false);
}
}
}
@@ -2144,6 +2151,7 @@
bestNetwork.networkLingered.clear();
bestNetwork.networkMonitor.sendMessage(NetworkMonitor.CMD_NETWORK_CONNECTED);
}
+ // TODO: This logic may be better replaced with a call to rematchNetworkAndRequests
bestNetwork.addRequest(nri.request);
mNetworkForRequestId.put(nri.request.requestId, bestNetwork);
notifyNetworkCallback(bestNetwork, nri);
@@ -3768,21 +3776,32 @@
// satisfied by newNetwork, and reassigns to newNetwork
// any such requests for which newNetwork is the best.
//
- // - Tears down any Networks that as a result are no longer
+ // - Lingers any Networks that as a result are no longer
// needed. A network is needed if it is the best network for
// one or more NetworkRequests, or if it is a VPN.
//
- // - Tears down newNetwork if it is validated but turns out to be
- // unneeded. Does not tear down newNetwork if it is
- // unvalidated, because future validation may improve
- // newNetwork's score enough that it is needed.
+ // - Tears down newNetwork if it just became validated
+ // (i.e. nascent==true) but turns out to be unneeded.
+ // Does not tear down newNetwork if it is unvalidated,
+ // because future validation may improve newNetwork's
+ // score enough that it is needed.
//
// NOTE: This function only adds NetworkRequests that "newNetwork" could satisfy,
// it does not remove NetworkRequests that other Networks could better satisfy.
// If you need to handle decreases in score, use {@link rematchAllNetworksAndRequests}.
// This function should be used when possible instead of {@code rematchAllNetworksAndRequests}
// as it performs better by a factor of the number of Networks.
- private void rematchNetworkAndRequests(NetworkAgentInfo newNetwork) {
+ //
+ // @param nascent indicates if newNetwork just became validated, in which case it should be
+ // torn down if unneeded. If nascent is false, no action is taken if newNetwork
+ // is found to be unneeded by this call. Presumably, in this case, either:
+ // - newNetwork is unvalidated (and left alive), or
+ // - the NetworkRequests keeping newNetwork alive have been transitioned to
+ // another higher scoring network by another call to rematchNetworkAndRequests()
+ // and this other call also lingered newNetwork.
+ private void rematchNetworkAndRequests(NetworkAgentInfo newNetwork, boolean nascent) {
+ if (!newNetwork.created) loge("ERROR: uncreated network being rematched.");
+ if (nascent && !newNetwork.validated) loge("ERROR: nascent network not validated.");
boolean keep = newNetwork.isVPN();
boolean isNewDefault = false;
if (DBG) log("rematching " + newNetwork.name());
@@ -3868,7 +3887,7 @@
}
// Linger any networks that are no longer needed.
for (NetworkAgentInfo nai : affectedNetworks) {
- boolean teardown = !nai.isVPN();
+ boolean teardown = !nai.isVPN() && nai.validated;
for (int i = 0; i < nai.networkRequests.size() && teardown; i++) {
NetworkRequest nr = nai.networkRequests.valueAt(i);
try {
@@ -3927,10 +3946,12 @@
}
notifyNetworkCallbacks(newNetwork, ConnectivityManager.CALLBACK_AVAILABLE);
- } else if (newNetwork.validated) {
- // Only tear down validated networks here. Leave unvalidated to either become
+ } else if (nascent) {
+ // Only tear down newly validated networks here. Leave unvalidated to either become
// validated (and get evaluated against peers, one losing here) or
// NetworkMonitor reports a bad network and we tear it down then.
+ // Networks that have been up for a while and are validated should be torn down via
+ // the lingering process so communication on that network is given time to wrap up.
// TODO: Could teardown unvalidated networks when their NetworkCapabilities
// satisfy no NetworkRequests.
if (DBG && newNetwork.networkRequests.size() != 0) {
@@ -3962,10 +3983,10 @@
// can only add more NetworkRequests satisfied by "changed", and this is exactly what
// rematchNetworkAndRequests() handles.
if (changed != null && oldScore < changed.getCurrentScore()) {
- rematchNetworkAndRequests(changed);
+ rematchNetworkAndRequests(changed, false);
} else {
for (NetworkAgentInfo nai : mNetworkAgentInfos.values()) {
- rematchNetworkAndRequests(nai);
+ rematchNetworkAndRequests(nai, false);
}
}
}
@@ -4039,14 +4060,7 @@
// TODO: support proxy per network.
}
// Consider network even though it is not yet validated.
- // TODO: All the if-statement conditions can be removed now that validation only confers
- // a score increase.
- if (mNetworkForRequestId.get(mDefaultRequest.requestId) == null &&
- networkAgent.isVPN() == false &&
- mDefaultRequest.networkCapabilities.satisfiedByNetworkCapabilities(
- networkAgent.networkCapabilities)) {
- rematchNetworkAndRequests(networkAgent);
- }
+ rematchNetworkAndRequests(networkAgent, false);
} else if (state == NetworkInfo.State.DISCONNECTED ||
state == NetworkInfo.State.SUSPENDED) {
networkAgent.asyncChannel.disconnect();