From d56c485af9eb0df5dc2b9f9269b4a5ca4305e779 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Thu, 7 Nov 2019 18:24:01 +0900 Subject: [PATCH 1/2] [NS A07] Move the last side effects out of the decision loop. This is a no-op. Reviewers : please scrutinize this for behavior changes. Test: ConnectivityServiceTest Change-Id: Icc621f0a64a72dae0192843e6b6587ec4e31ea4d --- .../android/server/ConnectivityService.java | 65 ++++++++++--------- 1 file changed, 33 insertions(+), 32 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index d714db7960..0a733cec0f 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -6366,37 +6366,6 @@ public class ConnectivityService extends IConnectivityManager.Stub } if (currentNetwork == null || currentNetwork.getCurrentScore() < score) { reassignedRequests.put(nri, newNetwork); - if (VDBG) log("rematch for " + newNetwork.name()); - if (currentNetwork != null) { - if (VDBG || DDBG){ - log(" accepting network in place of " + currentNetwork.name()); - } - currentNetwork.removeRequest(nri.request.requestId); - currentNetwork.lingerRequest(nri.request, now, mLingerDelayMs); - removedRequests.add(currentNetwork); - } else { - if (VDBG || DDBG) log(" accepting network in place of null"); - } - newNetwork.unlingerRequest(nri.request); - nri.mSatisfier = newNetwork; - if (!newNetwork.addRequest(nri.request)) { - Slog.wtf(TAG, "BUG: " + newNetwork.name() + " already has " + nri.request); - } - addedRequests.add(nri); - keep = true; - // Tell NetworkFactories about the new score, so they can stop - // trying to connect if they know they cannot match it. - // TODO - this could get expensive if we have a lot of requests for this - // network. Think about if there is a way to reduce this. Push - // netid->request mapping to each factory? - sendUpdatedScoreToFactories(nri.request, newNetwork); - if (isDefaultRequest(nri)) { - isNewDefault = true; - oldDefaultNetwork = currentNetwork; - if (currentNetwork != null) { - mLingerMonitor.noteLingerDefaultNetwork(currentNetwork, newNetwork); - } - } } } else if (newNetwork.isSatisfyingRequest(nri.request.requestId)) { reassignedRequests.put(nri, null); @@ -6408,7 +6377,39 @@ public class ConnectivityService extends IConnectivityManager.Stub final NetworkRequestInfo nri = entry.getKey(); final NetworkAgentInfo previousSatisfier = nri.mSatisfier; final NetworkAgentInfo newSatisfier = entry.getValue(); - if (newSatisfier == null) { + if (newSatisfier != null) { + if (VDBG) log("rematch for " + newSatisfier.name()); + if (previousSatisfier != null) { + if (VDBG || DDBG) { + log(" accepting network in place of " + previousSatisfier.name()); + } + previousSatisfier.removeRequest(nri.request.requestId); + previousSatisfier.lingerRequest(nri.request, now, mLingerDelayMs); + removedRequests.add(previousSatisfier); + } else { + if (VDBG || DDBG) log(" accepting network in place of null"); + } + newSatisfier.unlingerRequest(nri.request); + nri.mSatisfier = newSatisfier; + if (!newSatisfier.addRequest(nri.request)) { + Slog.wtf(TAG, "BUG: " + newSatisfier.name() + " already has " + nri.request); + } + addedRequests.add(nri); + keep = true; + // Tell NetworkFactories about the new score, so they can stop + // trying to connect if they know they cannot match it. + // TODO - this could get expensive if we have a lot of requests for this + // network. Think about if there is a way to reduce this. Push + // netid->request mapping to each factory? + sendUpdatedScoreToFactories(nri.request, newSatisfier); + if (isDefaultRequest(nri)) { + isNewDefault = true; + oldDefaultNetwork = previousSatisfier; + if (previousSatisfier != null) { + mLingerMonitor.noteLingerDefaultNetwork(previousSatisfier, newSatisfier); + } + } + } else { // If "newNetwork" is listed as satisfying "nri" but no longer satisfies "nri", // mark it as no longer satisfying "nri". Because networks are processed by // rematchAllNetworksAndRequests() in descending score order, "currentNetwork" will From f728b7c91b68e5c4b35f322c45c2cca5cc45d539 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Thu, 7 Nov 2019 18:32:38 +0900 Subject: [PATCH 2/2] [NS A08] Tiny refactoring Test: ConnectivityServiceTest Change-Id: I52f70756003cef61b450e9c1e9e57dc70f3091d5 --- .../core/java/com/android/server/ConnectivityService.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 0a733cec0f..a75d5d696b 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -6336,10 +6336,6 @@ public class ConnectivityService extends IConnectivityManager.Stub final ArrayMap reassignedRequests = new ArrayMap<>(); - // Find and migrate to this Network any NetworkRequests for - // which this network is now the best. - final ArrayList removedRequests = new ArrayList<>(); - final ArrayList addedRequests = new ArrayList<>(); NetworkCapabilities nc = newNetwork.networkCapabilities; if (VDBG) log(" network has: " + nc); for (NetworkRequestInfo nri : mNetworkRequests.values()) { @@ -6372,6 +6368,10 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + // Find and migrate to this Network any NetworkRequests for + // which this network is now the best. + final ArrayList removedRequests = new ArrayList<>(); + final ArrayList addedRequests = new ArrayList<>(); for (final Map.Entry entry : reassignedRequests.entrySet()) { final NetworkRequestInfo nri = entry.getKey();