QuotaController tracking only jobs started in bg.
We don't want to penalize apps for starting jobs while in the foreground
so QuotaController now only tracks jobs that started while the uid
wasn't active.
Bug: 117846754
Bug: 111423978
Test: atest com.android.server.job.controllers.QuotaControllerTest
Change-Id: Icc36a361a466571af75b74c0cd9c976c6c321b43
diff --git a/services/core/java/com/android/server/job/controllers/QuotaController.java b/services/core/java/com/android/server/job/controllers/QuotaController.java
index f73ffac..660c238 100644
--- a/services/core/java/com/android/server/job/controllers/QuotaController.java
+++ b/services/core/java/com/android/server/job/controllers/QuotaController.java
@@ -151,8 +151,7 @@
return "<" + userId + ">" + packageName;
}
- @VisibleForTesting
- static final class Package {
+ private static final class Package {
public final String packageName;
public final int userId;
@@ -387,8 +386,9 @@
private boolean isWithinQuotaLocked(@NonNull final JobStatus jobStatus) {
final int standbyBucket = getEffectiveStandbyBucket(jobStatus);
- return isWithinQuotaLocked(jobStatus.getSourceUserId(), jobStatus.getSourcePackageName(),
- standbyBucket);
+ // Jobs for the active app should always be able to run.
+ return jobStatus.uidActive || isWithinQuotaLocked(
+ jobStatus.getSourceUserId(), jobStatus.getSourcePackageName(), standbyBucket);
}
private boolean isWithinQuotaLocked(final int userId, @NonNull final String packageName,
@@ -579,7 +579,10 @@
boolean changed = false;
for (int i = jobs.size() - 1; i >= 0; --i) {
final JobStatus js = jobs.valueAt(i);
- if (realStandbyBucket == getEffectiveStandbyBucket(js)) {
+ if (js.uidActive) {
+ // Jobs for the active app should always be able to run.
+ changed |= js.setQuotaConstraintSatisfied(true);
+ } else if (realStandbyBucket == getEffectiveStandbyBucket(js)) {
changed |= js.setQuotaConstraintSatisfied(realInQuota);
} else {
// This job is somehow exempted. Need to determine its own quota status.
@@ -765,18 +768,18 @@
public final long startTimeElapsed;
// End timestamp in elapsed realtime timebase.
public final long endTimeElapsed;
- // How many jobs ran during this session.
- public final int jobCount;
+ // How many background jobs ran during this session.
+ public final int bgJobCount;
TimingSession(long startElapsed, long endElapsed, int jobCount) {
this.startTimeElapsed = startElapsed;
this.endTimeElapsed = endElapsed;
- this.jobCount = jobCount;
+ this.bgJobCount = jobCount;
}
@Override
public String toString() {
- return "TimingSession{" + startTimeElapsed + "->" + endTimeElapsed + ", " + jobCount
+ return "TimingSession{" + startTimeElapsed + "->" + endTimeElapsed + ", " + bgJobCount
+ "}";
}
@@ -786,7 +789,7 @@
TimingSession other = (TimingSession) obj;
return startTimeElapsed == other.startTimeElapsed
&& endTimeElapsed == other.endTimeElapsed
- && jobCount == other.jobCount;
+ && bgJobCount == other.bgJobCount;
} else {
return false;
}
@@ -794,7 +797,7 @@
@Override
public int hashCode() {
- return Arrays.hashCode(new long[] {startTimeElapsed, endTimeElapsed, jobCount});
+ return Arrays.hashCode(new long[] {startTimeElapsed, endTimeElapsed, bgJobCount});
}
public void dump(IndentingPrintWriter pw) {
@@ -804,8 +807,8 @@
pw.print(" (");
pw.print(endTimeElapsed - startTimeElapsed);
pw.print("), ");
- pw.print(jobCount);
- pw.print(" jobs.");
+ pw.print(bgJobCount);
+ pw.print(" bg jobs.");
pw.println();
}
@@ -816,7 +819,8 @@
startTimeElapsed);
proto.write(StateControllerProto.QuotaController.TimingSession.END_TIME_ELAPSED,
endTimeElapsed);
- proto.write(StateControllerProto.QuotaController.TimingSession.JOB_COUNT, jobCount);
+ proto.write(StateControllerProto.QuotaController.TimingSession.BG_JOB_COUNT,
+ bgJobCount);
proto.end(token);
}
@@ -825,23 +829,32 @@
private final class Timer {
private final Package mPkg;
- // List of jobs currently running for this package.
- private final ArraySet<JobStatus> mRunningJobs = new ArraySet<>();
+ // List of jobs currently running for this app that started when the app wasn't in the
+ // foreground.
+ private final ArraySet<JobStatus> mRunningBgJobs = new ArraySet<>();
private long mStartTimeElapsed;
- private int mJobCount;
+ private int mBgJobCount;
Timer(int userId, String packageName) {
mPkg = new Package(userId, packageName);
}
void startTrackingJob(@NonNull JobStatus jobStatus) {
+ if (jobStatus.uidActive) {
+ // We intentionally don't pay attention to fg state changes after a job has started.
+ if (DEBUG) {
+ Slog.v(TAG,
+ "Timer ignoring " + jobStatus.toShortString() + " because uidActive");
+ }
+ return;
+ }
if (DEBUG) Slog.v(TAG, "Starting to track " + jobStatus.toShortString());
synchronized (mLock) {
// Always track jobs, even when charging.
- mRunningJobs.add(jobStatus);
+ mRunningBgJobs.add(jobStatus);
if (!mChargeTracker.isCharging()) {
- mJobCount++;
- if (mRunningJobs.size() == 1) {
+ mBgJobCount++;
+ if (mRunningBgJobs.size() == 1) {
// Started tracking the first job.
mStartTimeElapsed = sElapsedRealtimeClock.millis();
scheduleCutoff();
@@ -853,7 +866,7 @@
void stopTrackingJob(@NonNull JobStatus jobStatus) {
if (DEBUG) Slog.v(TAG, "Stopping tracking of " + jobStatus.toShortString());
synchronized (mLock) {
- if (mRunningJobs.size() == 0) {
+ if (mRunningBgJobs.size() == 0) {
// maybeStopTrackingJobLocked can be called when an app cancels a job, so a
// timer may not be running when it's asked to stop tracking a job.
if (DEBUG) {
@@ -861,8 +874,8 @@
}
return;
}
- mRunningJobs.remove(jobStatus);
- if (!mChargeTracker.isCharging() && mRunningJobs.size() == 0) {
+ if (mRunningBgJobs.remove(jobStatus)
+ && !mChargeTracker.isCharging() && mRunningBgJobs.size() == 0) {
emitSessionLocked(sElapsedRealtimeClock.millis());
cancelCutoff();
}
@@ -870,13 +883,13 @@
}
private void emitSessionLocked(long nowElapsed) {
- if (mJobCount <= 0) {
+ if (mBgJobCount <= 0) {
// Nothing to emit.
return;
}
- TimingSession ts = new TimingSession(mStartTimeElapsed, nowElapsed, mJobCount);
+ TimingSession ts = new TimingSession(mStartTimeElapsed, nowElapsed, mBgJobCount);
saveTimingSession(mPkg.userId, mPkg.packageName, ts);
- mJobCount = 0;
+ mBgJobCount = 0;
// Don't reset the tracked jobs list as we need to keep tracking the current number
// of jobs.
// However, cancel the currently scheduled cutoff since it's not currently useful.
@@ -889,7 +902,7 @@
*/
public boolean isActive() {
synchronized (mLock) {
- return mJobCount > 0;
+ return mBgJobCount > 0;
}
}
@@ -905,12 +918,12 @@
emitSessionLocked(nowElapsed);
} else {
// Start timing from unplug.
- if (mRunningJobs.size() > 0) {
+ if (mRunningBgJobs.size() > 0) {
mStartTimeElapsed = nowElapsed;
// NOTE: this does have the unfortunate consequence that if the device is
// repeatedly plugged in and unplugged, the job count for a package may be
// artificially high.
- mJobCount = mRunningJobs.size();
+ mBgJobCount = mRunningBgJobs.size();
// Schedule cutoff since we're now actively tracking for quotas again.
scheduleCutoff();
}
@@ -958,12 +971,12 @@
pw.print("NOT active");
}
pw.print(", ");
- pw.print(mJobCount);
- pw.print(" running jobs");
+ pw.print(mBgJobCount);
+ pw.print(" running bg jobs");
pw.println();
pw.increaseIndent();
- for (int i = 0; i < mRunningJobs.size(); i++) {
- JobStatus js = mRunningJobs.valueAt(i);
+ for (int i = 0; i < mRunningBgJobs.size(); i++) {
+ JobStatus js = mRunningBgJobs.valueAt(i);
if (predicate.test(js)) {
pw.println(js.toShortString());
}
@@ -979,9 +992,9 @@
proto.write(StateControllerProto.QuotaController.Timer.IS_ACTIVE, isActive());
proto.write(StateControllerProto.QuotaController.Timer.START_TIME_ELAPSED,
mStartTimeElapsed);
- proto.write(StateControllerProto.QuotaController.Timer.JOB_COUNT, mJobCount);
- for (int i = 0; i < mRunningJobs.size(); i++) {
- JobStatus js = mRunningJobs.valueAt(i);
+ proto.write(StateControllerProto.QuotaController.Timer.BG_JOB_COUNT, mBgJobCount);
+ for (int i = 0; i < mRunningBgJobs.size(); i++) {
+ JobStatus js = mRunningBgJobs.valueAt(i);
if (predicate.test(js)) {
js.writeToShortProto(proto,
StateControllerProto.QuotaController.Timer.RUNNING_JOBS);
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 b2ec835..95da13f 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
@@ -775,6 +775,139 @@
assertEquals(expected, mQuotaController.getTimingSessions(SOURCE_USER_ID, SOURCE_PACKAGE));
}
+ /** Tests that TimingSessions are saved properly when all the jobs are background jobs. */
+ @Test
+ public void testTimerTracking_AllBackground() {
+ setDischarging();
+
+ JobStatus jobStatus = createJobStatus("testTimerTracking_AllBackground", 1);
+ mQuotaController.maybeStartTrackingJobLocked(jobStatus, null);
+
+ assertNull(mQuotaController.getTimingSessions(SOURCE_USER_ID, SOURCE_PACKAGE));
+
+ List<TimingSession> expected = new ArrayList<>();
+
+ // Test single job.
+ long start = JobSchedulerService.sElapsedRealtimeClock.millis();
+ mQuotaController.prepareForExecutionLocked(jobStatus);
+ advanceElapsedClock(5 * SECOND_IN_MILLIS);
+ mQuotaController.maybeStopTrackingJobLocked(jobStatus, null, false);
+ expected.add(createTimingSession(start, 5 * SECOND_IN_MILLIS, 1));
+ assertEquals(expected, mQuotaController.getTimingSessions(SOURCE_USER_ID, SOURCE_PACKAGE));
+
+ // Test overlapping jobs.
+ JobStatus jobStatus2 = createJobStatus("testTimerTracking_AllBackground", 2);
+ mQuotaController.maybeStartTrackingJobLocked(jobStatus2, null);
+
+ JobStatus jobStatus3 = createJobStatus("testTimerTracking_AllBackground", 3);
+ mQuotaController.maybeStartTrackingJobLocked(jobStatus3, null);
+
+ advanceElapsedClock(SECOND_IN_MILLIS);
+
+ start = JobSchedulerService.sElapsedRealtimeClock.millis();
+ mQuotaController.maybeStartTrackingJobLocked(jobStatus, null);
+ mQuotaController.prepareForExecutionLocked(jobStatus);
+ advanceElapsedClock(10 * SECOND_IN_MILLIS);
+ mQuotaController.prepareForExecutionLocked(jobStatus2);
+ advanceElapsedClock(10 * SECOND_IN_MILLIS);
+ mQuotaController.maybeStopTrackingJobLocked(jobStatus, null, false);
+ advanceElapsedClock(10 * SECOND_IN_MILLIS);
+ mQuotaController.prepareForExecutionLocked(jobStatus3);
+ advanceElapsedClock(20 * SECOND_IN_MILLIS);
+ mQuotaController.maybeStopTrackingJobLocked(jobStatus3, null, false);
+ advanceElapsedClock(10 * SECOND_IN_MILLIS);
+ mQuotaController.maybeStopTrackingJobLocked(jobStatus2, null, false);
+ expected.add(createTimingSession(start, MINUTE_IN_MILLIS, 3));
+ assertEquals(expected, mQuotaController.getTimingSessions(SOURCE_USER_ID, SOURCE_PACKAGE));
+ }
+
+ /** Tests that Timers don't count foreground jobs. */
+ @Test
+ public void testTimerTracking_AllForeground() {
+ setDischarging();
+
+ JobStatus jobStatus = createJobStatus("testTimerTracking_AllForeground", 1);
+ jobStatus.uidActive = true;
+ mQuotaController.maybeStartTrackingJobLocked(jobStatus, null);
+
+ assertNull(mQuotaController.getTimingSessions(SOURCE_USER_ID, SOURCE_PACKAGE));
+
+ mQuotaController.prepareForExecutionLocked(jobStatus);
+ advanceElapsedClock(5 * SECOND_IN_MILLIS);
+ mQuotaController.maybeStopTrackingJobLocked(jobStatus, null, false);
+ assertNull(mQuotaController.getTimingSessions(SOURCE_USER_ID, SOURCE_PACKAGE));
+ }
+
+ /**
+ * Tests that Timers properly track overlapping foreground and background jobs.
+ */
+ @Test
+ public void testTimerTracking_ForegroundAndBackground() {
+ setDischarging();
+
+ JobStatus jobBg1 = createJobStatus("testTimerTracking_ForegroundAndBackground", 1);
+ JobStatus jobBg2 = createJobStatus("testTimerTracking_ForegroundAndBackground", 2);
+ JobStatus jobFg3 = createJobStatus("testTimerTracking_ForegroundAndBackground", 3);
+ jobFg3.uidActive = true;
+ mQuotaController.maybeStartTrackingJobLocked(jobBg1, null);
+ mQuotaController.maybeStartTrackingJobLocked(jobBg2, null);
+ mQuotaController.maybeStartTrackingJobLocked(jobFg3, null);
+ assertNull(mQuotaController.getTimingSessions(SOURCE_USER_ID, SOURCE_PACKAGE));
+ List<TimingSession> expected = new ArrayList<>();
+
+ // UID starts out inactive.
+ long start = JobSchedulerService.sElapsedRealtimeClock.millis();
+ mQuotaController.prepareForExecutionLocked(jobBg1);
+ advanceElapsedClock(10 * SECOND_IN_MILLIS);
+ mQuotaController.maybeStopTrackingJobLocked(jobBg1, jobBg1, true);
+ expected.add(createTimingSession(start, 10 * SECOND_IN_MILLIS, 1));
+ assertEquals(expected, mQuotaController.getTimingSessions(SOURCE_USER_ID, SOURCE_PACKAGE));
+
+ advanceElapsedClock(SECOND_IN_MILLIS);
+
+ // Bg job starts while inactive, spans an entire active session, and ends after the
+ // active session.
+ // Fg job starts after the bg job and ends before the bg job.
+ // Entire bg job duration should be counted since it started before active session. However,
+ // count should only be 1 since Timer shouldn't count fg jobs.
+ start = JobSchedulerService.sElapsedRealtimeClock.millis();
+ mQuotaController.maybeStartTrackingJobLocked(jobBg2, null);
+ mQuotaController.prepareForExecutionLocked(jobBg2);
+ advanceElapsedClock(10 * SECOND_IN_MILLIS);
+ mQuotaController.prepareForExecutionLocked(jobFg3);
+ advanceElapsedClock(10 * SECOND_IN_MILLIS);
+ mQuotaController.maybeStopTrackingJobLocked(jobFg3, null, false);
+ advanceElapsedClock(10 * SECOND_IN_MILLIS);
+ mQuotaController.maybeStopTrackingJobLocked(jobBg2, null, false);
+ expected.add(createTimingSession(start, 30 * SECOND_IN_MILLIS, 1));
+ assertEquals(expected, mQuotaController.getTimingSessions(SOURCE_USER_ID, SOURCE_PACKAGE));
+
+ advanceElapsedClock(SECOND_IN_MILLIS);
+
+ // Bg job 1 starts, then fg job starts. Bg job 1 job ends. Shortly after, uid goes
+ // "inactive" and then bg job 2 starts. Then fg job ends.
+ // This should result in two TimingSessions with a count of one each.
+ start = JobSchedulerService.sElapsedRealtimeClock.millis();
+ mQuotaController.maybeStartTrackingJobLocked(jobBg1, null);
+ mQuotaController.maybeStartTrackingJobLocked(jobBg2, null);
+ mQuotaController.maybeStartTrackingJobLocked(jobFg3, null);
+ mQuotaController.prepareForExecutionLocked(jobBg1);
+ advanceElapsedClock(10 * SECOND_IN_MILLIS);
+ mQuotaController.prepareForExecutionLocked(jobFg3);
+ advanceElapsedClock(10 * SECOND_IN_MILLIS);
+ mQuotaController.maybeStopTrackingJobLocked(jobBg1, jobBg1, true);
+ expected.add(createTimingSession(start, 20 * SECOND_IN_MILLIS, 1));
+ advanceElapsedClock(10 * SECOND_IN_MILLIS); // UID "inactive" now
+ start = JobSchedulerService.sElapsedRealtimeClock.millis();
+ mQuotaController.prepareForExecutionLocked(jobBg2);
+ advanceElapsedClock(10 * SECOND_IN_MILLIS);
+ mQuotaController.maybeStopTrackingJobLocked(jobFg3, null, false);
+ advanceElapsedClock(10 * SECOND_IN_MILLIS);
+ mQuotaController.maybeStopTrackingJobLocked(jobBg2, null, false);
+ expected.add(createTimingSession(start, 20 * SECOND_IN_MILLIS, 1));
+ assertEquals(expected, mQuotaController.getTimingSessions(SOURCE_USER_ID, SOURCE_PACKAGE));
+ }
+
/**
* Tests that a job is properly updated and JobSchedulerService is notified when a job reaches
* its quota.