From e8ef2d8dea78df8425f9806fbd76250fdb40cebd Mon Sep 17 00:00:00 2001 From: Kweku Adams Date: Thu, 31 Mar 2022 18:07:05 +0000 Subject: [PATCH] Make sure to stop EJs when quota is finished. If a job is started as an EJ, we should stop it once the EJ quota is finished, regardless of remaining regular quota. If we don't, then the job would get EJ privileges even though the app shouldn't be getting them anymore. Bug: 224533485 Test: atest CtsJobSchedulerTestCases:JobThrottlingTest#testRestrictingStopReason_ExpeditedQuota_noCharging Test: atest CtsJobSchedulerTestCases:JobThrottlingTest#testRestrictingStopReason_ExpeditedQuota_startOnCharging Test: atest CtsJobSchedulerTestCases Change-Id: Ib11023c4e3c05f2b3512bd069bda953fdd9bd0b9 --- .../server/job/controllers/JobStatus.java | 14 ++++++++++ .../job/controllers/QuotaController.java | 26 ++++++++++++++----- .../job/controllers/TareController.java | 6 +---- 3 files changed, 34 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 30fdb1e9ad4eb..91fa3c4fdc956 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 @@ -1311,8 +1311,15 @@ public final class JobStatus { if (mExpeditedQuotaApproved == state) { return false; } + final boolean wasReady = !state && isReady(); mExpeditedQuotaApproved = state; updateExpeditedDependencies(); + final boolean isReady = isReady(); + if (wasReady && !isReady) { + mReasonReadyToUnready = JobParameters.STOP_REASON_QUOTA; + } else if (!wasReady && isReady) { + mReasonReadyToUnready = JobParameters.STOP_REASON_UNDEFINED; + } return true; } @@ -1326,8 +1333,15 @@ public final class JobStatus { if (mExpeditedTareApproved == state) { return false; } + final boolean wasReady = !state && isReady(); mExpeditedTareApproved = state; updateExpeditedDependencies(); + final boolean isReady = isReady(); + if (wasReady && !isReady) { + mReasonReadyToUnready = JobParameters.STOP_REASON_QUOTA; + } else if (!wasReady && isReady) { + mReasonReadyToUnready = JobParameters.STOP_REASON_UNDEFINED; + } return true; } 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 fdcd2fc82ecc5..f26e051581f33 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 @@ -606,7 +606,7 @@ public final class QuotaController extends StateController { final boolean isWithinQuota = isWithinQuotaLocked(jobStatus); final boolean isWithinEJQuota = jobStatus.isRequestedExpeditedJob() && isWithinEJQuotaLocked(jobStatus); - setConstraintSatisfied(jobStatus, nowElapsed, isWithinQuota || isWithinEJQuota); + setConstraintSatisfied(jobStatus, nowElapsed, isWithinQuota, isWithinEJQuota); final boolean outOfEJQuota; if (jobStatus.isRequestedExpeditedJob()) { setExpeditedQuotaApproved(jobStatus, nowElapsed, isWithinEJQuota); @@ -1627,13 +1627,13 @@ 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. - if (setConstraintSatisfied(js, nowElapsed, isWithinEJQuota || realInQuota)) { + if (setConstraintSatisfied(js, nowElapsed, realInQuota, isWithinEJQuota)) { changedJobs.add(js); } } else { // This job is somehow exempted. Need to determine its own quota status. if (setConstraintSatisfied(js, nowElapsed, - isWithinEJQuota || isWithinQuotaLocked(js))) { + isWithinQuotaLocked(js), isWithinEJQuota)) { changedJobs.add(js); } } @@ -1676,7 +1676,7 @@ public final class QuotaController extends StateController { isWithinEJQuota = false; } if (setConstraintSatisfied(jobStatus, mUpdateTimeElapsed, - isWithinEJQuota || isWithinQuotaLocked(jobStatus))) { + isWithinQuotaLocked(jobStatus), isWithinEJQuota)) { changedJobs.add(jobStatus); } if (setExpeditedQuotaApproved(jobStatus, mUpdateTimeElapsed, isWithinEJQuota)) { @@ -1833,12 +1833,24 @@ public final class QuotaController extends StateController { } private boolean setConstraintSatisfied(@NonNull JobStatus jobStatus, long nowElapsed, - boolean isWithinQuota) { - if (!isWithinQuota && jobStatus.getWhenStandbyDeferred() == 0) { + boolean isWithinQuota, boolean isWithinEjQuota) { + final boolean isSatisfied; + if (jobStatus.startedAsExpeditedJob) { + // If the job started as an EJ, then we should only consider EJ quota for the constraint + // satisfaction. + isSatisfied = isWithinEjQuota; + } else if (mService.isCurrentlyRunningLocked(jobStatus)) { + // Job is running but didn't start as an EJ, so only the regular quota should be + // considered. + isSatisfied = isWithinQuota; + } else { + isSatisfied = isWithinEjQuota || isWithinQuota; + } + if (!isSatisfied && jobStatus.getWhenStandbyDeferred() == 0) { // Mark that the job is being deferred due to buckets. jobStatus.setWhenStandbyDeferred(nowElapsed); } - return jobStatus.setQuotaConstraintSatisfied(nowElapsed, isWithinQuota); + return jobStatus.setQuotaConstraintSatisfied(nowElapsed, isSatisfied); } /** diff --git a/apex/jobscheduler/service/java/com/android/server/job/controllers/TareController.java b/apex/jobscheduler/service/java/com/android/server/job/controllers/TareController.java index 223091a27ee10..a1a541f92b38a 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/controllers/TareController.java +++ b/apex/jobscheduler/service/java/com/android/server/job/controllers/TareController.java @@ -528,7 +528,7 @@ public class TareController extends StateController { @NonNull private ActionBill getRunningBill(JobStatus jobStatus) { // TODO: factor in network cost when available - if (jobStatus.shouldTreatAsExpeditedJob()) { + if (jobStatus.shouldTreatAsExpeditedJob() || jobStatus.startedAsExpeditedJob) { if (jobStatus.getEffectivePriority() == JobInfo.PRIORITY_MAX) { return BILL_JOB_RUNNING_MAX_EXPEDITED; } else { @@ -636,10 +636,6 @@ public class TareController extends StateController { return true; } if (mService.isCurrentlyRunningLocked(jobStatus)) { - if (jobStatus.isRequestedExpeditedJob()) { - return canAffordBillLocked(jobStatus, getRunningBill(jobStatus)) - || canAffordBillLocked(jobStatus, BILL_JOB_RUNNING_DEFAULT); - } return canAffordBillLocked(jobStatus, getRunningBill(jobStatus)); }