Merge "Downgrade expedited to normal on reschedule."

6 files changed, 106 insertions, 66 deletions

diff --git a/services/core/java/com/android/server/content/SyncManager.java b/services/core/java/com/android/server/content/SyncManager.java
index e43dea9..026fd29 100644
--- a/services/core/java/com/android/server/content/SyncManager.java
+++ b/services/core/java/com/android/server/content/SyncManager.java
@@ -2450,7 +2450,7 @@ public class SyncManager {
                             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/core/java/com/android/server/content/SyncOperation.java b/services/core/java/com/android/server/content/SyncOperation.java
index 5233014..9a4abce 100644
--- a/services/core/java/com/android/server/content/SyncOperation.java
+++ b/services/core/java/com/android/server/content/SyncOperation.java
@@ -65,17 +65,18 @@ public class SyncOperation implements Comparable {
     /** Why this sync was kicked off. {@link #REASON_NAMES} */
     public final int reason;
     /** Where this sync was initiated. */
-    public int syncSource;
+    public final int syncSource;
     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 Bundle extras;
     /** Bare-bones version of this operation that is persisted across reboots. */
     public SyncStorageEngine.PendingOperation pendingOperation;
     /** Elapsed real time in millis at which to run this sync. */
     public long latestRunTime;
     /** Set by the SyncManager in order to delay retries. */
-    public Long backoff;
+    public long backoff;
     /** Specified by the adapter to delay subsequent sync operations. */
     public long delayUntil;
     /**
@@ -92,61 +93,58 @@ public class SyncOperation implements Comparable {
     public SyncOperation(Account account, int userId, int reason, int source, String provider,
             Bundle extras, long runTimeFromNow, long flexTime, long backoff,
             long delayUntil, boolean allowParallelSyncs) {
-        this.target = new SyncStorageEngine.EndPoint(account, provider, userId);
-        this.reason = reason;
-        this.allowParallelSyncs = allowParallelSyncs;
-        this.key = initialiseOperation(this.target, source, extras, runTimeFromNow, flexTime,
-                backoff, delayUntil);
+        this(new SyncStorageEngine.EndPoint(account, provider, userId),
+                reason, source, extras, runTimeFromNow, flexTime, backoff, delayUntil,
+                allowParallelSyncs);
     }
 
     public SyncOperation(ComponentName service, int userId, int reason, int source,
             Bundle extras, long runTimeFromNow, long flexTime, long backoff,
             long delayUntil) {
-        this.target = new SyncStorageEngine.EndPoint(service, userId);
-        // Default to true for sync service. The service itself decides how to handle this.
-        this.allowParallelSyncs = true;
-        this.reason = reason;
-        this.key =
-                initialiseOperation(this.target,
-                        source, extras, runTimeFromNow, flexTime, backoff, delayUntil);
-    }
-
-    /** Used to reschedule a sync at a new point in time. */
-    SyncOperation(SyncOperation other, long newRunTimeFromNow) {
-        this.target = other.target;
-        this.reason = other.reason;
-        this.expedited = other.expedited;
-        this.allowParallelSyncs = other.allowParallelSyncs;
-        // re-use old flex, but only 
-        long newFlexTime = Math.min(other.flexTime, newRunTimeFromNow);
-        this.key =
-                initialiseOperation(this.target,
-                    other.syncSource, other.extras,
-                    newRunTimeFromNow /* runTimeFromNow*/,
-                    newFlexTime /* flexTime */,
-                    other.backoff,
-                    0L /* delayUntil */);
+        this(new SyncStorageEngine.EndPoint(service, userId), reason, source, extras,
+                runTimeFromNow, flexTime, backoff, delayUntil, true /* allowParallelSyncs */);
     }
 
-    private String initialiseOperation(SyncStorageEngine.EndPoint info, int source, Bundle extras,
-            long runTimeFromNow, long flexTime, long backoff, long delayUntil) {
+    private SyncOperation(SyncStorageEngine.EndPoint info, int reason, int source, Bundle extras,
+            long runTimeFromNow, long flexTime, long backoff, long delayUntil,
+            boolean allowParallelSyncs) {
+        this.target = info;
+        this.reason = reason;
         this.syncSource = source;
         this.extras = new Bundle(extras);
         cleanBundle(this.extras);
         this.delayUntil = delayUntil;
         this.backoff = backoff;
+        this.allowParallelSyncs = allowParallelSyncs;
         final long now = SystemClock.elapsedRealtime();
-        if (runTimeFromNow < 0 || isExpedited()) {
+        // Set expedited based on runTimeFromNow. The SyncManager specifies whether the op is
+        // expedited (Not done solely based on bundle).
+        if (runTimeFromNow < 0) {
             this.expedited = true;
+            // Sanity check: Will always be true.
+            if (!this.extras.getBoolean(ContentResolver.SYNC_EXTRAS_EXPEDITED, false)) {
+                this.extras.putBoolean(ContentResolver.SYNC_EXTRAS_EXPEDITED, true);
+            }
             this.latestRunTime = now;
             this.flexTime = 0;
         } else {
             this.expedited = false;
+            this.extras.remove(ContentResolver.SYNC_EXTRAS_EXPEDITED);
             this.latestRunTime = now + runTimeFromNow;
             this.flexTime = flexTime;
         }
         updateEffectiveRunTime();
-        return toKey(info, this.extras);
+        this.key = toKey(info, this.extras);
+    }
+
+    /** Used to reschedule a sync at a new point in time. */
+    public SyncOperation(SyncOperation other, long newRunTimeFromNow) {
+        this(other.target, other.reason, other.syncSource, new Bundle(other.extras),
+                newRunTimeFromNow,
+                0L /* In back-off so no flex */,
+                other.backoff,
+                other.delayUntil,
+                other.allowParallelSyncs);
     }
 
     public boolean matchesAuthority(SyncOperation other) {
@@ -264,7 +262,7 @@ public class SyncOperation implements Comparable {
     }
 
     public boolean isExpedited() {
-        return extras.getBoolean(ContentResolver.SYNC_EXTRAS_EXPEDITED, false) || expedited;
+        return expedited;
     }
 
     public boolean ignoreBackoff() {
diff --git a/services/core/java/com/android/server/content/SyncQueue.java b/services/core/java/com/android/server/content/SyncQueue.java
index 5d93882..587de1c 100644
--- a/services/core/java/com/android/server/content/SyncQueue.java
+++ b/services/core/java/com/android/server/content/SyncQueue.java
@@ -76,12 +76,11 @@ public class SyncQueue {
                 operationToAdd = new SyncOperation(
                         info.account, info.userId, op.reason, op.syncSource, info.provider,
                         op.extras,
-                        0 /* delay */,
+                        op.expedited ? -1 : 0 /* delay */,
                         0 /* flex */,
-                        backoff != null ? backoff.first : 0,
+                        backoff != null ? backoff.first : 0L,
                         mSyncStorageEngine.getDelayUntilTime(info),
                         syncAdapterInfo.type.allowParallelSyncs());
-                operationToAdd.expedited = op.expedited;
                 operationToAdd.pendingOperation = op;
                 add(operationToAdd, op);
             } else if (info.target_service) {
@@ -96,11 +95,10 @@ public class SyncQueue {
                 operationToAdd = new SyncOperation(
                         info.service, info.userId, op.reason, op.syncSource,
                         op.extras,
-                        0 /* delay */,
+                        op.expedited ? -1 : 0 /* delay */,
                         0 /* flex */,
                         backoff != null ? backoff.first : 0,
                         mSyncStorageEngine.getDelayUntilTime(info));
-                operationToAdd.expedited = op.expedited;
                 operationToAdd.pendingOperation = op;
                 add(operationToAdd, op);
             }
@@ -129,7 +127,6 @@ public class SyncQueue {
         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.
diff --git a/services/core/java/com/android/server/content/SyncStorageEngine.java b/services/core/java/com/android/server/content/SyncStorageEngine.java
index 5df7fc6..3a2e4db 100644
--- a/services/core/java/com/android/server/content/SyncStorageEngine.java
+++ b/services/core/java/com/android/server/content/SyncStorageEngine.java
@@ -1088,7 +1088,7 @@ public class SyncStorageEngine extends Handler {
             }
 
             pop = new PendingOperation(authority, op.reason, op.syncSource, op.extras,
-                    op.expedited);
+                    op.isExpedited());
             mPendingOperations.add(pop);
             appendPendingOperationLocked(pop);
 
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..b0296a0 100644
--- a/services/tests/servicestests/src/com/android/server/content/SyncOperationTest.java
+++ b/services/tests/servicestests/src/com/android/server/content/SyncOperationTest.java
@@ -18,12 +18,13 @@ package com.android.server.content;
 
 import android.accounts.Account;
 import android.content.ContentResolver;
+import android.content.Context;
 import android.os.Bundle;
+import android.os.SystemClock;
+import android.provider.Settings;
 import android.test.AndroidTestCase;
 import android.test.suitebuilder.annotation.SmallTest;
 
-import com.android.server.content.SyncOperation;
-
 /**
  * You can run those tests with:
  *
@@ -35,6 +36,21 @@ import com.android.server.content.SyncOperation;
 
 public class SyncOperationTest extends AndroidTestCase {
 
+    Account mDummy;
+    /** Indicate an unimportant long that we're not testing. */
+    long mUnimportantLong = 0L;
+    /** Empty bundle. */
+    Bundle mEmpty;
+    /** Silly authority. */
+    String mAuthority;
+
+    @Override
+    public void setUp() {
+        mDummy = new Account("account1", "type1");
+        mEmpty = new Bundle();
+        mAuthority = "authority1";
+    }
+
     @SmallTest
     public void testToKey() {
         Account account1 = new Account("account1", "type1");
@@ -111,35 +127,64 @@ public class SyncOperationTest extends AndroidTestCase {
 
     @SmallTest
     public void testCompareTo() {
-        Account dummy = new Account("account1", "type1");
-        Bundle b1 = new Bundle();
-        final long unimportant = 0L;
         long soon = 1000;
         long soonFlex = 50;
         long after = 1500;
         long afterFlex = 100;
-        SyncOperation op1 = new SyncOperation(dummy, 0, 0, SyncOperation.REASON_PERIODIC,
-                "authority1", b1, soon, soonFlex, unimportant, unimportant, true);
+        SyncOperation op1 = new SyncOperation(mDummy, 0, 0, SyncOperation.REASON_PERIODIC,
+                "authority1", mEmpty, soon, soonFlex, mUnimportantLong, mUnimportantLong, true);
 
         // Interval disjoint from and after op1.
-        SyncOperation op2 = new SyncOperation(dummy, 0, 0, SyncOperation.REASON_PERIODIC,
-                "authority1", b1, after, afterFlex, unimportant, unimportant, true);
+        SyncOperation op2 = new SyncOperation(mDummy, 0, 0, SyncOperation.REASON_PERIODIC,
+                "authority1", mEmpty, after, afterFlex, mUnimportantLong, mUnimportantLong, true);
 
         // Interval equivalent to op1, but expedited.
         Bundle b2 = new Bundle();
         b2.putBoolean(ContentResolver.SYNC_EXTRAS_EXPEDITED, true);
-        SyncOperation op3 = new SyncOperation(dummy, 0, 0, 0,
-                "authority1", b2, soon, soonFlex, unimportant, unimportant, true);
+        SyncOperation op3 = new SyncOperation(mDummy, 0, 0, 0,
+                "authority1", b2, -1, soonFlex, mUnimportantLong, mUnimportantLong, true);
 
         // Interval overlaps but not equivalent to op1.
-        SyncOperation op4 = new SyncOperation(dummy, 0, 0, SyncOperation.REASON_PERIODIC,
-                "authority1", b1, soon + 100, soonFlex + 100, unimportant, unimportant, true);
+        SyncOperation op4 = new SyncOperation(mDummy, 0, 0, SyncOperation.REASON_PERIODIC,
+                "authority1", mEmpty, soon + 100, soonFlex + 100, mUnimportantLong, mUnimportantLong, true);
 
         assertTrue(op1.compareTo(op2) == -1);
         assertTrue("less than not transitive.", op2.compareTo(op1) == 1);
-        assertTrue(op1.compareTo(op3) == 1);
+        assertTrue("Expedited sync not smaller than non-expedited.", 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("overlapping intervals not correctly compared.", op1.compareTo(op4) == -1);
+        assertTrue("equality not transitive.", op4.compareTo(op1) == 1);
+    }
+
+    @SmallTest
+    public void testCopyConstructor() {
+        long fiveSecondsFromNow = 5 * 1000L;
+        long twoSecondsFlex = 2 * 1000L;
+        long eightSeconds = 8 * 1000L;
+        long fourSeconds = 4 * 1000L;
+
+        Bundle withExpedited = new Bundle();
+        withExpedited.putBoolean(ContentResolver.SYNC_EXTRAS_EXPEDITED, true);
+        SyncOperation op = new SyncOperation(mDummy, 0, 0, SyncOperation.REASON_USER_START,
+                mAuthority, withExpedited, fiveSecondsFromNow, twoSecondsFlex,
+                eightSeconds /* backoff */, fourSeconds /* delayUntil */, true);
+        // Create another sync op to be rerun in 5 minutes.
+        long now = SystemClock.elapsedRealtime();
+        SyncOperation copy = new SyncOperation(op, fiveSecondsFromNow * 60);
+        // Copying an expedited sync to be re-run should not keep expedited property.
+        assertFalse("A rescheduled sync based off an expedited should not be expedited!",
+                copy.isExpedited());
+        assertFalse("A rescheduled sync based off an expedited should not have expedited=true in"
+                + "its bundle.",
+                copy.extras.getBoolean(ContentResolver.SYNC_EXTRAS_EXPEDITED, false));
+        assertTrue("Copied sync is not respecting new provided run-time.",
+                copy.latestRunTime == (now + fiveSecondsFromNow * 60));
+        assertTrue("A rescheduled sync should not have any flex.",
+                copy.flexTime == 0L);
+        assertTrue("A rescheduled op should honour the old op's backoff.",
+                copy.backoff == eightSeconds);
+        assertTrue("A rescheduled op should honour the old op's delayUntil param.",
+                copy.delayUntil == fourSeconds);
+
     }
 }
diff --git a/services/tests/servicestests/src/com/android/server/content/SyncStorageEngineTest.java b/services/tests/servicestests/src/com/android/server/content/SyncStorageEngineTest.java
index 70fd810..ae1967e 100644
--- a/services/tests/servicestests/src/com/android/server/content/SyncStorageEngineTest.java
+++ b/services/tests/servicestests/src/com/android/server/content/SyncStorageEngineTest.java
@@ -131,7 +131,7 @@ public class SyncStorageEngineTest extends AndroidTestCase {
         assertEquals(sop.target.userId, popRetrieved.target.userId);
         assertEquals(sop.reason, popRetrieved.reason);
         assertEquals(sop.syncSource, popRetrieved.syncSource);
-        assertEquals(sop.expedited, popRetrieved.expedited);
+        assertEquals(sop.isExpedited(), popRetrieved.expedited);
         assert(android.content.PeriodicSync.syncExtrasEquals(sop.extras, popRetrieved.extras));
     }
 
@@ -169,7 +169,7 @@ public class SyncStorageEngineTest extends AndroidTestCase {
         assertEquals(deleted.target.userId, popDeleted.target.userId);
         assertEquals(deleted.reason, popDeleted.reason);
         assertEquals(deleted.syncSource, popDeleted.syncSource);
-        assertEquals(deleted.expedited, popDeleted.expedited);
+        assertEquals(deleted.isExpedited(), popDeleted.expedited);
         assert(android.content.PeriodicSync.syncExtrasEquals(deleted.extras, popDeleted.extras));
         // Delete one to force write-all
         engine.deleteFromPending(popDeleted);
@@ -192,7 +192,7 @@ public class SyncStorageEngineTest extends AndroidTestCase {
         assertEquals(sop.target.userId, popRetrieved.target.userId);
         assertEquals(sop.reason, popRetrieved.reason);
         assertEquals(sop.syncSource, popRetrieved.syncSource);
-        assertEquals(sop.expedited, popRetrieved.expedited);
+        assertEquals(sop.isExpedited(), popRetrieved.expedited);
         assert(android.content.PeriodicSync.syncExtrasEquals(sop.extras, popRetrieved.extras));
 
         popRetrieved = pops.get(1);
@@ -202,7 +202,7 @@ public class SyncStorageEngineTest extends AndroidTestCase {
         assertEquals(sop1.target.userId, popRetrieved.target.userId);
         assertEquals(sop1.reason, popRetrieved.reason);
         assertEquals(sop1.syncSource, popRetrieved.syncSource);
-        assertEquals(sop1.expedited, popRetrieved.expedited);
+        assertEquals(sop1.isExpedited(), popRetrieved.expedited);
         assert(android.content.PeriodicSync.syncExtrasEquals(sop1.extras, popRetrieved.extras));
     }