From c516abcf46229c27d2f8bcad9d39e8f95a603ab0 Mon Sep 17 00:00:00 2001 From: Varun Shah Date: Tue, 16 Feb 2021 19:10:58 -0800 Subject: [PATCH] Allow syncs to be scheduled as EJs. Bug: 178852366 Test: atest SyncRequestTest Test: atest SyncOperationTest Test: atest SyncManagerTest Test: atest CtsSyncManagerTest Test: atest ContentResolverTest [all] Change-Id: I4a78abdc1f2f5313ac18f739209b76fdf49388e5 --- .../commands/requestsync/RequestSync.java | 2 + core/api/current.txt | 2 + .../java/android/content/ContentResolver.java | 53 +++++++++++++--- core/java/android/content/SyncRequest.java | 63 ++++++++++++++++--- .../android/server/content/SyncManager.java | 25 ++++++++ .../android/server/content/SyncOperation.java | 18 ++++++ .../server/content/SyncOperationTest.java | 21 +++++++ 7 files changed, 169 insertions(+), 15 deletions(-) diff --git a/cmds/requestsync/src/com/android/commands/requestsync/RequestSync.java b/cmds/requestsync/src/com/android/commands/requestsync/RequestSync.java index a0361d0a39d3..ca9df39f83bf 100644 --- a/cmds/requestsync/src/com/android/commands/requestsync/RequestSync.java +++ b/cmds/requestsync/src/com/android/commands/requestsync/RequestSync.java @@ -182,6 +182,8 @@ public class RequestSync { mExtras.putBoolean(ContentResolver.SYNC_EXTRAS_UPLOAD, true); } else if (opt.equals("--rc") || opt.equals("--require-charging")) { mExtras.putBoolean(ContentResolver.SYNC_EXTRAS_REQUIRE_CHARGING, true); + } else if (opt.equals("--ej") || opt.equals("--schedule-as-ej")) { + mExtras.putBoolean(ContentResolver.SYNC_EXTRAS_SCHEDULE_AS_EXPEDITED_JOB, true); } else if (opt.equals("-e") || opt.equals("--es") || opt.equals("--extra-string")) { final String key = nextArgRequired(); final String value = nextArgRequired(); diff --git a/core/api/current.txt b/core/api/current.txt index 1648afbd6bdb..25bd5338bb41 100644 --- a/core/api/current.txt +++ b/core/api/current.txt @@ -10195,6 +10195,7 @@ package android.content { field public static final String SYNC_EXTRAS_MANUAL = "force"; field public static final String SYNC_EXTRAS_OVERRIDE_TOO_MANY_DELETIONS = "deletions_override"; field public static final String SYNC_EXTRAS_REQUIRE_CHARGING = "require_charging"; + field public static final String SYNC_EXTRAS_SCHEDULE_AS_EXPEDITED_JOB = "schedule_as_expedited_job"; field public static final String SYNC_EXTRAS_UPLOAD = "upload"; field public static final int SYNC_OBSERVER_TYPE_ACTIVE = 4; // 0x4 field public static final int SYNC_OBSERVER_TYPE_PENDING = 2; // 0x2 @@ -11516,6 +11517,7 @@ package android.content { method public android.content.SyncRequest.Builder setManual(boolean); method public android.content.SyncRequest.Builder setNoRetry(boolean); method public android.content.SyncRequest.Builder setRequiresCharging(boolean); + method @NonNull public android.content.SyncRequest.Builder setScheduleAsExpeditedJob(boolean); method public android.content.SyncRequest.Builder setSyncAdapter(android.accounts.Account, String); method public android.content.SyncRequest.Builder syncOnce(); method public android.content.SyncRequest.Builder syncPeriodic(long, long); diff --git a/core/java/android/content/ContentResolver.java b/core/java/android/content/ContentResolver.java index 46d8900e59a1..230c985d1dc8 100644 --- a/core/java/android/content/ContentResolver.java +++ b/core/java/android/content/ContentResolver.java @@ -132,8 +132,11 @@ public abstract class ContentResolver implements ContentInterface { public static final String SYNC_EXTRAS_ACCOUNT = "account"; /** - * If this extra is set to true, the sync request will be scheduled - * at the front of the sync request queue and without any delay + * If this extra is set to true, the sync request will be scheduled at the front of the + * sync request queue, but it is still subject to JobScheduler quota and throttling due to + * App Standby buckets. + * + *

This is different from {@link #SYNC_EXTRAS_SCHEDULE_AS_EXPEDITED_JOB}. */ public static final String SYNC_EXTRAS_EXPEDITED = "expedited"; @@ -144,6 +147,29 @@ public abstract class ContentResolver implements ContentInterface { */ public static final String SYNC_EXTRAS_REQUIRE_CHARGING = "require_charging"; + /** + * Run this sync operation as an "expedited job" + * (see {@link android.app.job.JobInfo.Builder#setExpedited(boolean)}). + * Normally (if this flag isn't specified), sync operations are executed as regular + * {@link android.app.job.JobService} jobs. + * + *

Because Expedited Jobs have various restrictions compared to regular jobs, this flag + * cannot be combined with certain other flags, otherwise an + * IllegalArgumentException will be thrown. Notably, because Expedited Jobs do not + * support various constraints, the following restriction apply: + *

+ * + *

This is different from {@link #SYNC_EXTRAS_EXPEDITED}. + */ + @SuppressLint("IntentName") + public static final String SYNC_EXTRAS_SCHEDULE_AS_EXPEDITED_JOB = "schedule_as_expedited_job"; + /** * @deprecated instead use * {@link #SYNC_EXTRAS_MANUAL} @@ -3219,6 +3245,18 @@ public abstract class ContentResolver implements ContentInterface { } } + /** + * {@hide} + * Helper function to throw an IllegalArgumentException if any illegal + * extras were set for a sync scheduled as an expedited job. + * + * @param extras bundle to validate. + */ + public static boolean hasInvalidScheduleAsEjExtras(Bundle extras) { + return extras.getBoolean(ContentResolver.SYNC_EXTRAS_REQUIRE_CHARGING) + || extras.getBoolean(ContentResolver.SYNC_EXTRAS_EXPEDITED); + } + /** * Specifies that a sync should be requested with the specified the account, authority, * and extras at the given frequency. If there is already another periodic sync scheduled @@ -3233,7 +3271,8 @@ public abstract class ContentResolver implements ContentInterface { * Periodic syncs are not allowed to have any of {@link #SYNC_EXTRAS_DO_NOT_RETRY}, * {@link #SYNC_EXTRAS_IGNORE_BACKOFF}, {@link #SYNC_EXTRAS_IGNORE_SETTINGS}, * {@link #SYNC_EXTRAS_INITIALIZE}, {@link #SYNC_EXTRAS_FORCE}, - * {@link #SYNC_EXTRAS_EXPEDITED}, {@link #SYNC_EXTRAS_MANUAL} set to true. + * {@link #SYNC_EXTRAS_EXPEDITED}, {@link #SYNC_EXTRAS_MANUAL}, + * {@link #SYNC_EXTRAS_SCHEDULE_AS_EXPEDITED_JOB} set to true. * If any are supplied then an {@link IllegalArgumentException} will be thrown. * *

This method requires the caller to hold the permission @@ -3273,16 +3312,14 @@ public abstract class ContentResolver implements ContentInterface { * @param extras bundle to validate. */ public static boolean invalidPeriodicExtras(Bundle extras) { - if (extras.getBoolean(ContentResolver.SYNC_EXTRAS_MANUAL, false) + return extras.getBoolean(ContentResolver.SYNC_EXTRAS_MANUAL, false) || extras.getBoolean(ContentResolver.SYNC_EXTRAS_DO_NOT_RETRY, false) || extras.getBoolean(ContentResolver.SYNC_EXTRAS_IGNORE_BACKOFF, false) || extras.getBoolean(ContentResolver.SYNC_EXTRAS_IGNORE_SETTINGS, false) || extras.getBoolean(ContentResolver.SYNC_EXTRAS_INITIALIZE, false) || extras.getBoolean(ContentResolver.SYNC_EXTRAS_FORCE, false) - || extras.getBoolean(ContentResolver.SYNC_EXTRAS_EXPEDITED, false)) { - return true; - } - return false; + || extras.getBoolean(ContentResolver.SYNC_EXTRAS_EXPEDITED, false) + || extras.getBoolean(ContentResolver.SYNC_EXTRAS_SCHEDULE_AS_EXPEDITED_JOB, false); } /** diff --git a/core/java/android/content/SyncRequest.java b/core/java/android/content/SyncRequest.java index 9e568a40e0ee..e1e6f75d152f 100644 --- a/core/java/android/content/SyncRequest.java +++ b/core/java/android/content/SyncRequest.java @@ -17,6 +17,7 @@ package android.content; import android.accounts.Account; +import android.annotation.NonNull; import android.compat.annotation.UnsupportedAppUsage; import android.os.Build; import android.os.Bundle; @@ -58,6 +59,8 @@ public class SyncRequest implements Parcelable { private final boolean mIsAuthority; /** Sync should be run in lieu of other syncs. */ private final boolean mIsExpedited; + /** Sync sound be ran as an expedited job. */ + private final boolean mIsScheduledAsExpeditedJob; /** * {@hide} @@ -77,6 +80,14 @@ public class SyncRequest implements Parcelable { return mIsExpedited; } + /** + * {@hide} + * @return whether this sync is scheduled as an expedited job. + */ + public boolean isScheduledAsExpeditedJob() { + return mIsScheduledAsExpeditedJob; + } + /** * {@hide} * @@ -149,6 +160,7 @@ public class SyncRequest implements Parcelable { parcel.writeInt((mDisallowMetered ? 1 : 0)); parcel.writeInt((mIsAuthority ? 1 : 0)); parcel.writeInt((mIsExpedited? 1 : 0)); + parcel.writeInt(mIsScheduledAsExpeditedJob ? 1 : 0); parcel.writeParcelable(mAccountToSync, flags); parcel.writeString(mAuthority); } @@ -161,6 +173,7 @@ public class SyncRequest implements Parcelable { mDisallowMetered = (in.readInt() != 0); mIsAuthority = (in.readInt() != 0); mIsExpedited = (in.readInt() != 0); + mIsScheduledAsExpeditedJob = (in.readInt() != 0); mAccountToSync = in.readParcelable(null); mAuthority = in.readString(); } @@ -174,6 +187,7 @@ public class SyncRequest implements Parcelable { mIsPeriodic = (b.mSyncType == Builder.SYNC_TYPE_PERIODIC); mIsAuthority = (b.mSyncTarget == Builder.SYNC_TARGET_ADAPTER); mIsExpedited = b.mExpedited; + mIsScheduledAsExpeditedJob = b.mScheduleAsExpeditedJob; mExtras = new Bundle(b.mCustomExtras); // For now we merge the sync config extras & the custom extras into one bundle. // TODO: pass the configuration extras through separately. @@ -258,6 +272,11 @@ public class SyncRequest implements Parcelable { */ private boolean mRequiresCharging; + /** + * Whether the sync should be scheduled as an expedited job. + */ + private boolean mScheduleAsExpeditedJob; + public Builder() { } @@ -309,7 +328,8 @@ public class SyncRequest implements Parcelable { * {@link ContentResolver#SYNC_EXTRAS_INITIALIZE}, * {@link ContentResolver#SYNC_EXTRAS_FORCE}, * {@link ContentResolver#SYNC_EXTRAS_EXPEDITED}, - * {@link ContentResolver#SYNC_EXTRAS_MANUAL} + * {@link ContentResolver#SYNC_EXTRAS_MANUAL}, + * {@link ContentResolver#SYNC_EXTRAS_SCHEDULE_AS_EXPEDITED_JOB} * set to true. If any are supplied then an IllegalArgumentException will * be thrown. * @@ -499,6 +519,22 @@ public class SyncRequest implements Parcelable { return this; } + /** + * Convenience function for setting + * {@link ContentResolver#SYNC_EXTRAS_SCHEDULE_AS_EXPEDITED_JOB}. + * + *

Not to be confused with {@link ContentResolver#SYNC_EXTRAS_EXPEDITED}. + * + *

Not valid for periodic syncs, expedited syncs, and syncs that require charging - an + * IllegalArgumentException will be thrown in {@link #build()}. + * + * @param scheduleAsExpeditedJob whether to schedule as an expedited job. Default false. + */ + public @NonNull Builder setScheduleAsExpeditedJob(boolean scheduleAsExpeditedJob) { + mScheduleAsExpeditedJob = scheduleAsExpeditedJob; + return this; + } + /** * Performs validation over the request and throws the runtime exception * IllegalArgumentException if this validation fails. @@ -507,11 +543,6 @@ public class SyncRequest implements Parcelable { * builder. */ public SyncRequest build() { - // Validate the extras bundle - ContentResolver.validateSyncExtrasBundle(mCustomExtras); - if (mCustomExtras == null) { - mCustomExtras = new Bundle(); - } // Combine builder extra flags into the config bundle. mSyncConfigExtras = new Bundle(); if (mIgnoreBackoff) { @@ -532,17 +563,35 @@ public class SyncRequest implements Parcelable { if (mExpedited) { mSyncConfigExtras.putBoolean(ContentResolver.SYNC_EXTRAS_EXPEDITED, true); } + if (mScheduleAsExpeditedJob) { + mSyncConfigExtras.putBoolean( + ContentResolver.SYNC_EXTRAS_SCHEDULE_AS_EXPEDITED_JOB, true); + } if (mIsManual) { mSyncConfigExtras.putBoolean(ContentResolver.SYNC_EXTRAS_IGNORE_BACKOFF, true); mSyncConfigExtras.putBoolean(ContentResolver.SYNC_EXTRAS_IGNORE_SETTINGS, true); } + + if (mCustomExtras == null) { + mCustomExtras = new Bundle(); + } + // Validate the extras bundles + ContentResolver.validateSyncExtrasBundle(mCustomExtras); + // If this is a periodic sync ensure than invalid extras were not set. if (mSyncType == SYNC_TYPE_PERIODIC) { - // If this is a periodic sync ensure than invalid extras were not set. if (ContentResolver.invalidPeriodicExtras(mCustomExtras) || ContentResolver.invalidPeriodicExtras(mSyncConfigExtras)) { throw new IllegalArgumentException("Illegal extras were set"); } } + // If this sync is scheduled as an EJ, ensure that invalid extras were not set. + if (mCustomExtras.getBoolean(ContentResolver.SYNC_EXTRAS_SCHEDULE_AS_EXPEDITED_JOB) + || mScheduleAsExpeditedJob) { + if (ContentResolver.hasInvalidScheduleAsEjExtras(mCustomExtras) + || ContentResolver.hasInvalidScheduleAsEjExtras(mSyncConfigExtras)) { + throw new IllegalArgumentException("Illegal extras were set"); + } + } // Ensure that a target for the sync has been set. if (mSyncTarget == SYNC_TARGET_UNKNOWN) { throw new IllegalArgumentException("Must specify an adapter with" + diff --git a/services/core/java/com/android/server/content/SyncManager.java b/services/core/java/com/android/server/content/SyncManager.java index fe89903c02d7..7b9ca37b1639 100644 --- a/services/core/java/com/android/server/content/SyncManager.java +++ b/services/core/java/com/android/server/content/SyncManager.java @@ -264,6 +264,10 @@ public class SyncManager { private final SyncLogger mLogger; + // NOTE: this is a temporary allow-list for testing purposes; it will be removed before release. + private final String[] mEjSyncAllowedPackages = new String[]{ + "com.google.android.google", "com.android.frameworks.servicestests"}; + private boolean isJobIdInUseLockedH(int jobId, List pendingJobs) { for (JobInfo job: pendingJobs) { if (job.getId() == jobId) { @@ -983,6 +987,14 @@ public class SyncManager { } } + final boolean scheduleAsEj = + extras.getBoolean(ContentResolver.SYNC_EXTRAS_SCHEDULE_AS_EXPEDITED_JOB, false); + // NOTE: this is a temporary check for internal testing - to be removed before release. + if (scheduleAsEj && !ArrayUtils.contains(mEjSyncAllowedPackages, callingPackage)) { + throw new IllegalArgumentException( + callingPackage + " is not allowed to schedule a sync as an EJ yet."); + } + for (AccountAndUser account : accounts) { // If userId is specified, do not sync accounts of other users if (userId >= UserHandle.USER_SYSTEM && account.userId >= UserHandle.USER_SYSTEM @@ -1490,6 +1502,12 @@ public class SyncManager { + logSafe(syncOperation.target)); backoff = new Pair(SyncStorageEngine.NOT_IN_BACKOFF_MODE, SyncStorageEngine.NOT_IN_BACKOFF_MODE); + } else { + // if an EJ is being backed-off but doesn't have SYNC_EXTRAS_IGNORE_BACKOFF set, + // reschedule it as a regular job + if (syncOperation.isScheduledAsExpeditedJob()) { + syncOperation.scheduleEjAsRegularJob = true; + } } long now = SystemClock.elapsedRealtime(); long backoffDelay = backoff.first == SyncStorageEngine.NOT_IN_BACKOFF_MODE ? 0 @@ -1640,6 +1658,10 @@ public class SyncManager { b.setRequiresCharging(true); } + if (syncOperation.isScheduledAsExpeditedJob() && !syncOperation.scheduleEjAsRegularJob) { + b.setExpedited(true); + } + if (syncOperation.syncExemptionFlag == ContentResolver.SYNC_EXEMPTION_PROMOTE_BUCKET_WITH_TEMP) { DeviceIdleInternal dic = @@ -3951,6 +3973,9 @@ public class SyncManager { if (key.equals(ContentResolver.SYNC_EXTRAS_EXPEDITED)) { return true; } + if (key.equals(ContentResolver.SYNC_EXTRAS_SCHEDULE_AS_EXPEDITED_JOB)) { + return true; + } if (key.equals(ContentResolver.SYNC_EXTRAS_IGNORE_SETTINGS)) { return true; } diff --git a/services/core/java/com/android/server/content/SyncOperation.java b/services/core/java/com/android/server/content/SyncOperation.java index 478763531abc..c8654d1b36ee 100644 --- a/services/core/java/com/android/server/content/SyncOperation.java +++ b/services/core/java/com/android/server/content/SyncOperation.java @@ -103,6 +103,13 @@ public class SyncOperation { /** Stores the number of times this sync operation failed and had to be retried. */ int retries; + /** + * Indicates if a sync that was originally scheduled as an EJ is being re-scheduled as a + * regular job. Specifically, this will be {@code true} if a sync is being backed-off but + * {@link ContentResolver#SYNC_EXTRAS_IGNORE_BACKOFF} is not set. + */ + boolean scheduleEjAsRegularJob; + /** jobId of the JobScheduler job corresponding to this sync */ public int jobId; @@ -408,6 +415,12 @@ public class SyncOperation { if (extras.getBoolean(ContentResolver.SYNC_EXTRAS_EXPEDITED, false)) { sb.append(" EXPEDITED"); } + if (extras.getBoolean(ContentResolver.SYNC_EXTRAS_SCHEDULE_AS_EXPEDITED_JOB, false)) { + sb.append(" EXPEDITED-JOB"); + if (scheduleEjAsRegularJob) { + sb.append("(scheduled-as-regular)"); + } + } switch (syncExemptionFlag) { case ContentResolver.SYNC_EXEMPTION_NONE: break; @@ -537,6 +550,11 @@ public class SyncOperation { return mImmutableExtras.getBoolean(ContentResolver.SYNC_EXTRAS_REQUIRE_CHARGING, false); } + boolean isScheduledAsExpeditedJob() { + return mImmutableExtras.getBoolean( + ContentResolver.SYNC_EXTRAS_SCHEDULE_AS_EXPEDITED_JOB, false); + } + boolean isAppStandbyExempted() { return syncExemptionFlag != ContentResolver.SYNC_EXEMPTION_NONE; } diff --git a/services/tests/servicestests/src/com/android/server/content/SyncOperationTest.java b/services/tests/servicestests/src/com/android/server/content/SyncOperationTest.java index 2cbc3f381909..a694d5e37566 100644 --- a/services/tests/servicestests/src/com/android/server/content/SyncOperationTest.java +++ b/services/tests/servicestests/src/com/android/server/content/SyncOperationTest.java @@ -153,4 +153,25 @@ public class SyncOperationTest extends AndroidTestCase { assertEquals("Period not restored", periodic.periodMillis, oneoff.periodMillis); assertEquals("Flex not restored", periodic.flexMillis, oneoff.flexMillis); } + + @SmallTest + public void testScheduleAsEjIsInExtras() { + Account account1 = new Account("account1", "type1"); + Bundle b1 = new Bundle(); + b1.putBoolean(ContentResolver.SYNC_EXTRAS_SCHEDULE_AS_EXPEDITED_JOB, true); + + SyncOperation op1 = new SyncOperation(account1, 0, 1, "foo", 0, + SyncOperation.REASON_USER_START, "authority1", b1, false, + ContentResolver.SYNC_EXEMPTION_NONE); + assertTrue(op1.isScheduledAsExpeditedJob()); + + PersistableBundle pb = op1.toJobInfoExtras(); + assertTrue("EJ extra not found in job extras", + ((PersistableBundle) pb.get("syncExtras")) + .containsKey(ContentResolver.SYNC_EXTRAS_SCHEDULE_AS_EXPEDITED_JOB)); + + SyncOperation op2 = SyncOperation.maybeCreateFromJobExtras(pb); + assertTrue("EJ extra not found in extras", op2.getClonedExtras() + .getBoolean(ContentResolver.SYNC_EXTRAS_SCHEDULE_AS_EXPEDITED_JOB)); + } }