Skip to content

Commit 6eead85

Browse files
JensenPaulAndroid (Google) Code Review
authored andcommitted
Merge "Allow any unvalidated Network to satisfy NetworkRequests." into lmp-dev
2 parents b591da9 + 4b9b2be commit 6eead85

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
@@ -1895,6 +1895,9 @@ public void handleMessage(Message msg) {
18951895
loge("EVENT_SET_EXPLICITLY_SELECTED from unknown NetworkAgent");
18961896
break;
18971897
}
1898+
if (nai.created && !nai.networkMisc.explicitlySelected) {
1899+
loge("ERROR: created network explicitly selected.");
1900+
}
18981901
nai.networkMisc.explicitlySelected = true;
18991902
break;
19001903
}
@@ -1904,8 +1907,9 @@ public void handleMessage(Message msg) {
19041907
boolean valid = (msg.arg1 == NetworkMonitor.NETWORK_TEST_RESULT_VALID);
19051908
if (valid) {
19061909
if (DBG) log("Validated " + nai.name());
1910+
final boolean previouslyValidated = nai.validated;
19071911
nai.validated = true;
1908-
rematchNetworkAndRequests(nai);
1912+
rematchNetworkAndRequests(nai, !previouslyValidated);
19091913
}
19101914
updateInetCondition(nai, valid);
19111915
// Let the NetworkAgent know the state of its network
@@ -2078,6 +2082,7 @@ private void handleAsyncChannelDisconnected(Message msg) {
20782082
}
20792083
// Since we've lost the network, go through all the requests that
20802084
// it was satisfying and see if any other factory can satisfy them.
2085+
// TODO: This logic may be better replaced with a call to rematchAllNetworksAndRequests
20812086
final ArrayList<NetworkAgentInfo> toActivate = new ArrayList<NetworkAgentInfo>();
20822087
for (int i = 0; i < nai.networkRequests.size(); i++) {
20832088
NetworkRequest request = nai.networkRequests.valueAt(i);
@@ -2113,7 +2118,9 @@ private void handleAsyncChannelDisconnected(Message msg) {
21132118
requestNetworkTransitionWakelock(nai.name());
21142119
}
21152120
for (NetworkAgentInfo networkToActivate : toActivate) {
2121+
networkToActivate.networkLingered.clear();
21162122
networkToActivate.networkMonitor.sendMessage(NetworkMonitor.CMD_NETWORK_CONNECTED);
2123+
rematchNetworkAndRequests(networkToActivate, false);
21172124
}
21182125
}
21192126
}
@@ -2151,6 +2158,7 @@ private void handleRegisterNetworkRequest(Message msg) {
21512158
bestNetwork.networkLingered.clear();
21522159
bestNetwork.networkMonitor.sendMessage(NetworkMonitor.CMD_NETWORK_CONNECTED);
21532160
}
2161+
// TODO: This logic may be better replaced with a call to rematchNetworkAndRequests
21542162
bestNetwork.addRequest(nri.request);
21552163
mNetworkForRequestId.put(nri.request.requestId, bestNetwork);
21562164
notifyNetworkCallback(bestNetwork, nri);
@@ -3775,21 +3783,32 @@ private void makeDefault(NetworkAgentInfo newNetwork) {
37753783
// satisfied by newNetwork, and reassigns to newNetwork
37763784
// any such requests for which newNetwork is the best.
37773785
//
3778-
// - Tears down any Networks that as a result are no longer
3786+
// - Lingers any Networks that as a result are no longer
37793787
// needed. A network is needed if it is the best network for
37803788
// one or more NetworkRequests, or if it is a VPN.
37813789
//
3782-
// - Tears down newNetwork if it is validated but turns out to be
3783-
// unneeded. Does not tear down newNetwork if it is
3784-
// unvalidated, because future validation may improve
3785-
// newNetwork's score enough that it is needed.
3790+
// - Tears down newNetwork if it just became validated
3791+
// (i.e. nascent==true) but turns out to be unneeded.
3792+
// Does not tear down newNetwork if it is unvalidated,
3793+
// because future validation may improve newNetwork's
3794+
// score enough that it is needed.
37863795
//
37873796
// NOTE: This function only adds NetworkRequests that "newNetwork" could satisfy,
37883797
// it does not remove NetworkRequests that other Networks could better satisfy.
37893798
// If you need to handle decreases in score, use {@link rematchAllNetworksAndRequests}.
37903799
// This function should be used when possible instead of {@code rematchAllNetworksAndRequests}
37913800
// as it performs better by a factor of the number of Networks.
3792-
private void rematchNetworkAndRequests(NetworkAgentInfo newNetwork) {
3801+
//
3802+
// @param nascent indicates if newNetwork just became validated, in which case it should be
3803+
// torn down if unneeded. If nascent is false, no action is taken if newNetwork
3804+
// is found to be unneeded by this call. Presumably, in this case, either:
3805+
// - newNetwork is unvalidated (and left alive), or
3806+
// - the NetworkRequests keeping newNetwork alive have been transitioned to
3807+
// another higher scoring network by another call to rematchNetworkAndRequests()
3808+
// and this other call also lingered newNetwork.
3809+
private void rematchNetworkAndRequests(NetworkAgentInfo newNetwork, boolean nascent) {
3810+
if (!newNetwork.created) loge("ERROR: uncreated network being rematched.");
3811+
if (nascent && !newNetwork.validated) loge("ERROR: nascent network not validated.");
37933812
boolean keep = newNetwork.isVPN();
37943813
boolean isNewDefault = false;
37953814
if (DBG) log("rematching " + newNetwork.name());
@@ -3875,7 +3894,7 @@ private void rematchNetworkAndRequests(NetworkAgentInfo newNetwork) {
38753894
}
38763895
// Linger any networks that are no longer needed.
38773896
for (NetworkAgentInfo nai : affectedNetworks) {
3878-
boolean teardown = !nai.isVPN();
3897+
boolean teardown = !nai.isVPN() && nai.validated;
38793898
for (int i = 0; i < nai.networkRequests.size() && teardown; i++) {
38803899
NetworkRequest nr = nai.networkRequests.valueAt(i);
38813900
try {
@@ -3934,10 +3953,12 @@ private void rematchNetworkAndRequests(NetworkAgentInfo newNetwork) {
39343953
}
39353954

39363955
notifyNetworkCallbacks(newNetwork, ConnectivityManager.CALLBACK_AVAILABLE);
3937-
} else if (newNetwork.validated) {
3938-
// Only tear down validated networks here. Leave unvalidated to either become
3956+
} else if (nascent) {
3957+
// Only tear down newly validated networks here. Leave unvalidated to either become
39393958
// validated (and get evaluated against peers, one losing here) or
39403959
// NetworkMonitor reports a bad network and we tear it down then.
3960+
// Networks that have been up for a while and are validated should be torn down via
3961+
// the lingering process so communication on that network is given time to wrap up.
39413962
// TODO: Could teardown unvalidated networks when their NetworkCapabilities
39423963
// satisfy no NetworkRequests.
39433964
if (DBG && newNetwork.networkRequests.size() != 0) {
@@ -3969,10 +3990,10 @@ private void rematchAllNetworksAndRequests(NetworkAgentInfo changed, int oldScor
39693990
// can only add more NetworkRequests satisfied by "changed", and this is exactly what
39703991
// rematchNetworkAndRequests() handles.
39713992
if (changed != null && oldScore < changed.getCurrentScore()) {
3972-
rematchNetworkAndRequests(changed);
3993+
rematchNetworkAndRequests(changed, false);
39733994
} else {
39743995
for (NetworkAgentInfo nai : mNetworkAgentInfos.values()) {
3975-
rematchNetworkAndRequests(nai);
3996+
rematchNetworkAndRequests(nai, false);
39763997
}
39773998
}
39783999
}
@@ -4046,14 +4067,7 @@ private void updateNetworkInfo(NetworkAgentInfo networkAgent, NetworkInfo newInf
40464067
// TODO: support proxy per network.
40474068
}
40484069
// Consider network even though it is not yet validated.
4049-
// TODO: All the if-statement conditions can be removed now that validation only confers
4050-
// a score increase.
4051-
if (mNetworkForRequestId.get(mDefaultRequest.requestId) == null &&
4052-
networkAgent.isVPN() == false &&
4053-
mDefaultRequest.networkCapabilities.satisfiedByNetworkCapabilities(
4054-
networkAgent.networkCapabilities)) {
4055-
rematchNetworkAndRequests(networkAgent);
4056-
}
4070+
rematchNetworkAndRequests(networkAgent, false);
40574071
} else if (state == NetworkInfo.State.DISCONNECTED ||
40584072
state == NetworkInfo.State.SUSPENDED) {
40594073
networkAgent.asyncChannel.disconnect();

0 commit comments

Comments
 (0)