From 985612bb406646a1322b7142e100c369bb6bfddd Mon Sep 17 00:00:00 2001 From: Kweku Adams Date: Thu, 22 Jul 2021 15:49:28 -0700 Subject: [PATCH] Rework expedited status. Change the OK for a job to be treated as expedited from a "constraint" to a boolean. This will make it a little nicer to add different ways of approving a job to be treated as expedited. Bug: 158300259 Test: atest frameworks/base/services/tests/mockingservicestests/src/com/android/server/job Test: atest frameworks/base/services/tests/servicestests/src/com/android/server/job Test: atest CtsJobSchedulerTestCases Change-Id: Id1c85b0bbdd7201ab325b0ef71f764c578869fd7 --- .../server/job/controllers/JobStatus.java | 68 +++++++++---------- .../job/controllers/QuotaController.java | 47 ++++++------- .../job/controllers/QuotaControllerTest.java | 36 +++++----- 3 files changed, 74 insertions(+), 77 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 fda57f72eb08c..76836e40c36ef 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 @@ -92,7 +92,6 @@ public final class JobStatus { static final int CONSTRAINT_CONTENT_TRIGGER = 1<<26; static final int CONSTRAINT_DEVICE_NOT_DOZING = 1 << 25; // Implicit constraint static final int CONSTRAINT_WITHIN_QUOTA = 1 << 24; // Implicit constraint - static final int CONSTRAINT_WITHIN_EXPEDITED_QUOTA = 1 << 23; // Implicit constraint static final int CONSTRAINT_BACKGROUND_NOT_RESTRICTED = 1 << 22; // Implicit constraint // The following set of dynamic constraints are for specific use cases (as explained in their @@ -148,8 +147,7 @@ public final class JobStatus { | CONSTRAINT_DEADLINE | CONSTRAINT_IDLE | CONSTRAINT_TIMING_DELAY - | CONSTRAINT_WITHIN_QUOTA - | CONSTRAINT_WITHIN_EXPEDITED_QUOTA; + | CONSTRAINT_WITHIN_QUOTA; // TODO(b/129954980) private static final boolean STATS_LOG_ENABLED = false; @@ -393,6 +391,11 @@ public final class JobStatus { private long mTotalNetworkUploadBytes = JobInfo.NETWORK_BYTES_UNKNOWN; private long mMinimumNetworkChunkBytes = JobInfo.NETWORK_BYTES_UNKNOWN; + /** + * Whether or not this job is approved to be treated as expedited per quota policy. + */ + private boolean mExpeditedQuotaApproved; + /////// Booleans that track if a job is ready to run. They should be updated whenever dependent /////// states change. @@ -418,9 +421,6 @@ public final class JobStatus { /** The job is within its quota based on its standby bucket. */ private boolean mReadyWithinQuota; - /** The job is an expedited job with sufficient quota to run as an expedited job. */ - private boolean mReadyWithinExpeditedQuota; - /** The job's dynamic requirements have been satisfied. */ private boolean mReadyDynamicSatisfied; @@ -1132,7 +1132,7 @@ public final class JobStatus { * treated as an expedited job. */ public boolean shouldTreatAsExpeditedJob() { - return mReadyWithinExpeditedQuota && isRequestedExpeditedJob(); + return mExpeditedQuotaApproved && isRequestedExpeditedJob(); } /** @@ -1230,19 +1230,27 @@ public final class JobStatus { return false; } - /** @return true if the constraint was changed, false otherwise. */ - boolean setExpeditedJobQuotaConstraintSatisfied(final long nowElapsed, boolean state) { - if (setConstraintSatisfied(CONSTRAINT_WITHIN_EXPEDITED_QUOTA, nowElapsed, state)) { - // The constraint was changed. Update the ready flag. - mReadyWithinExpeditedQuota = state; - // DeviceIdleJobsController currently only tracks jobs with the WILL_BE_FOREGROUND flag. - // Making it also track requested-expedited jobs would add unnecessary hops since the - // controller would then defer to canRunInDoze. Avoid the hops and just update - // mReadyNotDozing directly. - mReadyNotDozing = isConstraintSatisfied(CONSTRAINT_DEVICE_NOT_DOZING) || canRunInDoze(); - return true; + /** + * Sets whether or not this job is approved to be treated as an expedited job based on quota + * policy. + * + * @return true if the approval bit was changed, false otherwise. + */ + boolean setExpeditedJobQuotaApproved(final long nowElapsed, boolean state) { + if (mExpeditedQuotaApproved == state) { + return false; } - return false; + mExpeditedQuotaApproved = state; + updateExpeditedDependencies(); + return true; + } + + private void updateExpeditedDependencies() { + // DeviceIdleJobsController currently only tracks jobs with the WILL_BE_FOREGROUND flag. + // Making it also track requested-expedited jobs would add unnecessary hops since the + // controller would then defer to canRunInDoze. Avoid the hops and just update + // mReadyNotDozing directly. + mReadyNotDozing = isConstraintSatisfied(CONSTRAINT_DEVICE_NOT_DOZING) || canRunInDoze(); } /** @return true if the state was changed, false otherwise. */ @@ -1346,7 +1354,6 @@ public final class JobStatus { return JobParameters.STOP_REASON_DEVICE_STATE; case CONSTRAINT_WITHIN_QUOTA: - case CONSTRAINT_WITHIN_EXPEDITED_QUOTA: return JobParameters.STOP_REASON_QUOTA; // These should never be stop reasons since they can never go from true to false. @@ -1363,6 +1370,10 @@ public final class JobStatus { return (satisfiedConstraints&constraint) != 0; } + boolean isExpeditedQuotaApproved() { + return mExpeditedQuotaApproved; + } + boolean clearTrackingController(int which) { if ((trackingControllers&which) != 0) { trackingControllers &= ~which; @@ -1465,10 +1476,6 @@ public final class JobStatus { oldValue = mReadyWithinQuota; mReadyWithinQuota = value; break; - case CONSTRAINT_WITHIN_EXPEDITED_QUOTA: - oldValue = mReadyWithinExpeditedQuota; - mReadyWithinExpeditedQuota = value; - break; default: if (value) { satisfied |= constraint; @@ -1496,9 +1503,6 @@ public final class JobStatus { case CONSTRAINT_WITHIN_QUOTA: mReadyWithinQuota = oldValue; break; - case CONSTRAINT_WITHIN_EXPEDITED_QUOTA: - mReadyWithinExpeditedQuota = oldValue; - break; default: mReadyDynamicSatisfied = mDynamicConstraints != 0 && mDynamicConstraints == (satisfiedConstraints & mDynamicConstraints); @@ -1726,9 +1730,6 @@ public final class JobStatus { if ((constraints & CONSTRAINT_WITHIN_QUOTA) != 0) { pw.print(" WITHIN_QUOTA"); } - if ((constraints & CONSTRAINT_WITHIN_EXPEDITED_QUOTA) != 0) { - pw.print(" WITHIN_EXPEDITED_QUOTA"); - } if (constraints != 0) { pw.print(" [0x"); pw.print(Integer.toHexString(constraints)); @@ -1801,9 +1802,6 @@ public final class JobStatus { if ((constraints & CONSTRAINT_BACKGROUND_NOT_RESTRICTED) != 0) { proto.write(fieldId, JobServerProtoEnums.CONSTRAINT_BACKGROUND_NOT_RESTRICTED); } - if ((constraints & CONSTRAINT_WITHIN_EXPEDITED_QUOTA) != 0) { - proto.write(fieldId, JobServerProtoEnums.CONSTRAINT_WITHIN_EXPEDITED_JOB_QUOTA); - } } private void dumpJobWorkItem(IndentingPrintWriter pw, JobWorkItem work, int index) { @@ -2050,8 +2048,8 @@ public final class JobStatus { pw.print("readyComponentEnabled: "); pw.println(serviceInfo != null); if ((getFlags() & JobInfo.FLAG_EXPEDITED) != 0) { - pw.print("readyWithinExpeditedQuota: "); - pw.print(mReadyWithinExpeditedQuota); + pw.print("expeditedQuotaApproved: "); + pw.print(mExpeditedQuotaApproved); pw.print(" (started as EJ: "); pw.print(startedAsExpeditedJob); pw.println(")"); diff --git a/apex/jobscheduler/service/java/com/android/server/job/controllers/QuotaController.java b/apex/jobscheduler/service/java/com/android/server/job/controllers/QuotaController.java index f7181a4d00d7a..36afac8fec78f 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/controllers/QuotaController.java +++ b/apex/jobscheduler/service/java/com/android/server/job/controllers/QuotaController.java @@ -634,11 +634,12 @@ public final class QuotaController extends StateController { jobs.add(jobStatus); jobStatus.setTrackingController(JobStatus.TRACKING_QUOTA); final boolean isWithinQuota = isWithinQuotaLocked(jobStatus); - setConstraintSatisfied(jobStatus, nowElapsed, isWithinQuota); + final boolean isWithinEJQuota = + jobStatus.isRequestedExpeditedJob() && isWithinEJQuotaLocked(jobStatus); + setConstraintSatisfied(jobStatus, nowElapsed, isWithinQuota || isWithinEJQuota); final boolean outOfEJQuota; if (jobStatus.isRequestedExpeditedJob()) { - final boolean isWithinEJQuota = isWithinEJQuotaLocked(jobStatus); - setExpeditedConstraintSatisfied(jobStatus, nowElapsed, isWithinEJQuota); + setExpeditedQuotaApproved(jobStatus, nowElapsed, isWithinEJQuota); outOfEJQuota = !isWithinEJQuota; } else { outOfEJQuota = false; @@ -1614,6 +1615,8 @@ public final class QuotaController extends StateController { boolean changed = false; for (int i = jobs.size() - 1; i >= 0; --i) { final JobStatus js = jobs.valueAt(i); + final boolean isWithinEJQuota = + js.isRequestedExpeditedJob() && isWithinEJQuotaLocked(js); if (isTopStartedJobLocked(js)) { // Job was started while the app was in the TOP state so we should allow it to // finish. @@ -1623,15 +1626,15 @@ public final class QuotaController extends StateController { // An app in the ACTIVE bucket may be out of quota while the job could be in quota // for some reason. Therefore, avoid setting the real value here and check each job // individually. - changed |= setConstraintSatisfied(js, nowElapsed, realInQuota); + changed |= setConstraintSatisfied(js, nowElapsed, isWithinEJQuota || realInQuota); } else { // This job is somehow exempted. Need to determine its own quota status. - changed |= setConstraintSatisfied(js, nowElapsed, isWithinQuotaLocked(js)); + changed |= setConstraintSatisfied(js, nowElapsed, + isWithinEJQuota || isWithinQuotaLocked(js)); } if (js.isRequestedExpeditedJob()) { - boolean isWithinEJQuota = isWithinEJQuotaLocked(js); - changed |= setExpeditedConstraintSatisfied(js, nowElapsed, isWithinEJQuota); + changed |= setExpeditedQuotaApproved(js, nowElapsed, isWithinEJQuota); outOfEJQuota |= !isWithinEJQuota; } } @@ -1659,26 +1662,24 @@ public final class QuotaController extends StateController { @Override public void accept(JobStatus jobStatus) { - if (setConstraintSatisfied( - jobStatus, mUpdateTimeElapsed, isWithinQuotaLocked(jobStatus))) { + final boolean isWithinEJQuota; + if (jobStatus.isRequestedExpeditedJob()) { + isWithinEJQuota = isWithinEJQuotaLocked(jobStatus); + } else { + isWithinEJQuota = false; + } + if (setConstraintSatisfied(jobStatus, mUpdateTimeElapsed, + isWithinEJQuota || isWithinQuotaLocked(jobStatus))) { changedJobs.add(jobStatus); } - final boolean outOfEJQuota; - if (jobStatus.isRequestedExpeditedJob()) { - final boolean isWithinEJQuota = isWithinEJQuotaLocked(jobStatus); - if (setExpeditedConstraintSatisfied( - jobStatus, mUpdateTimeElapsed, isWithinEJQuota)) { - changedJobs.add(jobStatus); - } - outOfEJQuota = !isWithinEJQuota; - } else { - outOfEJQuota = false; + if (setExpeditedQuotaApproved(jobStatus, mUpdateTimeElapsed, isWithinEJQuota)) { + changedJobs.add(jobStatus); } final int userId = jobStatus.getSourceUserId(); final String packageName = jobStatus.getSourcePackageName(); final int realStandbyBucket = jobStatus.getStandbyBucket(); - if (isWithinQuotaLocked(userId, packageName, realStandbyBucket) && !outOfEJQuota) { + if (isWithinQuotaLocked(userId, packageName, realStandbyBucket) && isWithinEJQuota) { // TODO(141645789): we probably shouldn't cancel the alarm until we've verified // that all jobs for the userId-package are within quota. mInQuotaAlarmListener.removeAlarmLocked(userId, packageName); @@ -1827,9 +1828,9 @@ public final class QuotaController extends StateController { * If the satisfaction changes, this will tell connectivity & background jobs controller to * also re-evaluate their state. */ - private boolean setExpeditedConstraintSatisfied(@NonNull JobStatus jobStatus, long nowElapsed, + private boolean setExpeditedQuotaApproved(@NonNull JobStatus jobStatus, long nowElapsed, boolean isWithinQuota) { - if (jobStatus.setExpeditedJobQuotaConstraintSatisfied(nowElapsed, isWithinQuota)) { + if (jobStatus.setExpeditedJobQuotaApproved(nowElapsed, isWithinQuota)) { mBackgroundJobsController.evaluateStateLocked(jobStatus); mConnectivityController.evaluateStateLocked(jobStatus); if (isWithinQuota && jobStatus.isReady()) { @@ -4330,7 +4331,7 @@ public final class QuotaController extends StateController { js.isRequestedExpeditedJob()); proto.write( StateControllerProto.QuotaController.TrackedJob.IS_WITHIN_FG_JOB_QUOTA, - js.isConstraintSatisfied(JobStatus.CONSTRAINT_WITHIN_EXPEDITED_QUOTA)); + js.isExpeditedQuotaApproved()); proto.end(jsToken); } }); 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 906e6d208a795..dd70c874eb866 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 @@ -5417,10 +5417,9 @@ public class QuotaControllerTest { timeout(remainingTimeMs / 2 + 2 * SECOND_IN_MILLIS).times(1)) .onControllerStateChanged(any()); // Top should still be "in quota" since it started before the app ran on top out of quota. - assertFalse(jobBg.isConstraintSatisfied(JobStatus.CONSTRAINT_WITHIN_EXPEDITED_QUOTA)); - assertTrue(jobTop.isConstraintSatisfied(JobStatus.CONSTRAINT_WITHIN_EXPEDITED_QUOTA)); - assertFalse( - jobUnstarted.isConstraintSatisfied(JobStatus.CONSTRAINT_WITHIN_EXPEDITED_QUOTA)); + assertFalse(jobBg.isExpeditedQuotaApproved()); + assertTrue(jobTop.isExpeditedQuotaApproved()); + assertFalse(jobUnstarted.isExpeditedQuotaApproved()); synchronized (mQuotaController.mLock) { assertTrue( 0 >= mQuotaController @@ -5445,18 +5444,18 @@ public class QuotaControllerTest { synchronized (mQuotaController.mLock) { mQuotaController.prepareForExecutionLocked(jobTop2); } - assertTrue(jobTop2.isConstraintSatisfied(JobStatus.CONSTRAINT_WITHIN_EXPEDITED_QUOTA)); - assertTrue(jobFg.isConstraintSatisfied(JobStatus.CONSTRAINT_WITHIN_EXPEDITED_QUOTA)); - assertTrue(jobBg.isConstraintSatisfied(JobStatus.CONSTRAINT_WITHIN_EXPEDITED_QUOTA)); - assertTrue(jobUnstarted.isConstraintSatisfied(JobStatus.CONSTRAINT_WITHIN_EXPEDITED_QUOTA)); + assertTrue(jobTop2.isExpeditedQuotaApproved()); + assertTrue(jobFg.isExpeditedQuotaApproved()); + assertTrue(jobBg.isExpeditedQuotaApproved()); + assertTrue(jobUnstarted.isExpeditedQuotaApproved()); // App still in foreground so everything should be in quota. advanceElapsedClock(20 * SECOND_IN_MILLIS); setProcessState(ActivityManager.PROCESS_STATE_FOREGROUND_SERVICE); - assertTrue(jobTop2.isConstraintSatisfied(JobStatus.CONSTRAINT_WITHIN_EXPEDITED_QUOTA)); - assertTrue(jobFg.isConstraintSatisfied(JobStatus.CONSTRAINT_WITHIN_EXPEDITED_QUOTA)); - assertTrue(jobBg.isConstraintSatisfied(JobStatus.CONSTRAINT_WITHIN_EXPEDITED_QUOTA)); - assertTrue(jobUnstarted.isConstraintSatisfied(JobStatus.CONSTRAINT_WITHIN_EXPEDITED_QUOTA)); + assertTrue(jobTop2.isExpeditedQuotaApproved()); + assertTrue(jobFg.isExpeditedQuotaApproved()); + assertTrue(jobBg.isExpeditedQuotaApproved()); + assertTrue(jobUnstarted.isExpeditedQuotaApproved()); advanceElapsedClock(20 * SECOND_IN_MILLIS); setProcessState(ActivityManager.PROCESS_STATE_SERVICE); @@ -5464,13 +5463,12 @@ public class QuotaControllerTest { .onControllerStateChanged(any()); // App is now in background and out of quota. Fg should now change to out of quota since it // wasn't started. Top should remain in quota since it started when the app was in TOP. - assertTrue(jobTop2.isConstraintSatisfied(JobStatus.CONSTRAINT_WITHIN_EXPEDITED_QUOTA)); - assertFalse(jobFg.isConstraintSatisfied(JobStatus.CONSTRAINT_WITHIN_EXPEDITED_QUOTA)); - assertFalse(jobBg.isConstraintSatisfied(JobStatus.CONSTRAINT_WITHIN_EXPEDITED_QUOTA)); + assertTrue(jobTop2.isExpeditedQuotaApproved()); + assertFalse(jobFg.isExpeditedQuotaApproved()); + assertFalse(jobBg.isExpeditedQuotaApproved()); trackJobs(jobBg2); - assertFalse(jobBg2.isConstraintSatisfied(JobStatus.CONSTRAINT_WITHIN_EXPEDITED_QUOTA)); - assertFalse( - jobUnstarted.isConstraintSatisfied(JobStatus.CONSTRAINT_WITHIN_EXPEDITED_QUOTA)); + assertFalse(jobBg2.isExpeditedQuotaApproved()); + assertFalse(jobUnstarted.isExpeditedQuotaApproved()); synchronized (mQuotaController.mLock) { assertTrue( 0 >= mQuotaController @@ -5602,7 +5600,7 @@ public class QuotaControllerTest { verify(mJobSchedulerService, timeout(remainingTimeMs + 2 * SECOND_IN_MILLIS).times(0)) .onControllerStateChanged(any()); - assertTrue(jobStatus.isConstraintSatisfied(JobStatus.CONSTRAINT_WITHIN_EXPEDITED_QUOTA)); + assertTrue(jobStatus.isExpeditedQuotaApproved()); // The job used up the remaining quota, but in that time, the same amount of time in the // old TimingSession also fell out of the quota window, so it should still have the same // amount of remaining time left its quota.