Merge "Cache network capabilities." into rvc-qpr-dev am: 2a1b3c7408

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/12901724

Change-Id: If16b0c7b056e9123926f68fa21b5cd4e298367c3

1 files changed, 46 insertions, 25 deletions

diff --git a/apex/jobscheduler/service/java/com/android/server/job/controllers/ConnectivityController.java b/apex/jobscheduler/service/java/com/android/server/job/controllers/ConnectivityController.java
index bb94275fc409..ba8348337981 100644
--- a/apex/jobscheduler/service/java/com/android/server/job/controllers/ConnectivityController.java
+++ b/apex/jobscheduler/service/java/com/android/server/job/controllers/ConnectivityController.java
@@ -22,6 +22,7 @@ import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_METERED;
 
 import static com.android.server.job.JobSchedulerService.RESTRICTED_INDEX;
 
+import android.annotation.Nullable;
 import android.app.job.JobInfo;
 import android.net.ConnectivityManager;
 import android.net.ConnectivityManager.NetworkCallback;
@@ -86,9 +87,12 @@ public final class ConnectivityController extends RestrictingController implemen
     @GuardedBy("mLock")
     private final SparseArray<ArraySet<JobStatus>> mRequestedWhitelistJobs = new SparseArray<>();
 
-    /** List of currently available networks. */
+    /**
+     * Set of currently available networks mapped to their latest network capabilities. Cache the
+     * latest capabilities to avoid unnecessary calls into ConnectivityManager.
+     */
     @GuardedBy("mLock")
-    private final ArraySet<Network> mAvailableNetworks = new ArraySet<>();
+    private final ArrayMap<Network, NetworkCapabilities> mAvailableNetworks = new ArrayMap<>();
 
     private static final int MSG_DATA_SAVER_TOGGLED = 0;
     private static final int MSG_UID_RULES_CHANGES = 1;
@@ -165,9 +169,8 @@ public final class ConnectivityController extends RestrictingController implemen
     public boolean isNetworkAvailable(JobStatus job) {
         synchronized (mLock) {
             for (int i = 0; i < mAvailableNetworks.size(); ++i) {
-                final Network network = mAvailableNetworks.valueAt(i);
-                final NetworkCapabilities capabilities = mConnManager.getNetworkCapabilities(
-                        network);
+                final Network network = mAvailableNetworks.keyAt(i);
+                final NetworkCapabilities capabilities = mAvailableNetworks.valueAt(i);
                 final boolean satisfied = isSatisfied(job, network, capabilities, mConstants);
                 if (DEBUG) {
                     Slog.v(TAG, "isNetworkAvailable(" + job + ") with network " + network
@@ -427,9 +430,33 @@ public final class ConnectivityController extends RestrictingController implemen
         return false;
     }
 
+    @Nullable
+    private NetworkCapabilities getNetworkCapabilities(@Nullable Network network) {
+        if (network == null) {
+            return null;
+        }
+        synchronized (mLock) {
+            // There is technically a race here if the Network object is reused. This can happen
+            // only if that Network disconnects and the auto-incrementing network ID in
+            // ConnectivityService wraps. This should no longer be a concern if/when we only make
+            // use of asynchronous calls.
+            if (mAvailableNetworks.get(network) != null) {
+                return mAvailableNetworks.get(network);
+            }
+
+            // This should almost never happen because any time a new network connects, the
+            // NetworkCallback would populate mAvailableNetworks. However, it's currently necessary
+            // because we also call synchronous methods such as getActiveNetworkForUid.
+            // TODO(134978280): remove after switching to callback-based APIs
+            final NetworkCapabilities capabilities = mConnManager.getNetworkCapabilities(network);
+            mAvailableNetworks.put(network, capabilities);
+            return capabilities;
+        }
+    }
+
     private boolean updateConstraintsSatisfied(JobStatus jobStatus) {
         final Network network = mConnManager.getActiveNetworkForUid(jobStatus.getSourceUid());
-        final NetworkCapabilities capabilities = mConnManager.getNetworkCapabilities(network);
+        final NetworkCapabilities capabilities = getNetworkCapabilities(network);
         return updateConstraintsSatisfied(jobStatus, network, capabilities);
     }
 
@@ -470,19 +497,13 @@ public final class ConnectivityController extends RestrictingController implemen
      */
     private void updateTrackedJobs(int filterUid, Network filterNetwork) {
         synchronized (mLock) {
-            // Since this is a really hot codepath, temporarily cache any
-            // answers that we get from ConnectivityManager.
-            final ArrayMap<Network, NetworkCapabilities> networkToCapabilities = new ArrayMap<>();
-
             boolean changed = false;
             if (filterUid == -1) {
                 for (int i = mTrackedJobs.size() - 1; i >= 0; i--) {
-                    changed |= updateTrackedJobsLocked(mTrackedJobs.valueAt(i),
-                            filterNetwork, networkToCapabilities);
+                    changed |= updateTrackedJobsLocked(mTrackedJobs.valueAt(i), filterNetwork);
                 }
             } else {
-                changed = updateTrackedJobsLocked(mTrackedJobs.get(filterUid),
-                        filterNetwork, networkToCapabilities);
+                changed = updateTrackedJobsLocked(mTrackedJobs.get(filterUid), filterNetwork);
             }
             if (changed) {
                 mStateChangedListener.onControllerStateChanged();
@@ -490,18 +511,13 @@ public final class ConnectivityController extends RestrictingController implemen
         }
     }
 
-    private boolean updateTrackedJobsLocked(ArraySet<JobStatus> jobs, Network filterNetwork,
-            ArrayMap<Network, NetworkCapabilities> networkToCapabilities) {
+    private boolean updateTrackedJobsLocked(ArraySet<JobStatus> jobs, Network filterNetwork) {
         if (jobs == null || jobs.size() == 0) {
             return false;
         }
 
         final Network network = mConnManager.getActiveNetworkForUid(jobs.valueAt(0).getSourceUid());
-        NetworkCapabilities capabilities = networkToCapabilities.get(network);
-        if (capabilities == null) {
-            capabilities = mConnManager.getNetworkCapabilities(network);
-            networkToCapabilities.put(network, capabilities);
-        }
+        final NetworkCapabilities capabilities = getNetworkCapabilities(network);
         final boolean networkMatch = (filterNetwork == null
                 || Objects.equals(filterNetwork, network));
 
@@ -544,9 +560,9 @@ public final class ConnectivityController extends RestrictingController implemen
         @Override
         public void onAvailable(Network network) {
             if (DEBUG) Slog.v(TAG, "onAvailable: " + network);
-            synchronized (mLock) {
-                mAvailableNetworks.add(network);
-            }
+            // Documentation says not to call getNetworkCapabilities here but wait for
+            // onCapabilitiesChanged instead.  onCapabilitiesChanged should be called immediately
+            // after this, so no need to update mAvailableNetworks here.
         }
 
         @Override
@@ -554,6 +570,9 @@ public final class ConnectivityController extends RestrictingController implemen
             if (DEBUG) {
                 Slog.v(TAG, "onCapabilitiesChanged: " + network);
             }
+            synchronized (mLock) {
+                mAvailableNetworks.put(network, capabilities);
+            }
             updateTrackedJobs(-1, network);
         }
 
@@ -630,6 +649,8 @@ public final class ConnectivityController extends RestrictingController implemen
             pw.println("Available networks:");
             pw.increaseIndent();
             for (int i = 0; i < mAvailableNetworks.size(); i++) {
+                pw.print(mAvailableNetworks.keyAt(i));
+                pw.print(": ");
                 pw.println(mAvailableNetworks.valueAt(i));
             }
             pw.decreaseIndent();
@@ -667,7 +688,7 @@ public final class ConnectivityController extends RestrictingController implemen
                     mRequestedWhitelistJobs.keyAt(i));
         }
         for (int i = 0; i < mAvailableNetworks.size(); i++) {
-            Network network = mAvailableNetworks.valueAt(i);
+            Network network = mAvailableNetworks.keyAt(i);
             if (network != null) {
                 network.dumpDebug(proto,
                         StateControllerProto.ConnectivityController.AVAILABLE_NETWORKS);