From 0c38d7c8cde5cb67f8b904c933434ccd76f11b11 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Wed, 20 Jul 2016 02:39:22 +0900 Subject: [PATCH] Rematch requests first and listens second. This CL splits rematching in two parts: first rematch requests, then rematch listens. This will allow us to change a network's capabilities depending on what requests are on that network, and properly dispatch callbacks depending on those capabilities. The behaviour changes are as follows: - Before this CL, callbacks for requests and listens were sent intermingled. After this CL, all request callbacks will be grouped together, and all listen callbacks will be grouped together. - Before this CL, the order was: 1. Send onLost callbacks. 2. If applicable, switch the default network. 3. Send onAvailable callbacks. After this CL, the order is: 1. Send onLost callbacks for requests. 2. If applicable, switch the default network. 3. Send onLost callbacks for listens. 4. Send onAvailable callbacks for listens. 5. Send onAvailable callbacks for requests. These changes shouldn't affect any apps because: 1. The order of callbacks continues to be first all onLost, then all onAvailable. 2. Both the old and the new code send callbacks in no particular order. Thus, the possible ordering combinations of callbacks in the new code are a strict subset of the possible ordering combinations of the old code. 3. The default network is switched before any onAvailable callback is sent. 4. Even though the new code does not send all onLost callbacks before switching the default network, even before this CL there was no guarantee that those callbacks would be received before the default network switch anyway, because callbacks are asynchronous. Bug: 23113288 Change-Id: Ia08900c50db9ff43895047e2f4f36b6c6c31a1f9 --- .../android/server/ConnectivityService.java | 53 +++++++++++++------ 1 file changed, 37 insertions(+), 16 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 58431c856d..0c23d1a3ab 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -4631,6 +4631,27 @@ public class ConnectivityService extends IConnectivityManager.Stub setDefaultDnsSystemProperties(newNetwork.linkProperties.getDnsServers()); } + private void processListenRequests(NetworkAgentInfo nai) { + // For consistency with previous behaviour, send onLost callbacks before onAvailable. + for (NetworkRequestInfo nri : mNetworkRequests.values()) { + NetworkRequest nr = nri.request; + if (!nr.isListen()) continue; + if (nai.isSatisfyingRequest(nr.requestId) && !nai.satisfies(nr)) { + nai.removeRequest(nri.request.requestId); + callCallbackForRequest(nri, nai, ConnectivityManager.CALLBACK_LOST, 0); + } + } + + for (NetworkRequestInfo nri : mNetworkRequests.values()) { + NetworkRequest nr = nri.request; + if (!nr.isListen()) continue; + if (nai.satisfies(nr) && !nai.isSatisfyingRequest(nr.requestId)) { + nai.addRequest(nr); + notifyNetworkCallback(nai, nri); + } + } + } + // Handles a network appearing or improving its score. // // - Evaluates all current NetworkRequests that can be @@ -4671,6 +4692,12 @@ public class ConnectivityService extends IConnectivityManager.Stub ArrayList addedRequests = new ArrayList(); if (VDBG) log(" network has: " + newNetwork.networkCapabilities); for (NetworkRequestInfo nri : mNetworkRequests.values()) { + // Process requests in the first pass and listens in the second pass. This allows us to + // change a network's capabilities depending on which requests it has. This is only + // correct if the change in capabilities doesn't affect whether the network satisfies + // requests or not, and doesn't affect the network's score. + if (nri.request.isListen()) continue; + final NetworkAgentInfo currentNetwork = mNetworkForRequestId.get(nri.request.requestId); final boolean satisfies = newNetwork.satisfies(nri.request); if (newNetwork == currentNetwork && satisfies) { @@ -4685,13 +4712,6 @@ public class ConnectivityService extends IConnectivityManager.Stub // check if it satisfies the NetworkCapabilities if (VDBG) log(" checking if request is satisfied: " + nri.request); if (satisfies) { - if (nri.request.isListen()) { - // This is not a request, it's a callback listener. - // Add it to newNetwork regardless of score. - if (newNetwork.addRequest(nri.request)) addedRequests.add(nri); - continue; - } - // next check if it's better than any current network we're using for // this request if (VDBG) { @@ -4748,16 +4768,14 @@ public class ConnectivityService extends IConnectivityManager.Stub mNetworkForRequestId.remove(nri.request.requestId); sendUpdatedScoreToFactories(nri.request, 0); } else { - if (nri.request.isRequest()) { - Slog.wtf(TAG, "BUG: Removing request " + nri.request.requestId + " from " + - newNetwork.name() + - " without updating mNetworkForRequestId or factories!"); - } + Slog.wtf(TAG, "BUG: Removing request " + nri.request.requestId + " from " + + newNetwork.name() + + " without updating mNetworkForRequestId or factories!"); } - // TODO: technically, sending CALLBACK_LOST here is - // incorrect if nri is a request (not a listen) and there - // is a replacement network currently connected that can - // satisfy it. However, the only capability that can both + // TODO: Technically, sending CALLBACK_LOST here is + // incorrect if there is a replacement network currently + // connected that can satisfy nri, which is a request + // (not a listen). However, the only capability that can both // a) be requested and b) change is NET_CAPABILITY_TRUSTED, // so this code is only incorrect for a network that loses // the TRUSTED capability, which is a rare case. @@ -4782,6 +4800,9 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + // Second pass: process all listens. + processListenRequests(newNetwork); + // do this after the default net is switched, but // before LegacyTypeTracker sends legacy broadcasts for (NetworkRequestInfo nri : addedRequests) notifyNetworkCallback(newNetwork, nri);