From 9e8a3df4e5fab238ba082fa5737bfb4fcbc99f95 Mon Sep 17 00:00:00 2001
From: Kweku Adams <kwekua@google.com>
Date: Wed, 14 Apr 2021 09:08:54 -0700
Subject: [PATCH] Avoid spammy adjustCallback iterations.

Add a delay to calling maybeAdjustRegisteredCallbacks in some instances
so that we don't spin doing the same work with no substantial change.

Bug: 141645789
Bug: 184740668
Test: atest frameworks/base/services/tests/servicestests/src/com/android/server/job
Test: atest frameworks/base/services/tests/mockingservicestests/src/com/android/server/job
Test: atest CtsJobSchedulerTestCases
Change-Id: I39890192b3e96476e7416e046581e45eb7a20060
---
 .../controllers/ConnectivityController.java   | 26 ++++++++++++++++---
 1 file changed, 23 insertions(+), 3 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 11a8b3b730759..500735b0b2991 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
@@ -83,6 +83,7 @@ public final class ConnectivityController extends RestrictingController implemen
      * instance.
      */
     private static final long MIN_STATS_UPDATE_INTERVAL_MS = 30_000L;
+    private static final long MIN_ADJUST_CALLBACK_INTERVAL_MS = 1_000L;
 
     private static final int UNBYPASSABLE_BG_BLOCKED_REASONS =
             ~ConnectivityManager.BLOCKED_REASON_NONE;
@@ -210,6 +211,7 @@ public final class ConnectivityController extends RestrictingController implemen
      * is only done in {@link #maybeAdjustRegisteredCallbacksLocked()} and may sometimes be stale.
      */
     private final List<UidStats> mSortedStats = new ArrayList<>();
+    private long mLastCallbackAdjustmentTimeElapsed;
 
     private static final int MSG_ADJUST_CALLBACKS = 0;
 
@@ -693,7 +695,11 @@ public final class ConnectivityController extends RestrictingController implemen
     }
 
     private void postAdjustCallbacks() {
-        mHandler.obtainMessage(MSG_ADJUST_CALLBACKS).sendToTarget();
+        postAdjustCallbacks(0);
+    }
+
+    private void postAdjustCallbacks(long delayMs) {
+        mHandler.sendEmptyMessageDelayed(MSG_ADJUST_CALLBACKS, delayMs);
     }
 
     @GuardedBy("mLock")
@@ -708,6 +714,12 @@ public final class ConnectivityController extends RestrictingController implemen
         }
 
         final long nowElapsed = sElapsedRealtimeClock.millis();
+        if (nowElapsed - mLastCallbackAdjustmentTimeElapsed < MIN_ADJUST_CALLBACK_INTERVAL_MS) {
+            postAdjustCallbacks(MIN_ADJUST_CALLBACK_INTERVAL_MS);
+            return;
+        }
+
+        mLastCallbackAdjustmentTimeElapsed = nowElapsed;
         mSortedStats.clear();
 
         for (int u = 0; u < mUidStats.size(); ++u) {
@@ -926,7 +938,10 @@ public final class ConnectivityController extends RestrictingController implemen
         UidDefaultNetworkCallback defaultNetworkCallback =
                 mCurrentDefaultNetworkCallbacks.get(jobs.valueAt(0).getSourceUid());
         if (defaultNetworkCallback == null) {
-            maybeRegisterDefaultNetworkCallbackLocked(jobs.valueAt(0));
+            // This method is only called via a network callback object. That means something
+            // changed about a general network characteristic (since we wouldn't be in this
+            // situation if called from a UID_specific callback). The general network callback
+            // will handle adjusting the per-UID callbacks, so nothing left to do here.
             return false;
         }
 
@@ -1106,8 +1121,13 @@ public final class ConnectivityController extends RestrictingController implemen
             synchronized (mLock) {
                 if (Objects.equals(mDefaultNetwork, network)) {
                     mDefaultNetwork = null;
+                    updateTrackedJobsLocked(mUid, network);
+                    // Add a delay in case onAvailable()+onBlockedStatusChanged is called for a
+                    // new network. If this onLost was called because the network is completely
+                    // gone, the delay will hel make sure we don't have a short burst of adjusting
+                    // callback calls.
+                    postAdjustCallbacks(1000);
                 }
-                updateTrackedJobsLocked(mUid, network);
             }
         }