From d8bea3bb9057645cdd05fa6f434f1d99efbefee5 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Mon, 2 Dec 2019 18:59:27 +0900 Subject: [PATCH 1/2] [NS A26] Move available callbacks out of the rematch computation Bug: 113554781 Test: ConnectivityServiceTest Change-Id: I3a804a9f6eaf50a3995eaaf6469a1c2b9387be14 --- .../android/server/ConnectivityService.java | 39 ++++++++++++++++--- 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 478b87c010..2852a21a0a 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -6323,12 +6323,34 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + static class RequestReassignment { + @NonNull public final NetworkRequestInfo mRequest; + @Nullable public final NetworkAgentInfo mOldNetwork; + @Nullable public final NetworkAgentInfo mNewNetwork; + RequestReassignment(@NonNull final NetworkRequestInfo request, + @Nullable final NetworkAgentInfo oldNetwork, + @Nullable final NetworkAgentInfo newNetwork) { + mRequest = request; + mOldNetwork = oldNetwork; + mNewNetwork = newNetwork; + } + } + @NonNull private final Set mRematchedNetworks = new ArraySet<>(); + @NonNull private final List mReassignments = new ArrayList<>(); @NonNull Iterable getRematchedNetworks() { return mRematchedNetworks; } + @NonNull Iterable getRequestReassignments() { + return mReassignments; + } + + void addRequestReassignment(@NonNull final RequestReassignment reassignment) { + mReassignments.add(reassignment); + } + void addRematchedNetwork(@NonNull final NetworkBgStatePair network) { mRematchedNetworks.add(network); } @@ -6417,7 +6439,6 @@ 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(); @@ -6440,7 +6461,8 @@ public class ConnectivityService extends IConnectivityManager.Stub if (!newSatisfier.addRequest(nri.request)) { Slog.wtf(TAG, "BUG: " + newSatisfier.name() + " already has " + nri.request); } - addedRequests.add(nri); + changes.addRequestReassignment(new NetworkReassignment.RequestReassignment( + nri, previousSatisfier, newSatisfier)); // Tell NetworkProviders 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 @@ -6508,10 +6530,6 @@ public class ConnectivityService extends IConnectivityManager.Stub "BUG: %s changed score during rematch: %d -> %d", newNetwork.name(), score, newNetwork.getCurrentScore())); } - - // Notify requested networks are available after the default net is switched, but - // before LegacyTypeTracker sends legacy broadcasts - for (NetworkRequestInfo nri : addedRequests) notifyNetworkAvailable(newNetwork, nri); } /** @@ -6540,6 +6558,15 @@ public class ConnectivityService extends IConnectivityManager.Stub final NetworkAgentInfo newDefaultNetwork = getDefaultNetwork(); + // Notify requested networks are available after the default net is switched, but + // before LegacyTypeTracker sends legacy broadcasts + for (final NetworkReassignment.RequestReassignment event : + changes.getRequestReassignments()) { + if (null != event.mNewNetwork) { + notifyNetworkAvailable(event.mNewNetwork, event.mRequest); + } + } + for (final NetworkReassignment.NetworkBgStatePair event : changes.getRematchedNetworks()) { // Process listen requests and update capabilities if the background state has // changed for this network. For consistency with previous behavior, send onLost From 0c7b9a9eb7d7790d590b0e7b8eaa1471bd82ffff Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Tue, 3 Dec 2019 14:07:57 +0900 Subject: [PATCH 2/2] [NS A27] Remove useless logs and a useless var These logs haven't found a bug in a long time and we now have some structural guarantees that the conditions they check for can't happen (like the checks that everything is happening on the same thread). Maybe we'll reinstate similar checks later, but for now they are in the way and removing them is a small sacrifice for the intended benefit. The local was simply not used any more. Test: FrameworksNetTests Change-Id: If8c8d1f3eb883ffcf0fbdb70824b87dd70da507c --- .../android/server/ConnectivityService.java | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 2852a21a0a..31edc5606a 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -6426,19 +6426,13 @@ public class ConnectivityService extends IConnectivityManager.Stub changes.addRematchedNetwork(new NetworkReassignment.NetworkBgStatePair(newNetwork, newNetwork.isBackgroundNetwork())); - final int score = newNetwork.getCurrentScore(); - if (VDBG || DDBG) log("rematching " + newNetwork.name()); final ArrayMap reassignedRequests = computeRequestReassignmentForNetwork(newNetwork); - NetworkCapabilities nc = newNetwork.networkCapabilities; - if (VDBG) log(" network has: " + nc); - // Find and migrate to this Network any NetworkRequests for // which this network is now the best. - final ArrayList removedRequests = new ArrayList<>(); for (final Map.Entry entry : reassignedRequests.entrySet()) { final NetworkRequestInfo nri = entry.getKey(); @@ -6452,7 +6446,6 @@ public class ConnectivityService extends IConnectivityManager.Stub } 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"); } @@ -6519,17 +6512,6 @@ public class ConnectivityService extends IConnectivityManager.Stub // Have a new default network, release the transition wakelock in scheduleReleaseNetworkTransitionWakelock(); } - - if (!newNetwork.networkCapabilities.equalRequestableCapabilities(nc)) { - Slog.wtf(TAG, String.format( - "BUG: %s changed requestable capabilities during rematch: %s -> %s", - newNetwork.name(), nc, newNetwork.networkCapabilities)); - } - if (newNetwork.getCurrentScore() != score) { - Slog.wtf(TAG, String.format( - "BUG: %s changed score during rematch: %d -> %d", - newNetwork.name(), score, newNetwork.getCurrentScore())); - } } /**