Skip to content

Commit 4b9b2be

Browse files
committed
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
1 parent 8df099d commit 4b9b2be

1 file changed

Lines changed: 34 additions & 20 deletions

File tree

services/core/java/com/android/server/ConnectivityService.java

Lines changed: 34 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1888,6 +1888,9 @@ public void handleMessage(Message msg) {
18881888
loge("EVENT_SET_EXPLICITLY_SELECTED from unknown NetworkAgent");
18891889
break;
18901890
}
1891+
if (nai.created && !nai.networkMisc.explicitlySelected) {
1892+
loge("ERROR: created network explicitly selected.");
1893+
}
18911894
nai.networkMisc.explicitlySelected = true;
18921895
break;
18931896
}
@@ -1897,8 +1900,9 @@ public void handleMessage(Message msg) {
18971900
boolean valid = (msg.arg1 == NetworkMonitor.NETWORK_TEST_RESULT_VALID);
18981901
if (valid) {
18991902
if (DBG) log("Validated " + nai.name());
1903+
final boolean previouslyValidated = nai.validated;
19001904
nai.validated = true;
1901-
rematchNetworkAndRequests(nai);
1905+
rematchNetworkAndRequests(nai, !previouslyValidated);
19021906
}
19031907
updateInetCondition(nai, valid);
19041908
// Let the NetworkAgent know the state of its network
@@ -2071,6 +2075,7 @@ private void handleAsyncChannelDisconnected(Message msg) {
20712075
}
20722076
// Since we've lost the network, go through all the requests that
20732077
// it was satisfying and see if any other factory can satisfy them.
2078+
// TODO: This logic may be better replaced with a call to rematchAllNetworksAndRequests
20742079
final ArrayList<NetworkAgentInfo> toActivate = new ArrayList<NetworkAgentInfo>();
20752080
for (int i = 0; i < nai.networkRequests.size(); i++) {
20762081
NetworkRequest request = nai.networkRequests.valueAt(i);
@@ -2106,7 +2111,9 @@ private void handleAsyncChannelDisconnected(Message msg) {
21062111
requestNetworkTransitionWakelock(nai.name());
21072112
}
21082113
for (NetworkAgentInfo networkToActivate : toActivate) {
2114+
networkToActivate.networkLingered.clear();
21092115
networkToActivate.networkMonitor.sendMessage(NetworkMonitor.CMD_NETWORK_CONNECTED);
2116+
rematchNetworkAndRequests(networkToActivate, false);
21102117
}
21112118
}
21122119
}
@@ -2144,6 +2151,7 @@ private void handleRegisterNetworkRequest(Message msg) {
21442151
bestNetwork.networkLingered.clear();
21452152
bestNetwork.networkMonitor.sendMessage(NetworkMonitor.CMD_NETWORK_CONNECTED);
21462153
}
2154+
// TODO: This logic may be better replaced with a call to rematchNetworkAndRequests
21472155
bestNetwork.addRequest(nri.request);
21482156
mNetworkForRequestId.put(nri.request.requestId, bestNetwork);
21492157
notifyNetworkCallback(bestNetwork, nri);
@@ -3768,21 +3776,32 @@ private void makeDefault(NetworkAgentInfo newNetwork) {
37683776
// satisfied by newNetwork, and reassigns to newNetwork
37693777
// any such requests for which newNetwork is the best.
37703778
//
3771-
// - Tears down any Networks that as a result are no longer
3779+
// - Lingers any Networks that as a result are no longer
37723780
// needed. A network is needed if it is the best network for
37733781
// one or more NetworkRequests, or if it is a VPN.
37743782
//
3775-
// - Tears down newNetwork if it is validated but turns out to be
3776-
// unneeded. Does not tear down newNetwork if it is
3777-
// unvalidated, because future validation may improve
3778-
// newNetwork's score enough that it is needed.
3783+
// - Tears down newNetwork if it just became validated
3784+
// (i.e. nascent==true) but turns out to be unneeded.
3785+
// Does not tear down newNetwork if it is unvalidated,
3786+
// because future validation may improve newNetwork's
3787+
// score enough that it is needed.
37793788
//
37803789
// NOTE: This function only adds NetworkRequests that "newNetwork" could satisfy,
37813790
// it does not remove NetworkRequests that other Networks could better satisfy.
37823791
// If you need to handle decreases in score, use {@link rematchAllNetworksAndRequests}.
37833792
// This function should be used when possible instead of {@code rematchAllNetworksAndRequests}
37843793
// as it performs better by a factor of the number of Networks.
3785-
private void rematchNetworkAndRequests(NetworkAgentInfo newNetwork) {
3794+
//
3795+
// @param nascent indicates if newNetwork just became validated, in which case it should be
3796+
// torn down if unneeded. If nascent is false, no action is taken if newNetwork
3797+
// is found to be unneeded by this call. Presumably, in this case, either:
3798+
// - newNetwork is unvalidated (and left alive), or
3799+
// - the NetworkRequests keeping newNetwork alive have been transitioned to
3800+
// another higher scoring network by another call to rematchNetworkAndRequests()
3801+
// and this other call also lingered newNetwork.
3802+
private void rematchNetworkAndRequests(NetworkAgentInfo newNetwork, boolean nascent) {
3803+
if (!newNetwork.created) loge("ERROR: uncreated network being rematched.");
3804+
if (nascent && !newNetwork.validated) loge("ERROR: nascent network not validated.");
37863805
boolean keep = newNetwork.isVPN();
37873806
boolean isNewDefault = false;
37883807
if (DBG) log("rematching " + newNetwork.name());
@@ -3868,7 +3887,7 @@ private void rematchNetworkAndRequests(NetworkAgentInfo newNetwork) {
38683887
}
38693888
// Linger any networks that are no longer needed.
38703889
for (NetworkAgentInfo nai : affectedNetworks) {
3871-
boolean teardown = !nai.isVPN();
3890+
boolean teardown = !nai.isVPN() && nai.validated;
38723891
for (int i = 0; i < nai.networkRequests.size() && teardown; i++) {
38733892
NetworkRequest nr = nai.networkRequests.valueAt(i);
38743893
try {
@@ -3927,10 +3946,12 @@ private void rematchNetworkAndRequests(NetworkAgentInfo newNetwork) {
39273946
}
39283947

39293948
notifyNetworkCallbacks(newNetwork, ConnectivityManager.CALLBACK_AVAILABLE);
3930-
} else if (newNetwork.validated) {
3931-
// Only tear down validated networks here. Leave unvalidated to either become
3949+
} else if (nascent) {
3950+
// Only tear down newly validated networks here. Leave unvalidated to either become
39323951
// validated (and get evaluated against peers, one losing here) or
39333952
// NetworkMonitor reports a bad network and we tear it down then.
3953+
// Networks that have been up for a while and are validated should be torn down via
3954+
// the lingering process so communication on that network is given time to wrap up.
39343955
// TODO: Could teardown unvalidated networks when their NetworkCapabilities
39353956
// satisfy no NetworkRequests.
39363957
if (DBG && newNetwork.networkRequests.size() != 0) {
@@ -3962,10 +3983,10 @@ private void rematchAllNetworksAndRequests(NetworkAgentInfo changed, int oldScor
39623983
// can only add more NetworkRequests satisfied by "changed", and this is exactly what
39633984
// rematchNetworkAndRequests() handles.
39643985
if (changed != null && oldScore < changed.getCurrentScore()) {
3965-
rematchNetworkAndRequests(changed);
3986+
rematchNetworkAndRequests(changed, false);
39663987
} else {
39673988
for (NetworkAgentInfo nai : mNetworkAgentInfos.values()) {
3968-
rematchNetworkAndRequests(nai);
3989+
rematchNetworkAndRequests(nai, false);
39693990
}
39703991
}
39713992
}
@@ -4039,14 +4060,7 @@ private void updateNetworkInfo(NetworkAgentInfo networkAgent, NetworkInfo newInf
40394060
// TODO: support proxy per network.
40404061
}
40414062
// Consider network even though it is not yet validated.
4042-
// TODO: All the if-statement conditions can be removed now that validation only confers
4043-
// a score increase.
4044-
if (mNetworkForRequestId.get(mDefaultRequest.requestId) == null &&
4045-
networkAgent.isVPN() == false &&
4046-
mDefaultRequest.networkCapabilities.satisfiedByNetworkCapabilities(
4047-
networkAgent.networkCapabilities)) {
4048-
rematchNetworkAndRequests(networkAgent);
4049-
}
4063+
rematchNetworkAndRequests(networkAgent, false);
40504064
} else if (state == NetworkInfo.State.DISCONNECTED ||
40514065
state == NetworkInfo.State.SUSPENDED) {
40524066
networkAgent.asyncChannel.disconnect();

0 commit comments

Comments
 (0)