From a8fa0615bde34da3a32f29517aaef2b256de50e7 Mon Sep 17 00:00:00 2001 From: Kweku Adams Date: Thu, 30 Apr 2020 10:44:13 -0700 Subject: [PATCH] Fix job readiness checking. Only mark dynamic constraints as satisfied if there are any dynamic constraints specified, otherwise, JS will bypass the quota check and always consider jobs without dynamic constraints as ready. Bug: 151274168 Test: atest CtsJobSchedulerTestCases Test: atest QuotaControllerTest Change-Id: Ie70c8b1ae138eb063af981ae2b248b1a0bdbdc60 --- .../server/job/controllers/JobStatus.java | 22 +++++++++---------- .../job/controllers/QuotaControllerTest.java | 9 +++++++- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/apex/jobscheduler/service/java/com/android/server/job/controllers/JobStatus.java b/apex/jobscheduler/service/java/com/android/server/job/controllers/JobStatus.java index d7c6ddbb0c63e..d7be2595e88ba 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/controllers/JobStatus.java +++ b/apex/jobscheduler/service/java/com/android/server/job/controllers/JobStatus.java @@ -472,7 +472,7 @@ public final class JobStatus { if (standbyBucket == RESTRICTED_INDEX) { addDynamicConstraints(DYNAMIC_RESTRICTED_CONSTRAINTS); } else { - mReadyDynamicSatisfied = true; + mReadyDynamicSatisfied = false; } mLastSuccessfulRunTime = lastSuccessfulRunTime; @@ -1132,8 +1132,8 @@ public final class JobStatus { } satisfiedConstraints = (satisfiedConstraints&~constraint) | (state ? constraint : 0); mSatisfiedConstraintsOfInterest = satisfiedConstraints & CONSTRAINTS_OF_INTEREST; - mReadyDynamicSatisfied = - mDynamicConstraints == (satisfiedConstraints & mDynamicConstraints); + mReadyDynamicSatisfied = mDynamicConstraints != 0 + && mDynamicConstraints == (satisfiedConstraints & mDynamicConstraints); if (STATS_LOG_ENABLED && (STATSD_CONSTRAINTS_TO_LOG & constraint) != 0) { FrameworkStatsLog.write_non_chained( FrameworkStatsLog.SCHEDULED_JOB_CONSTRAINT_CHANGED, @@ -1184,8 +1184,8 @@ public final class JobStatus { } mDynamicConstraints |= constraints; - mReadyDynamicSatisfied = - mDynamicConstraints == (satisfiedConstraints & mDynamicConstraints); + mReadyDynamicSatisfied = mDynamicConstraints != 0 + && mDynamicConstraints == (satisfiedConstraints & mDynamicConstraints); } /** @@ -1196,8 +1196,8 @@ public final class JobStatus { */ private void removeDynamicConstraints(int constraints) { mDynamicConstraints &= ~constraints; - mReadyDynamicSatisfied = - mDynamicConstraints == (satisfiedConstraints & mDynamicConstraints); + mReadyDynamicSatisfied = mDynamicConstraints != 0 + && mDynamicConstraints == (satisfiedConstraints & mDynamicConstraints); } public long getLastSuccessfulRunTime() { @@ -1241,8 +1241,8 @@ public final class JobStatus { break; default: satisfied |= constraint; - mReadyDynamicSatisfied = - mDynamicConstraints == (satisfied & mDynamicConstraints); + mReadyDynamicSatisfied = mDynamicConstraints != 0 + && mDynamicConstraints == (satisfied & mDynamicConstraints); break; } @@ -1262,8 +1262,8 @@ public final class JobStatus { mReadyWithinQuota = oldValue; break; default: - mReadyDynamicSatisfied = - mDynamicConstraints == (satisfiedConstraints & mDynamicConstraints); + mReadyDynamicSatisfied = mDynamicConstraints != 0 + && mDynamicConstraints == (satisfiedConstraints & mDynamicConstraints); break; } return toReturn; diff --git a/services/tests/mockingservicestests/src/com/android/server/job/controllers/QuotaControllerTest.java b/services/tests/mockingservicestests/src/com/android/server/job/controllers/QuotaControllerTest.java index c8ec7b5503d18..d8874e40a7609 100644 --- a/services/tests/mockingservicestests/src/com/android/server/job/controllers/QuotaControllerTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/job/controllers/QuotaControllerTest.java @@ -291,7 +291,6 @@ public class QuotaControllerTest { private JobStatus createJobStatus(String testTag, int jobId) { JobInfo jobInfo = new JobInfo.Builder(jobId, new ComponentName(mContext, "TestQuotaJobService")) - .setMinimumLatency(Math.abs(jobId) + 1) .build(); return createJobStatus(testTag, SOURCE_PACKAGE, CALLING_UID, jobInfo); } @@ -302,6 +301,9 @@ public class QuotaControllerTest { jobInfo, callingUid, packageName, SOURCE_USER_ID, testTag); // Make sure tests aren't passing just because the default bucket is likely ACTIVE. js.setStandbyBucket(FREQUENT_INDEX); + // Make sure Doze and background-not-restricted don't affect tests. + js.setDeviceNotDozingConstraintSatisfied(/* state */ true, /* whitelisted */false); + js.setBackgroundNotRestrictedConstraintSatisfied(true); return js; } @@ -649,6 +651,7 @@ public class QuotaControllerTest { assertEquals(expectedStats, inputStats); assertTrue(mQuotaController.isWithinQuotaLocked(SOURCE_USER_ID, SOURCE_PACKAGE, RARE_INDEX)); + assertTrue("Job not ready: " + jobStatus, jobStatus.isReady()); } // Add old session. Make sure values are combined correctly. @@ -689,6 +692,7 @@ public class QuotaControllerTest { assertEquals(expectedStats, inputStats); assertFalse( mQuotaController.isWithinQuotaLocked(SOURCE_USER_ID, SOURCE_PACKAGE, RARE_INDEX)); + assertFalse("Job unexpectedly ready: " + jobStatus, jobStatus.isReady()); } /** @@ -1325,6 +1329,7 @@ public class QuotaControllerTest { assertEquals(2, mQuotaController.getExecutionStatsLocked( SOURCE_USER_ID, SOURCE_PACKAGE, ACTIVE_INDEX).jobCountInRateLimitingWindow); assertTrue(mQuotaController.isWithinQuotaLocked(jobStatus)); + assertTrue(jobStatus.isReady()); } @Test @@ -1387,7 +1392,9 @@ public class QuotaControllerTest { mQuotaController.maybeStopTrackingJobLocked(unaffected, null, false); assertTrue(mQuotaController.isWithinQuotaLocked(unaffected)); + assertTrue(unaffected.isReady()); assertFalse(mQuotaController.isWithinQuotaLocked(fgStateChanger)); + assertFalse(fgStateChanger.isReady()); assertEquals(1, mQuotaController.getTimingSessions(SOURCE_USER_ID, unaffectedPkgName).size()); assertEquals(42,