am 672ebb61: Merge "Improved ignore-backoff handling Allow a non-epidited ignore-backoff op to pass through an expidited backed off op." into gingerbread

* commit '672ebb61a755e4bbe60e4e884b1adadf186733b6':
  Improved ignore-backoff handling Allow a non-epidited ignore-backoff op to pass through an expidited backed off op.

core/java/android/content/SyncQueue.java

@@ -16,7 +16,7 @@
 
 package android.content;
 
-import com.google.android.collect.Lists;
+import android.os.SystemClock;
 import com.google.android.collect.Maps;
 
 import android.util.Pair;
@@ -130,33 +130,13 @@ public class SyncQueue {
     public Pair<SyncOperation, Long> nextOperation() {
         SyncOperation best = null;
         long bestRunTime = 0;
-        boolean bestSyncableIsUnknownAndNotARetry = false;
+        boolean bestIsInitial = false;
         for (SyncOperation op : mOperationsMap.values()) {
-            long opRunTime = op.earliestRunTime;
+            final long opRunTime = getOpTime(op);
-            if (!op.extras.getBoolean(ContentResolver.SYNC_EXTRAS_IGNORE_BACKOFF, false)) {
+            final boolean opIsInitial = getIsInitial(op);
-                Pair<Long, Long> backoff = mSyncStorageEngine.getBackoff(op.account, op.authority);
+            if (isOpBetter(best, bestRunTime, bestIsInitial, op, opRunTime, opIsInitial)) {
-                long delayUntil = mSyncStorageEngine.getDelayUntilTime(op.account, op.authority);
-                opRunTime = Math.max(
-                        Math.max(opRunTime, delayUntil),
-                        backoff != null ? backoff.first : 0);
-            }
-            // we know a sync is a retry if the intialization flag is set, since that will only
-            // be set by the sync dispatching code, thus if it is set it must have already been
-            // dispatched
-            final boolean syncableIsUnknownAndNotARetry =
-                    !op.extras.getBoolean(ContentResolver.SYNC_EXTRAS_INITIALIZE, false)
-                    && mSyncStorageEngine.getIsSyncable(op.account, op.authority) < 0;
-            // if the unsyncable state differs, make the current the best if it is unsyncable
-            // else, if the expedited state differs, make the current the best if it is expedited
-            // else, make the current the best if it is earlier than the best
-            if (best == null
-                    || ((bestSyncableIsUnknownAndNotARetry == syncableIsUnknownAndNotARetry)
-                        ? (best.expedited == op.expedited
-                           ? opRunTime < bestRunTime
-                           : op.expedited)
-                        : syncableIsUnknownAndNotARetry)) {
                 best = op;
-                bestSyncableIsUnknownAndNotARetry = syncableIsUnknownAndNotARetry;
+                bestIsInitial = opIsInitial;
                 bestRunTime = opRunTime;
             }
         }
@@ -166,6 +146,91 @@ public class SyncQueue {
         return Pair.create(best, bestRunTime);
     }
 
+    // VisibleForTesting
+    long getOpTime(SyncOperation op) {
+        long opRunTime = op.earliestRunTime;
+        if (!op.extras.getBoolean(ContentResolver.SYNC_EXTRAS_IGNORE_BACKOFF, false)) {
+            Pair<Long, Long> backoff = mSyncStorageEngine.getBackoff(op.account, op.authority);
+            long delayUntil = mSyncStorageEngine.getDelayUntilTime(op.account, op.authority);
+            opRunTime = Math.max(
+                    Math.max(opRunTime, delayUntil),
+                    backoff != null ? backoff.first : 0);
+        }
+        return opRunTime;
+    }
+
+    // VisibleForTesting
+    boolean getIsInitial(SyncOperation op) {
+        // Initial op is defined as an op with an unknown syncable that is not a retry.
+        // We know a sync is a retry if the intialization flag is set, since that will only
+        // be set by the sync dispatching code, thus if it is set it must have already been
+        // dispatched
+        return !op.extras.getBoolean(ContentResolver.SYNC_EXTRAS_INITIALIZE, false)
+        && mSyncStorageEngine.getIsSyncable(op.account, op.authority) < 0;
+    }
+
+    // return true if op is a better candidate than best. Rules:
+    // if the "Initial" state differs, make the current the best if it is "Initial".
+    // else, if the expedited state differs, pick the expedited unless it is backed off and the
+    // non-expedited isn't
+    // VisibleForTesting
+    boolean isOpBetter(
+            SyncOperation best, long bestRunTime, boolean bestIsInitial,
+            SyncOperation op, long opRunTime, boolean opIsInitial) {
+        boolean setBest = false;
+        if (Log.isLoggable(TAG, Log.VERBOSE)) {
+            Log.v(TAG,  "nextOperation: Processing op: " + op);
+        }
+        if (best == null) {
+            if (Log.isLoggable(TAG, Log.VERBOSE)) {
+                Log.v(TAG,  "   First op selected");
+            }
+            setBest = true;
+        } else if (bestIsInitial == opIsInitial) {
+            if (best.expedited == op.expedited) {
+                if (opRunTime < bestRunTime) {
+                    //  if both have same level, earlier time wins
+                    if (Log.isLoggable(TAG, Log.VERBOSE)) {
+                        Log.v(TAG,  "   Same expedite level - new op selected");
+                    }
+                    setBest = true;
+                }
+            } else {
+                final long now = SystemClock.elapsedRealtime();
+                if (op.expedited) {
+                    if (opRunTime <= now || bestRunTime > now) {
+                        // if op is expedited, it wins unless op can't run yet and best can
+                        if (Log.isLoggable(TAG, Log.VERBOSE)) {
+                            Log.v(TAG,  "   New op is expedited and can run - new op selected");
+                        }
+                        setBest = true;
+                    } else {
+                        if (Log.isLoggable(TAG, Log.VERBOSE)) {
+                            Log.v(TAG,  "   New op is expedited but can't run and best can");
+                        }
+                    }
+                } else {
+                    if (bestRunTime > now && opRunTime <= now) {
+                        // if best is expedited but can't run yet and op can run, op wins
+                        if (Log.isLoggable(TAG, Log.VERBOSE)) {
+                            Log.v(TAG,
+                                    "   New op is not expedited but can run - new op selected");
+                        }
+                        setBest = true;
+                    }
+                }
+            }
+        } else {
+            if (opIsInitial) {
+                if (Log.isLoggable(TAG, Log.VERBOSE)) {
+                    Log.v(TAG,  "   New op is init - new op selected");
+                }
+                setBest = true;
+            }
+        }
+        return setBest;
+    }
+
     /**
      * Find and return the SyncOperation that should be run next and is ready to run.
      * @param now the current {@link android.os.SystemClock#elapsedRealtime()}, used to

core/tests/coretests/src/android/content/SyncQueueTest.java

@@ -24,6 +24,8 @@ import android.accounts.Account;
 import android.os.Bundle;
 import android.os.SystemClock;
 
+import java.io.File;
+
 public class SyncQueueTest extends AndroidTestCase {
     private static final Account ACCOUNT1 = new Account("test.account1", "test.type1");
     private static final Account ACCOUNT2 = new Account("test.account2", "test.type2");
@@ -138,6 +140,38 @@ public class SyncQueueTest extends AndroidTestCase {
         mSyncQueue.remove(op3);
     }
 
+    public void testExpeditedVsBackoff() throws Exception {
+
+        final SyncOperation op1 = new SyncOperation(
+                ACCOUNT1, SyncStorageEngine.SOURCE_USER, AUTHORITY1, newTestBundle("1"), 0);
+        final SyncOperation op2 = new SyncOperation(
+                ACCOUNT1, SyncStorageEngine.SOURCE_USER, AUTHORITY1, newTestBundle("2"), 0);
+
+        // op1 is expedited but backed off
+        mSettings.setBackoff(ACCOUNT1,  AUTHORITY1, SystemClock.elapsedRealtime() + 1000000, 5);
+        op1.expedited = true;
+
+        // op2 is not expidaited but ignores back off so it should run
+        op2.extras.putBoolean(ContentResolver.SYNC_EXTRAS_IGNORE_BACKOFF, true);
+
+        // First test top level method
+        mSyncQueue.remove(ACCOUNT1, AUTHORITY1);
+        mSyncQueue.add(op1);
+        mSyncQueue.add(op2);
+        assertEquals(op2, mSyncQueue.nextReadyToRun(SystemClock.elapsedRealtime()).first);
+
+        // Since the queue is implemented as a hash, we cannot control the order the op's get
+        // fed into the algorithm so test the inner method in both cases
+        final long opTime1 = mSyncQueue.getOpTime(op1);
+        final boolean isInitial1 = mSyncQueue.getIsInitial(op1);
+        final long opTime2 = mSyncQueue.getOpTime(op2);
+        final boolean opIsInitial2 = mSyncQueue.getIsInitial(op2);
+
+        assertTrue(mSyncQueue.isOpBetter(op1, opTime1, isInitial1, op2, opTime2, opIsInitial2));
+        assertFalse(mSyncQueue.isOpBetter(op2, opTime2, opIsInitial2, op1, opTime1, isInitial1));
+    }
+
+
     Bundle newTestBundle(String val) {
         Bundle bundle = new Bundle();
         bundle.putString("test", val);
@@ -148,7 +182,12 @@ public class SyncQueueTest extends AndroidTestCase {
         ContentResolver mResolver;
 
         public TestContext(ContentResolver resolver, Context realContext) {
-            super(new RenamingDelegatingContext(new MockContext(), realContext, "test."));
+                        super(new RenamingDelegatingContext(new MockContext() {
+                @Override
+                public File getFilesDir() {
+                    return new File("/data");
+                }
+            }, realContext, "test."));
             mResolver = resolver;
         }