DO NOT MERGE: Downgrade expedited to normal on reschedule.
bug: 12033540
Expedited was previously tracked by a redundant internal variable, ostensibly
as an optimisation. This variable could differ from the value in the bundle
depending on how the operation is initialised, which led to confusion. Now an
expedited sync will only be treated as such on its first execution.
Change-Id: Ibfc4e9e49b86c82f2364a6ef55f887705a053eb6
diff --git a/services/java/com/android/server/content/SyncManager.java b/services/java/com/android/server/content/SyncManager.java
index 71d8d99..9e3dad6 100644
--- a/services/java/com/android/server/content/SyncManager.java
+++ b/services/java/com/android/server/content/SyncManager.java
@@ -2352,7 +2352,7 @@
Log.v(TAG, "canceling and rescheduling sync since an initialization "
+ "takes higher priority, " + conflict);
}
- } else if (candidate.expedited && !conflict.mSyncOperation.expedited
+ } else if (candidate.isExpedited() && !conflict.mSyncOperation.isExpedited()
&& (candidateIsInitialization
== conflict.mSyncOperation.isInitialization())) {
toReschedule = conflict;
diff --git a/services/java/com/android/server/content/SyncOperation.java b/services/java/com/android/server/content/SyncOperation.java
index 48567478..67e3b09 100644
--- a/services/java/com/android/server/content/SyncOperation.java
+++ b/services/java/com/android/server/content/SyncOperation.java
@@ -66,7 +66,8 @@
public final boolean allowParallelSyncs;
public Bundle extras;
public final String key;
- public boolean expedited;
+ /** Internal boolean to avoid reading a bundle everytime we want to compare operations. */
+ private final boolean expedited;
public SyncStorageEngine.PendingOperation pendingOperation;
/** Elapsed real time in millis at which to run this sync. */
public long latestRunTime;
@@ -79,7 +80,7 @@
* Depends on max(backoff, latestRunTime, and delayUntil).
*/
public long effectiveRunTime;
- /** Amount of time before {@link effectiveRunTime} from which this sync can run. */
+ /** Amount of time before {@link #effectiveRunTime} from which this sync can run. */
public long flexTime;
public SyncOperation(Account account, int userId, int reason, int source, String authority,
@@ -98,11 +99,16 @@
this.backoff = backoff;
final long now = SystemClock.elapsedRealtime();
// Checks the extras bundle. Must occur after we set the internal bundle.
- if (runTimeFromNow < 0 || isExpedited()) {
+ if (runTimeFromNow < 0) {
+ // Sanity check: Will always be true.
+ if (!this.extras.getBoolean(ContentResolver.SYNC_EXTRAS_EXPEDITED, false)) {
+ this.extras.putBoolean(ContentResolver.SYNC_EXTRAS_EXPEDITED, true);
+ }
this.expedited = true;
this.latestRunTime = now;
this.flexTime = 0;
} else {
+ this.extras.remove(ContentResolver.SYNC_EXTRAS_EXPEDITED);
this.expedited = false;
this.latestRunTime = now + runTimeFromNow;
this.flexTime = flexTime;
@@ -111,6 +117,24 @@
this.key = toKey();
}
+ /** Only used to immediately reschedule a sync. */
+ SyncOperation(SyncOperation other) {
+ this.service = other.service;
+ this.account = other.account;
+ this.authority = other.authority;
+ this.userId = other.userId;
+ this.reason = other.reason;
+ this.syncSource = other.syncSource;
+ this.extras = new Bundle(other.extras);
+ this.expedited = other.expedited;
+ this.latestRunTime = SystemClock.elapsedRealtime();
+ this.flexTime = 0L;
+ this.backoff = other.backoff;
+ this.allowParallelSyncs = other.allowParallelSyncs;
+ this.updateEffectiveRunTime();
+ this.key = toKey();
+ }
+
/**
* Make sure the bundle attached to this SyncOperation doesn't have unnecessary
* flags set.
@@ -138,24 +162,6 @@
}
}
- /** Only used to immediately reschedule a sync. */
- SyncOperation(SyncOperation other) {
- this.service = other.service;
- this.account = other.account;
- this.authority = other.authority;
- this.userId = other.userId;
- this.reason = other.reason;
- this.syncSource = other.syncSource;
- this.extras = new Bundle(other.extras);
- this.expedited = other.expedited;
- this.latestRunTime = SystemClock.elapsedRealtime();
- this.flexTime = 0L;
- this.backoff = other.backoff;
- this.allowParallelSyncs = other.allowParallelSyncs;
- this.updateEffectiveRunTime();
- this.key = toKey();
- }
-
@Override
public String toString() {
return dump(null, true);
@@ -220,7 +226,7 @@
}
public boolean isExpedited() {
- return extras.getBoolean(ContentResolver.SYNC_EXTRAS_EXPEDITED, false) || expedited;
+ return expedited;
}
public boolean ignoreBackoff() {
diff --git a/services/java/com/android/server/content/SyncQueue.java b/services/java/com/android/server/content/SyncQueue.java
index 6f3fe6e..22fa2de 100644
--- a/services/java/com/android/server/content/SyncQueue.java
+++ b/services/java/com/android/server/content/SyncQueue.java
@@ -73,10 +73,9 @@
}
SyncOperation syncOperation = new SyncOperation(
op.account, op.userId, op.reason, op.syncSource, op.authority, op.extras,
- 0 /* delay */, 0 /* flex */, backoff != null ? backoff.first : 0,
+ op.expedited ? -1: 0 /* delay */, 0 /* flex */, backoff != null ? backoff.first : 0,
mSyncStorageEngine.getDelayUntilTime(op.account, op.userId, op.authority),
syncAdapterInfo.type.allowParallelSyncs());
- syncOperation.expedited = op.expedited;
syncOperation.pendingOperation = op;
add(syncOperation, op);
}
@@ -104,7 +103,6 @@
if (existingOperation != null) {
boolean changed = false;
if (operation.compareTo(existingOperation) <= 0 ) {
- existingOperation.expedited = operation.expedited;
long newRunTime =
Math.min(existingOperation.latestRunTime, operation.latestRunTime);
// Take smaller runtime.
@@ -123,7 +121,7 @@
if (operation.pendingOperation == null) {
pop = new SyncStorageEngine.PendingOperation(
operation.account, operation.userId, operation.reason, operation.syncSource,
- operation.authority, operation.extras, operation.expedited);
+ operation.authority, operation.extras, operation.isExpedited());
pop = mSyncStorageEngine.insertIntoPending(pop);
if (pop == null) {
throw new IllegalStateException("error adding pending sync operation "
diff --git a/services/java/com/android/server/content/SyncStorageEngine.java b/services/java/com/android/server/content/SyncStorageEngine.java
index 5ebf9ea..781280e 100644
--- a/services/java/com/android/server/content/SyncStorageEngine.java
+++ b/services/java/com/android/server/content/SyncStorageEngine.java
@@ -501,7 +501,7 @@
* @return amount of seconds before syncTimeSeconds that the sync can occur.
* I.e.
* earliest_sync_time = syncTimeSeconds - calculateDefaultFlexTime(syncTimeSeconds)
- * The flex time is capped at a percentage of the {@link DEFAULT_POLL_FREQUENCY_SECONDS}.
+ * The flex time is capped at a percentage of the {@link #DEFAULT_POLL_FREQUENCY_SECONDS}.
*/
public static long calculateDefaultFlexTime(long syncTimeSeconds) {
if (syncTimeSeconds < DEFAULT_MIN_FLEX_ALLOWED_SECS) {
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 37176d6..910b685 100644
--- a/services/tests/servicestests/src/com/android/server/content/SyncOperationTest.java
+++ b/services/tests/servicestests/src/com/android/server/content/SyncOperationTest.java
@@ -136,10 +136,11 @@
"authority1", b1, soon + 100, soonFlex + 100, unimportant, unimportant, true);
assertTrue(op1.compareTo(op2) == -1);
- assertTrue("less than not transitive.", op2.compareTo(op1) == 1);
- assertTrue(op1.compareTo(op3) == 1);
- assertTrue("greater than not transitive. ", op3.compareTo(op1) == -1);
- assertTrue("overlapping intervals not the same.", op1.compareTo(op4) == 0);
- assertTrue("equality not transitive.", op4.compareTo(op1) == 0);
+ assertTrue("Less than not transitive.", op2.compareTo(op1) == 1);
+ assertEquals("Expedited not smaller than non-expedited.", -1, op1.compareTo(op3));
+ assertEquals("Greater than not transitive for expedited. ", 1, op3.compareTo(op1));
+ assertTrue("overlapping intervals not compared based on start interval.",
+ op1.compareTo(op4) == -1);
+ assertTrue("overlapping interval comparison not transitive.", op4.compareTo(op1) == 1);
}
}