Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 97d09a26 authored by Lorenzo Colitti's avatar Lorenzo Colitti
Browse files

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
parent 2954e8af
Loading
Loading
Loading
Loading
+1 −19
Original line number Diff line number Diff line
@@ -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;
    }

    /**
+9 −12
Original line number Diff line number Diff line
@@ -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;
            }