From 0812647050a9c588773ba9a0520d661db3d7f1b2 Mon Sep 17 00:00:00 2001 From: Suprabh Shukla Date: Tue, 4 Feb 2020 20:46:35 -0800 Subject: [PATCH] Standby exemption for system gallery's backup jobs New media requiring backup can appear on the device without the user directly using the gallery app. If the gallery app is responsible for media backups, this can lead to a lot of pending backup jobs because of the likely low-priority standby bucket of the gallery app. We should be lenient in imposing throttling to such jobs. Test: atest FrameworksMockingServicesTests:JobStatusTest Bug: 148887841 Change-Id: I54a42037c335f86a9078cf15e3e8d745d03c7337 --- .../java/android/app/job/JobInfo.java | 2 +- .../server/job/JobSchedulerInternal.java | 5 + .../server/job/JobSchedulerService.java | 12 +++ .../ContentObserverController.java | 2 +- .../server/job/controllers/JobStatus.java | 37 +++++++- .../server/job/controllers/JobStatusTest.java | 93 ++++++++++++++++++- 6 files changed, 146 insertions(+), 5 deletions(-) diff --git a/apex/jobscheduler/framework/java/android/app/job/JobInfo.java b/apex/jobscheduler/framework/java/android/app/job/JobInfo.java index 088cadba89ab8..8a9c774117d3b 100644 --- a/apex/jobscheduler/framework/java/android/app/job/JobInfo.java +++ b/apex/jobscheduler/framework/java/android/app/job/JobInfo.java @@ -896,7 +896,7 @@ public class JobInfo implements Parcelable { * @param flags Flags for the observer. */ public TriggerContentUri(@NonNull Uri uri, @Flags int flags) { - mUri = uri; + mUri = Objects.requireNonNull(uri); mFlags = flags; } diff --git a/apex/jobscheduler/framework/java/com/android/server/job/JobSchedulerInternal.java b/apex/jobscheduler/framework/java/com/android/server/job/JobSchedulerInternal.java index 1072406d26cce..7833a037463c0 100644 --- a/apex/jobscheduler/framework/java/com/android/server/job/JobSchedulerInternal.java +++ b/apex/jobscheduler/framework/java/com/android/server/job/JobSchedulerInternal.java @@ -16,6 +16,7 @@ package com.android.server.job; +import android.annotation.NonNull; import android.app.job.JobInfo; import android.util.proto.ProtoOutputStream; @@ -44,6 +45,10 @@ public interface JobSchedulerInternal { void removeBackingUpUid(int uid); void clearAllBackingUpUids(); + /** Returns the package responsible for backing up media on the device. */ + @NonNull + String getMediaBackupPackage(); + /** * The user has started interacting with the app. Take any appropriate action. */ 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 e4c6b52f94bb0..ff7944d073108 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java +++ b/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java @@ -77,6 +77,7 @@ import android.util.SparseIntArray; import android.util.TimeUtils; import android.util.proto.ProtoOutputStream; +import com.android.internal.R; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.app.IBatteryStats; import com.android.internal.util.ArrayUtils; @@ -248,6 +249,9 @@ public class JobSchedulerService extends com.android.server.SystemService */ private final List mJobRestrictions; + @NonNull + private final String mSystemGalleryPackage; + private final CountQuotaTracker mQuotaTracker; private static final String QUOTA_TRACKER_SCHEDULE_PERSISTED_TAG = ".schedulePersisted()"; @@ -1394,6 +1398,9 @@ public class JobSchedulerService extends com.android.server.SystemService mJobRestrictions = new ArrayList<>(); mJobRestrictions.add(new ThermalStatusRestriction(this)); + mSystemGalleryPackage = Objects.requireNonNull( + context.getString(R.string.config_systemGallery)); + // If the job store determined that it can't yet reschedule persisted jobs, // we need to start watching the clock. if (!mJobs.jobTimesInflatedValid()) { @@ -2358,6 +2365,11 @@ public class JobSchedulerService extends com.android.server.SystemService } } + @Override + public String getMediaBackupPackage() { + return mSystemGalleryPackage; + } + @Override public void reportAppUsage(String packageName, int userId) { JobSchedulerService.this.reportAppUsage(packageName, userId); diff --git a/apex/jobscheduler/service/java/com/android/server/job/controllers/ContentObserverController.java b/apex/jobscheduler/service/java/com/android/server/job/controllers/ContentObserverController.java index a775cf5a671c8..5fcd774189ac2 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/controllers/ContentObserverController.java +++ b/apex/jobscheduler/service/java/com/android/server/job/controllers/ContentObserverController.java @@ -344,7 +344,7 @@ public final class ContentObserverController extends StateController { mContext.getContentResolver().unregisterContentObserver(obs); ArrayMap observerOfUser = mObservers.get(obs.mUserId); - if (observerOfUser != null) { + if (observerOfUser != null) { observerOfUser.remove(obs.mUri); } } 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 1e89158ca4bb7..cf7f3804b34df 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 @@ -19,6 +19,7 @@ package com.android.server.job.controllers; import static com.android.server.job.JobSchedulerService.ACTIVE_INDEX; import static com.android.server.job.JobSchedulerService.NEVER_INDEX; import static com.android.server.job.JobSchedulerService.RESTRICTED_INDEX; +import static com.android.server.job.JobSchedulerService.WORKING_INDEX; import static com.android.server.job.JobSchedulerService.sElapsedRealtimeClock; import android.app.AppGlobals; @@ -30,6 +31,7 @@ import android.net.Network; import android.net.Uri; import android.os.RemoteException; import android.os.UserHandle; +import android.provider.MediaStore; import android.text.format.DateFormat; import android.util.ArraySet; import android.util.Pair; @@ -207,6 +209,18 @@ public final class JobStatus { */ private int mDynamicConstraints = 0; + /** + * Indicates whether the job is responsible for backing up media, so we can be lenient in + * applying standby throttling. + * + * Doesn't exempt jobs with a deadline constraint, as they can be started without any content or + * network changes, in which case this exemption does not make sense. + * + * TODO(b/149519887): Use a more explicit signal, maybe an API flag, that the scheduling package + * needs to provide at the time of scheduling a job. + */ + private final boolean mHasMediaBackupExemption; + // Set to true if doze constraint was satisfied due to app being whitelisted. public boolean dozeWhitelisted; @@ -415,9 +429,11 @@ public final class JobStatus { this.mOriginalLatestRunTimeElapsedMillis = latestRunTimeElapsedMillis; this.numFailures = numFailures; + boolean requiresNetwork = false; int requiredConstraints = job.getConstraintFlags(); if (job.getRequiredNetwork() != null) { requiredConstraints |= CONSTRAINT_CONNECTIVITY; + requiresNetwork = true; } if (earliestRunTimeElapsedMillis != NO_EARLIEST_RUNTIME) { requiredConstraints |= CONSTRAINT_TIMING_DELAY; @@ -425,8 +441,16 @@ public final class JobStatus { if (latestRunTimeElapsedMillis != NO_LATEST_RUNTIME) { requiredConstraints |= CONSTRAINT_DEADLINE; } + boolean mediaOnly = false; if (job.getTriggerContentUris() != null) { requiredConstraints |= CONSTRAINT_CONTENT_TRIGGER; + mediaOnly = true; + for (JobInfo.TriggerContentUri uri : job.getTriggerContentUris()) { + if (!MediaStore.AUTHORITY.equals(uri.getUri().getAuthority())) { + mediaOnly = false; + break; + } + } } this.requiredConstraints = requiredConstraints; mRequiredConstraintsOfInterest = requiredConstraints & CONSTRAINTS_OF_INTEREST; @@ -450,6 +474,9 @@ public final class JobStatus { // our source UID into place. job.getRequiredNetwork().networkCapabilities.setSingleUid(this.sourceUid); } + final JobSchedulerInternal jsi = LocalServices.getService(JobSchedulerInternal.class); + mHasMediaBackupExemption = !job.hasLateConstraint() && mediaOnly && requiresNetwork + && this.sourcePackageName.equals(jsi.getMediaBackupPackage()); } /** Copy constructor: used specifically when cloning JobStatus objects for persistence, @@ -545,7 +572,6 @@ public final class JobStatus { int standbyBucket = JobSchedulerService.standbyBucketForPackage(jobPackage, sourceUserId, elapsedNow); - JobSchedulerInternal js = LocalServices.getService(JobSchedulerInternal.class); return new JobStatus(job, callingUid, sourcePkg, sourceUserId, standbyBucket, tag, 0, earliestRunTimeElapsedMillis, latestRunTimeElapsedMillis, @@ -734,7 +760,14 @@ public final class JobStatus { // like other ACTIVE apps. return ACTIVE_INDEX; } - return getStandbyBucket(); + final int actualBucket = getStandbyBucket(); + if (actualBucket != RESTRICTED_INDEX && actualBucket != NEVER_INDEX + && mHasMediaBackupExemption) { + // Cap it at WORKING_INDEX as media back up jobs are important to the user, and the + // source package may not have been used directly in a while. + return Math.min(WORKING_INDEX, actualBucket); + } + return actualBucket; } /** Returns the real standby bucket of the job. */ 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 64da6f6b85907..d7a3cfd8aeca3 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 @@ -19,6 +19,12 @@ package com.android.server.job.controllers; import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn; import static com.android.dx.mockito.inline.extended.ExtendedMockito.mock; import static com.android.dx.mockito.inline.extended.ExtendedMockito.mockitoSession; +import static com.android.server.job.JobSchedulerService.ACTIVE_INDEX; +import static com.android.server.job.JobSchedulerService.FREQUENT_INDEX; +import static com.android.server.job.JobSchedulerService.NEVER_INDEX; +import static com.android.server.job.JobSchedulerService.RARE_INDEX; +import static com.android.server.job.JobSchedulerService.RESTRICTED_INDEX; +import static com.android.server.job.JobSchedulerService.WORKING_INDEX; import static com.android.server.job.controllers.JobStatus.CONSTRAINT_BACKGROUND_NOT_RESTRICTED; import static com.android.server.job.controllers.JobStatus.CONSTRAINT_BATTERY_NOT_LOW; import static com.android.server.job.controllers.JobStatus.CONSTRAINT_CHARGING; @@ -34,13 +40,16 @@ import static com.android.server.job.controllers.JobStatus.CONSTRAINT_WITHIN_QUO import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.when; import android.app.job.JobInfo; import android.app.usage.UsageStatsManagerInternal; import android.content.ComponentName; import android.content.pm.PackageManagerInternal; +import android.net.Uri; import android.os.SystemClock; import android.provider.MediaStore; +import android.util.SparseIntArray; import androidx.test.runner.AndroidJUnit4; @@ -52,6 +61,7 @@ import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.Mock; import org.mockito.MockitoSession; import org.mockito.quality.Strictness; @@ -61,7 +71,12 @@ import java.time.ZoneOffset; @RunWith(AndroidJUnit4.class) public class JobStatusTest { private static final double DELTA = 0.00001; + private static final String TEST_PACKAGE = "job.test.package"; + private static final ComponentName TEST_JOB_COMPONENT = new ComponentName(TEST_PACKAGE, "test"); + private static final Uri TEST_MEDIA_URI = Uri.parse("content://media/path/to/media"); + @Mock + private JobSchedulerInternal mJobSchedulerInternal; private MockitoSession mMockingSession; @Before @@ -71,7 +86,7 @@ public class JobStatusTest { .strictness(Strictness.LENIENT) .mockStatic(LocalServices.class) .startMocking(); - doReturn(mock(JobSchedulerInternal.class)) + doReturn(mJobSchedulerInternal) .when(() -> LocalServices.getService(JobSchedulerInternal.class)); doReturn(mock(PackageManagerInternal.class)) .when(() -> LocalServices.getService(PackageManagerInternal.class)); @@ -94,6 +109,82 @@ public class JobStatusTest { } } + private static void assertEffectiveBucketForMediaExemption(JobStatus jobStatus, + boolean exemptionGranted) { + final SparseIntArray effectiveBucket = new SparseIntArray(); + effectiveBucket.put(ACTIVE_INDEX, ACTIVE_INDEX); + effectiveBucket.put(WORKING_INDEX, WORKING_INDEX); + effectiveBucket.put(FREQUENT_INDEX, exemptionGranted ? WORKING_INDEX : FREQUENT_INDEX); + effectiveBucket.put(RARE_INDEX, exemptionGranted ? WORKING_INDEX : RARE_INDEX); + effectiveBucket.put(NEVER_INDEX, NEVER_INDEX); + effectiveBucket.put(RESTRICTED_INDEX, RESTRICTED_INDEX); + for (int i = 0; i < effectiveBucket.size(); i++) { + jobStatus.setStandbyBucket(effectiveBucket.keyAt(i)); + assertEquals(effectiveBucket.valueAt(i), jobStatus.getEffectiveStandbyBucket()); + } + } + + @Test + public void testMediaBackupExemption_lateConstraint() { + final JobInfo triggerContentJob = new JobInfo.Builder(42, TEST_JOB_COMPONENT) + .addTriggerContentUri(new JobInfo.TriggerContentUri(TEST_MEDIA_URI, 0)) + .setOverrideDeadline(12) + .setRequiredNetworkType(JobInfo.NETWORK_TYPE_ANY) + .build(); + when(mJobSchedulerInternal.getMediaBackupPackage()).thenReturn(TEST_PACKAGE); + assertEffectiveBucketForMediaExemption(createJobStatus(triggerContentJob), false); + } + + @Test + public void testMediaBackupExemption_noConnectivityConstraint() { + final JobInfo triggerContentJob = new JobInfo.Builder(42, TEST_JOB_COMPONENT) + .addTriggerContentUri(new JobInfo.TriggerContentUri(TEST_MEDIA_URI, 0)) + .build(); + when(mJobSchedulerInternal.getMediaBackupPackage()).thenReturn(TEST_PACKAGE); + assertEffectiveBucketForMediaExemption(createJobStatus(triggerContentJob), false); + } + + @Test + public void testMediaBackupExemption_noContentTriggerConstraint() { + final JobInfo networkJob = new JobInfo.Builder(42, TEST_JOB_COMPONENT) + .setRequiredNetworkType(JobInfo.NETWORK_TYPE_ANY) + .build(); + when(mJobSchedulerInternal.getMediaBackupPackage()).thenReturn(TEST_PACKAGE); + assertEffectiveBucketForMediaExemption(createJobStatus(networkJob), false); + } + + @Test + public void testMediaBackupExemption_wrongSourcePackage() { + final JobInfo networkContentJob = new JobInfo.Builder(42, TEST_JOB_COMPONENT) + .addTriggerContentUri(new JobInfo.TriggerContentUri(TEST_MEDIA_URI, 0)) + .setRequiredNetworkType(JobInfo.NETWORK_TYPE_ANY) + .build(); + when(mJobSchedulerInternal.getMediaBackupPackage()).thenReturn("not.test.package"); + assertEffectiveBucketForMediaExemption(createJobStatus(networkContentJob), false); + } + + @Test + public void testMediaBackupExemption_nonMediaUri() { + final Uri nonMediaUri = Uri.parse("content://not-media/any/path"); + final JobInfo networkContentJob = new JobInfo.Builder(42, TEST_JOB_COMPONENT) + .addTriggerContentUri(new JobInfo.TriggerContentUri(TEST_MEDIA_URI, 0)) + .addTriggerContentUri(new JobInfo.TriggerContentUri(nonMediaUri, 0)) + .setRequiredNetworkType(JobInfo.NETWORK_TYPE_ANY) + .build(); + when(mJobSchedulerInternal.getMediaBackupPackage()).thenReturn(TEST_PACKAGE); + assertEffectiveBucketForMediaExemption(createJobStatus(networkContentJob), false); + } + + @Test + public void testMediaBackupExemptionGranted() { + final JobInfo networkContentJob = new JobInfo.Builder(42, TEST_JOB_COMPONENT) + .addTriggerContentUri(new JobInfo.TriggerContentUri(TEST_MEDIA_URI, 0)) + .setRequiredNetworkType(JobInfo.NETWORK_TYPE_ANY) + .build(); + when(mJobSchedulerInternal.getMediaBackupPackage()).thenReturn(TEST_PACKAGE); + assertEffectiveBucketForMediaExemption(createJobStatus(networkContentJob), true); + } + @Test public void testFraction() throws Exception { final long now = JobSchedulerService.sElapsedRealtimeClock.millis();