From a2282a9c6548618f9bf453def3da0d837abb9879 Mon Sep 17 00:00:00 2001
From: Kweku Adams <kwekua@google.com>
Date: Mon, 17 Apr 2023 20:03:24 +0000
Subject: [PATCH] Shift job readiness re-evaluation check.

Sometimes controllers actually update their relative bit on the job
when the JobSchedulerService double checks with controllers. However,
if JSS is looping through the changedJobs list when this happens, the
controller will tell JSS of the change, but adding it to the changedJobs
list will be a no-op so the job won't be added to the pending job list.
And if the controller gets a callback and evaluate the job on its own,
the bit won't change and so the job won't be added to the changedJobs
list. This leads to inconsistent behavior based on timing/order of
operations. Shifting the controller re-evaluation to when we're
determine the job's readiness fixes the issue.

Bug: 263216453
Bug: 263699506
Bug: 271128261
Test: atest --rerun-until-failure 50 android.jobscheduler.cts.JobThrottlingTest#testRestrictedEJAllowedWhenAutoRestrictedBucketFeatureOn
Test: atest FrameworksMockingServicesTests:JobSchedulerServiceTest
Change-Id: Idb41cf030f63c697adb8e7bfebc132019f9dffbc
---
 .../android/server/job/JobSchedulerService.java    | 14 +++++++-------
 .../server/job/JobSchedulerServiceTest.java        |  2 +-
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java b/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java
index 6eeff82f11584..577260e5106f4 100644
--- a/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java
+++ b/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java
@@ -1577,8 +1577,6 @@ public class JobSchedulerService extends com.android.server.SystemService
                 mJobPackageTracker.notePending(jobStatus);
                 mPendingJobQueue.add(jobStatus);
                 maybeRunPendingJobsLocked();
-            } else {
-                evaluateControllerStatesLocked(jobStatus);
             }
         }
         return JobScheduler.RESULT_SUCCESS;
@@ -3050,8 +3048,6 @@ public class JobSchedulerService extends com.android.server.SystemService
                     Slog.d(TAG, "    queued " + job.toShortString());
                 }
                 newReadyJobs.add(job);
-            } else {
-                evaluateControllerStatesLocked(job);
             }
         }
 
@@ -3171,7 +3167,6 @@ public class JobSchedulerService extends com.android.server.SystemService
                 } else if (mPendingJobQueue.remove(job)) {
                     noteJobNonPending(job);
                 }
-                evaluateControllerStatesLocked(job);
             }
         }
 
@@ -3297,7 +3292,7 @@ public class JobSchedulerService extends com.android.server.SystemService
 
     @GuardedBy("mLock")
     boolean isReadyToBeExecutedLocked(JobStatus job, boolean rejectActive) {
-        final boolean jobReady = job.isReady();
+        final boolean jobReady = job.isReady() || evaluateControllerStatesLocked(job);
 
         if (DEBUG) {
             Slog.v(TAG, "isReadyToBeExecutedLocked: " + job.toShortString()
@@ -3372,12 +3367,17 @@ public class JobSchedulerService extends com.android.server.SystemService
         return !appIsBad;
     }
 
+    /**
+     * Gets each controller to evaluate the job's state
+     * and then returns the value of {@link JobStatus#isReady()}.
+     */
     @VisibleForTesting
-    void evaluateControllerStatesLocked(final JobStatus job) {
+    boolean evaluateControllerStatesLocked(final JobStatus job) {
         for (int c = mControllers.size() - 1; c >= 0; --c) {
             final StateController sc = mControllers.get(c);
             sc.evaluateStateLocked(job);
         }
+        return job.isReady();
     }
 
     /**
diff --git a/services/tests/mockingservicestests/src/com/android/server/job/JobSchedulerServiceTest.java b/services/tests/mockingservicestests/src/com/android/server/job/JobSchedulerServiceTest.java
index 8f38f25b2f62d..9cd22dd292a56 100644
--- a/services/tests/mockingservicestests/src/com/android/server/job/JobSchedulerServiceTest.java
+++ b/services/tests/mockingservicestests/src/com/android/server/job/JobSchedulerServiceTest.java
@@ -1132,7 +1132,7 @@ public class JobSchedulerServiceTest {
     @Test
     public void testRareJobBatching() {
         spyOn(mService);
-        doNothing().when(mService).evaluateControllerStatesLocked(any());
+        doReturn(false).when(mService).evaluateControllerStatesLocked(any());
         doNothing().when(mService).noteJobsPending(any());
         doReturn(true).when(mService).isReadyToBeExecutedLocked(any(), anyBoolean());
         advanceElapsedClock(24 * HOUR_IN_MILLIS);