From 96742d9695fad0711dd125be7045bb59af9869c9 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Fri, 29 Jan 2021 20:18:03 +0900 Subject: [PATCH] More cleanly separate requests and listens. Currently, NetworkRequest has several types of requests (LISTEN, TRACK_DEFAULT, REQUEST, BACKGROUND_REQUEST), and we expect more to be added. There are really three categories of request: 1. Requests satisfied by only one network and will keep that network up, and thus need to be sent to NetworkProviders: REQUEST, BACKGROUND_REQUEST. 2. Requests satisfied by only one network but will not keep that network up: TRACK_DEFAULT 3. Requests satisfied by multiple networks and will not keep any networks up: LISTEN. Unfortunately the separation is not very clear. Currently, for any valid request, either isListen() will return true or isRequest() will return true. This makes it impossible to tell whether a particular request should be sent to NetworkProviders, so the current code sends TRACK_DEFAULT requests to NetworkProviders as well. This is incorrect - a TRACK_DEFAULT should never keep a network up, for example. This CL attempts to clarify things by making isRequest() return false for TRACK_DEFAULT requests and thus never sending them to NetworkProviders. After this CL: - isRequest will return true only for requests that attempt to bring up or keep up a network. - isListen will return true only for requests that match multiple networks but do not keep any of them up. - Neither will return true for TRACK_DEFAULT. Test: atest ConnectivityServiceTest Change-Id: I7aad30ade8f7ab2a179e53483d9afd8675f64a12 --- framework/src/android/net/NetworkRequest.java | 20 +----------------- .../android/server/ConnectivityService.java | 21 ++++++++----------- 2 files changed, 10 insertions(+), 31 deletions(-) diff --git a/framework/src/android/net/NetworkRequest.java b/framework/src/android/net/NetworkRequest.java index c4d1b09a5c..b9ef4c21ef 100644 --- a/framework/src/android/net/NetworkRequest.java +++ b/framework/src/android/net/NetworkRequest.java @@ -435,25 +435,7 @@ public class NetworkRequest implements Parcelable { * @hide */ public boolean isRequest() { - return isForegroundRequest() || isBackgroundRequest(); - } - - /** - * Returns true iff. the contained NetworkRequest is one that: - * - * - should be associated with at most one satisfying network - * at a time; - * - * - should cause a network to be kept up and in the foreground if - * it is the best network which can satisfy the NetworkRequest. - * - * For full detail of how isRequest() is used for pairing Networks with - * NetworkRequests read rematchNetworkAndRequests(). - * - * @hide - */ - public boolean isForegroundRequest() { - return type == Type.TRACK_DEFAULT || type == Type.REQUEST; + return type == Type.REQUEST || type == Type.BACKGROUND_REQUEST; } /** diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index bf9d564e43..6789de0695 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -3588,10 +3588,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // As this request was not satisfied on rematch and thus never had any scores sent to the // factories, send null now for each request of type REQUEST. for (final NetworkRequest req : nri.mRequests) { - if (!req.isRequest()) { - continue; - } - sendUpdatedScoreToFactories(req, null); + if (req.isRequest()) sendUpdatedScoreToFactories(req, null); } } @@ -3768,7 +3765,7 @@ public class ConnectivityService extends IConnectivityManager.Stub mNetworkRequestInfoLogs.log("RELEASE " + nri); if (null != nri.getActiveRequest()) { - if (nri.getActiveRequest().isRequest()) { + if (!nri.getActiveRequest().isListen()) { removeSatisfiedNetworkRequestFromNetwork(nri); } else { nri.setSatisfier(null, null); @@ -5516,7 +5513,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } // The network currently satisfying this NRI. Only one request in an NRI can have a - // satisfier. For non-multilayer requests, only REQUEST-type requests can have a satisfier. + // satisfier. For non-multilayer requests, only non-listen requests can have a satisfier. @Nullable private NetworkAgentInfo mSatisfier; NetworkAgentInfo getSatisfier() { @@ -7004,8 +7001,8 @@ public class ConnectivityService extends IConnectivityManager.Stub private void sendUpdatedScoreToFactories(NetworkAgentInfo nai) { for (int i = 0; i < nai.numNetworkRequests(); i++) { NetworkRequest nr = nai.requestAt(i); - // Don't send listening requests to factories. b/17393458 - if (nr.isListen()) continue; + // Don't send listening or track default request to factories. b/17393458 + if (!nr.isRequest()) continue; sendUpdatedScoreToFactories(nr, nai); } } @@ -7067,10 +7064,10 @@ public class ConnectivityService extends IConnectivityManager.Stub ensureRunningOnConnectivityServiceThread(); for (final NetworkRequestInfo nri : getNrisFromGlobalRequests()) { for (final NetworkRequest req : nri.mRequests) { - if (req.isListen() && nri.getActiveRequest() == req) { + if (!req.isRequest() && nri.getActiveRequest() == req) { break; } - if (req.isListen()) { + if (!req.isRequest()) { continue; } // Only set the nai for the request it is satisfying. @@ -7220,8 +7217,8 @@ public class ConnectivityService extends IConnectivityManager.Stub if (nai.numRequestNetworkRequests() != 0) { for (int i = 0; i < nai.numNetworkRequests(); i++) { NetworkRequest nr = nai.requestAt(i); - // Ignore listening requests. - if (nr.isListen()) continue; + // Ignore listening and track default requests. + if (!nr.isRequest()) continue; loge("Dead network still had at least " + nr); break; }