From 959db93d6a6ae170f1b6cfc3d7e61bdcc56d8a76 Mon Sep 17 00:00:00 2001 From: Kweku Adams Date: Thu, 22 Dec 2022 19:43:28 +0000 Subject: [PATCH] Don't persist dynamic constraints. Dynamic constraints are only placed on jobs in certain situations and shouldn't be persisted to override the underlying job's actual requested constraints. Bug: 141645789 Test: atest FrameworksServicesTests:JobStoreTest Change-Id: I9ba5f80685f98677fe63f4814392da23c0c84d62 --- .../java/com/android/server/job/JobStore.java | 10 +++---- .../server/job/controllers/JobStatus.java | 17 +++++++---- .../com/android/server/job/JobStoreTest.java | 29 +++++++++++++++++++ 3 files changed, 46 insertions(+), 10 deletions(-) diff --git a/apex/jobscheduler/service/java/com/android/server/job/JobStore.java b/apex/jobscheduler/service/java/com/android/server/job/JobStore.java index 8827049497077..53c56e790831e 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/JobStore.java +++ b/apex/jobscheduler/service/java/com/android/server/job/JobStore.java @@ -832,8 +832,8 @@ public final class JobStore { private void writeConstraintsToXml(TypedXmlSerializer out, JobStatus jobStatus) throws IOException { out.startTag(null, XML_TAG_PARAMS_CONSTRAINTS); + final JobInfo job = jobStatus.getJob(); if (jobStatus.hasConnectivityConstraint()) { - final JobInfo job = jobStatus.getJob(); final NetworkRequest network = jobStatus.getJob().getRequiredNetwork(); out.attribute(null, "net-capabilities-csv", intArrayToString( network.getCapabilities())); @@ -854,16 +854,16 @@ public final class JobStore { job.getMinimumNetworkChunkBytes()); } } - if (jobStatus.hasIdleConstraint()) { + if (job.isRequireDeviceIdle()) { out.attribute(null, "idle", Boolean.toString(true)); } - if (jobStatus.hasChargingConstraint()) { + if (job.isRequireCharging()) { out.attribute(null, "charging", Boolean.toString(true)); } - if (jobStatus.hasBatteryNotLowConstraint()) { + if (job.isRequireBatteryNotLow()) { out.attribute(null, "battery-not-low", Boolean.toString(true)); } - if (jobStatus.hasStorageNotLowConstraint()) { + if (job.isRequireStorageNotLow()) { out.attribute(null, "storage-not-low", Boolean.toString(true)); } out.endTag(null, XML_TAG_PARAMS_CONSTRAINTS); 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 b0b6a01f62819..0b875ccffb212 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 @@ -96,10 +96,16 @@ public final class JobStatus { public static final long NO_LATEST_RUNTIME = Long.MAX_VALUE; public static final long NO_EARLIEST_RUNTIME = 0L; - static final int CONSTRAINT_CHARGING = JobInfo.CONSTRAINT_FLAG_CHARGING; // 1 < 0 - static final int CONSTRAINT_IDLE = JobInfo.CONSTRAINT_FLAG_DEVICE_IDLE; // 1 << 2 - static final int CONSTRAINT_BATTERY_NOT_LOW = JobInfo.CONSTRAINT_FLAG_BATTERY_NOT_LOW; // 1 << 1 - static final int CONSTRAINT_STORAGE_NOT_LOW = JobInfo.CONSTRAINT_FLAG_STORAGE_NOT_LOW; // 1 << 3 + @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE) + public static final int CONSTRAINT_CHARGING = JobInfo.CONSTRAINT_FLAG_CHARGING; // 1 < 0 + @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE) + public static final int CONSTRAINT_IDLE = JobInfo.CONSTRAINT_FLAG_DEVICE_IDLE; // 1 << 2 + @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE) + public static final int CONSTRAINT_BATTERY_NOT_LOW = + JobInfo.CONSTRAINT_FLAG_BATTERY_NOT_LOW; // 1 << 1 + @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE) + public static final int CONSTRAINT_STORAGE_NOT_LOW = + JobInfo.CONSTRAINT_FLAG_STORAGE_NOT_LOW; // 1 << 3 public static final int CONSTRAINT_TIMING_DELAY = 1 << 31; public static final int CONSTRAINT_DEADLINE = 1 << 30; public static final int CONSTRAINT_CONNECTIVITY = 1 << 28; @@ -1820,7 +1826,8 @@ public final class JobStatus { * separately from the job's explicitly requested constraints and MUST be satisfied before * the job can run if the app doesn't have quota. */ - private void addDynamicConstraints(int constraints) { + @VisibleForTesting + public void addDynamicConstraints(int constraints) { if ((constraints & CONSTRAINT_WITHIN_QUOTA) != 0) { // Quota should never be used as a dynamic constraint. Slog.wtf(TAG, "Tried to set quota as a dynamic constraint"); diff --git a/services/tests/servicestests/src/com/android/server/job/JobStoreTest.java b/services/tests/servicestests/src/com/android/server/job/JobStoreTest.java index 1c44da1fdb892..5560b41b164fa 100644 --- a/services/tests/servicestests/src/com/android/server/job/JobStoreTest.java +++ b/services/tests/servicestests/src/com/android/server/job/JobStoreTest.java @@ -6,6 +6,7 @@ import static android.net.NetworkCapabilities.TRANSPORT_WIFI; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThrows; @@ -208,6 +209,34 @@ public class JobStoreTest { assertEquals("Incorrect # of persisted tasks.", 0, jobStatusSet.size()); } + /** + * Test that dynamic constraints aren't written to disk. + */ + @Test + public void testDynamicConstraintsNotPersisted() throws Exception { + JobInfo.Builder b = new Builder(42, mComponent).setPersisted(true); + JobStatus js = JobStatus.createFromJobInfo(b.build(), SOME_UID, null, -1, null, null); + js.addDynamicConstraints(JobStatus.CONSTRAINT_BATTERY_NOT_LOW + | JobStatus.CONSTRAINT_CHARGING + | JobStatus.CONSTRAINT_IDLE + | JobStatus.CONSTRAINT_STORAGE_NOT_LOW); + assertTrue(js.hasBatteryNotLowConstraint()); + assertTrue(js.hasChargingConstraint()); + assertTrue(js.hasIdleConstraint()); + assertTrue(js.hasStorageNotLowConstraint()); + mTaskStoreUnderTest.add(js); + waitForPendingIo(); + + final JobSet jobStatusSet = new JobSet(); + mTaskStoreUnderTest.readJobMapFromDisk(jobStatusSet, true); + assertEquals("Job count is incorrect.", 1, jobStatusSet.size()); + JobStatus loaded = jobStatusSet.getAllJobs().iterator().next(); + assertFalse(loaded.hasBatteryNotLowConstraint()); + assertFalse(loaded.hasChargingConstraint()); + assertFalse(loaded.hasIdleConstraint()); + assertFalse(loaded.hasStorageNotLowConstraint()); + } + @Test public void testExtractUidFromJobFileName() { File file = new File(mTestContext.getFilesDir(), "randomName");