From 4e443ad09ca3d528bba69e579796a3ce3787c2a6 Mon Sep 17 00:00:00 2001 From: Kweku Adams Date: Wed, 10 May 2023 15:29:38 +0000 Subject: [PATCH] Remove demotion flags when a JobWorkItem is enqueued. Remove the demotion flags from a user-initiated job if the app successfully enqueues a new JobWorkItem. If an app successfully enqueues a JobWorkItem to a UI job, then the app is in a state to successfully schedule a UI job. Presumably, the user has asked for this additional bit of work, so it makes sense to remove the demotion flags. For now, we only do this for UI jobs since they have strict scheduling requirements. It's harder to assume other jobs were scheduled due to user interaction/request. This mimics the behavior when a UI job is rescheduled (without adding a JobWorkItem). Internal flags aren't copied over when a job is rescheduled. Bug: 281838527 Test: atest FrameworksMockingServicesTests:JobStatusTest Change-Id: I591afc7c0ac23fa0bda646fd12053be58ff5d4ee --- .../server/job/JobSchedulerService.java | 10 ++++ .../server/job/controllers/JobStatus.java | 4 ++ .../server/job/controllers/JobStatusTest.java | 49 +++++++++++++++++++ 3 files changed, 63 insertions(+) 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 72e66450d3394..3cf4fe5ad8488 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java +++ b/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java @@ -1500,6 +1500,16 @@ public class JobSchedulerService extends com.android.server.SystemService } toCancel.enqueueWorkLocked(work); + if (toCancel.getJob().isUserInitiated()) { + // The app is in a state to successfully schedule a UI job. Presumably, the + // user has asked for this additional bit of work, so remove any demotion + // flags. Only do this for UI jobs since they have strict scheduling + // requirements; it's harder to assume other jobs were scheduled due to + // user interaction/request. + toCancel.removeInternalFlags( + JobStatus.INTERNAL_FLAG_DEMOTED_BY_USER + | JobStatus.INTERNAL_FLAG_DEMOTED_BY_SYSTEM_UIJ); + } mJobs.touchJob(toCancel); sEnqueuedJwiHighWaterMarkLogger.logSampleWithUid(uId, toCancel.getWorkCount()); 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 6445c3bb6f8d8..b5d763c17e594 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 @@ -1177,6 +1177,10 @@ public final class JobStatus { mInternalFlags |= flags; } + public void removeInternalFlags(int flags) { + mInternalFlags = mInternalFlags & ~flags; + } + int getPreferredConstraintFlags() { return mPreferredConstraints; } diff --git a/services/tests/mockingservicestests/src/com/android/server/job/controllers/JobStatusTest.java b/services/tests/mockingservicestests/src/com/android/server/job/controllers/JobStatusTest.java index c040b1928ce41..05780ebe6c4be 100644 --- a/services/tests/mockingservicestests/src/com/android/server/job/controllers/JobStatusTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/job/controllers/JobStatusTest.java @@ -581,6 +581,49 @@ public class JobStatusTest { assertEquals(JobInfo.PRIORITY_HIGH, job.getEffectivePriority()); } + @Test + public void testModifyingInternalFlags() { + final JobInfo jobInfo = + new JobInfo.Builder(101, new ComponentName("foo", "bar")) + .setExpedited(true) + .build(); + JobStatus job = createJobStatus(jobInfo); + + assertEquals(0, job.getInternalFlags()); + + // Add single flag + job.addInternalFlags(JobStatus.INTERNAL_FLAG_HAS_FOREGROUND_EXEMPTION); + assertEquals(JobStatus.INTERNAL_FLAG_HAS_FOREGROUND_EXEMPTION, job.getInternalFlags()); + + // Add multiple flags + job.addInternalFlags(JobStatus.INTERNAL_FLAG_DEMOTED_BY_USER + | JobStatus.INTERNAL_FLAG_DEMOTED_BY_SYSTEM_UIJ); + assertEquals(JobStatus.INTERNAL_FLAG_HAS_FOREGROUND_EXEMPTION + | JobStatus.INTERNAL_FLAG_DEMOTED_BY_USER + | JobStatus.INTERNAL_FLAG_DEMOTED_BY_SYSTEM_UIJ, + job.getInternalFlags()); + + // Add flag that's already set + job.addInternalFlags(JobStatus.INTERNAL_FLAG_DEMOTED_BY_SYSTEM_UIJ); + assertEquals(JobStatus.INTERNAL_FLAG_HAS_FOREGROUND_EXEMPTION + | JobStatus.INTERNAL_FLAG_DEMOTED_BY_USER + | JobStatus.INTERNAL_FLAG_DEMOTED_BY_SYSTEM_UIJ, + job.getInternalFlags()); + + // Remove multiple + job.removeInternalFlags(JobStatus.INTERNAL_FLAG_HAS_FOREGROUND_EXEMPTION + | JobStatus.INTERNAL_FLAG_DEMOTED_BY_USER); + assertEquals(JobStatus.INTERNAL_FLAG_DEMOTED_BY_SYSTEM_UIJ, job.getInternalFlags()); + + // Remove one that isn't set + job.removeInternalFlags(JobStatus.INTERNAL_FLAG_DEMOTED_BY_USER); + assertEquals(JobStatus.INTERNAL_FLAG_DEMOTED_BY_SYSTEM_UIJ, job.getInternalFlags()); + + // Remove final flag. + job.removeInternalFlags(JobStatus.INTERNAL_FLAG_DEMOTED_BY_SYSTEM_UIJ); + assertEquals(0, job.getInternalFlags()); + } + @Test public void testShouldTreatAsUserInitiated() { JobInfo jobInfo = new JobInfo.Builder(101, new ComponentName("foo", "bar")) @@ -619,6 +662,9 @@ public class JobStatusTest { rescheduledJob = new JobStatus(job, NO_EARLIEST_RUNTIME, NO_LATEST_RUNTIME, 0, 0, 0, 0, 0); assertFalse(rescheduledJob.shouldTreatAsUserInitiatedJob()); + + rescheduledJob.removeInternalFlags(JobStatus.INTERNAL_FLAG_DEMOTED_BY_USER); + assertTrue(rescheduledJob.shouldTreatAsUserInitiatedJob()); } @Test @@ -641,6 +687,9 @@ public class JobStatusTest { rescheduledJob = new JobStatus(job, NO_EARLIEST_RUNTIME, NO_LATEST_RUNTIME, 0, 0, 0, 0, 0); assertFalse(rescheduledJob.shouldTreatAsUserInitiatedJob()); + + rescheduledJob.removeInternalFlags(JobStatus.INTERNAL_FLAG_DEMOTED_BY_SYSTEM_UIJ); + assertTrue(rescheduledJob.shouldTreatAsUserInitiatedJob()); } /**