- changed periodic sync scheduling to just creating pending

and changed the "get next operation to sync" logic just look
  at pending syncs, rather than them and periodic syncs
- made syncoperation dup-detection ignore the initialization
  sync extra
- made the sync dispatcher treat initialization syncs as just
  a regular sync request and also made it explicitly set or
  clear the initialization extra based on whether the sync
  adapter was in the syncable or unknown state
- change the getNextSync logic to prioritize syncable "unknown"
  syncs above everything else (since they should be fast and
  are important)
- make it reschedule completed initialization syncs if the
  sync adapter is now marked syncable
- fix some logging in SyncStorageEngine
- change SyncStorageEngine to not reuse authority ids when one
  is removed

http://b/issue?id=2531359
http://b/issue?id=2429638

Change-Id: I79805b582da74f4f0b6193eafaff24c2371d51e8

core/java/android/content/SyncManager.java

@@ -398,10 +398,6 @@ public class SyncManager implements OnAccountsUpdateListener {
         return minValue + random.nextInt((int)spread);
     }
 
-    public ActiveSyncContext getActiveSyncContext() {
-        return mActiveSyncContext;
-    }
-
     public SyncStorageEngine getSyncStorageEngine() {
         return mSyncStorageEngine;
     }
@@ -412,11 +408,6 @@ public class SyncManager implements OnAccountsUpdateListener {
         }
     }
 
-    public Account getSyncingAccount() {
-        ActiveSyncContext activeSyncContext = mActiveSyncContext;
-        return (activeSyncContext != null) ? activeSyncContext.mSyncOperation.account : null;
-    }
-
     private void initializeSyncAdapter(Account account, String authority) {
         if (Log.isLoggable(TAG, Log.VERBOSE)) {
             Log.v(TAG, "initializeSyncAdapter: " + account + ", authority " + authority);
@@ -425,7 +416,8 @@ public class SyncManager implements OnAccountsUpdateListener {
         RegisteredServicesCache.ServiceInfo<SyncAdapterType> syncAdapterInfo =
                 mSyncAdapters.getServiceInfo(syncAdapterType);
         if (syncAdapterInfo == null) {
-            Log.w(TAG, "can't find a sync adapter for " + syncAdapterType);
+            Log.w(TAG, "can't find a sync adapter for " + syncAdapterType + ", removing");
+            mSyncStorageEngine.removeAuthority(account, authority);
             return;
         }
 
@@ -466,6 +458,8 @@ public class SyncManager implements OnAccountsUpdateListener {
                 }
             } catch (RemoteException e) {
                 // doesn't matter, we will retry again later
+                Log.d(TAG, "error while initializing: " + mAccount + ", authority " + mAuthority,
+                        e);
             } finally {
                 // give the sync adapter time to initialize before unbinding from it
                 // TODO: change this API to not rely on this timing, http://b/2500805
@@ -602,16 +596,12 @@ public class SyncManager implements OnAccountsUpdateListener {
                         continue;
                     }
 
-                    // initialize the SyncAdapter if the isSyncable state is unknown
+                    // always allow if the isSyncable state is unknown
-                    if (isSyncable < 0) {
-                        initializeSyncAdapter(account, authority);
-                        continue;
-                    }
-
-                    final boolean syncAutomatically = masterSyncAutomatically
-                            && mSyncStorageEngine.getSyncAutomatically(account, authority);
                     boolean syncAllowed =
-                            ignoreSettings || (backgroundDataUsageAllowed && syncAutomatically);
+                            (isSyncable < 0)
+                            || ignoreSettings
+                            || (backgroundDataUsageAllowed && masterSyncAutomatically
+                                && mSyncStorageEngine.getSyncAutomatically(account, authority));
                     if (!syncAllowed) {
                         if (isLoggable) {
                             Log.d(TAG, "scheduleSync: sync of " + account + ", " + authority
@@ -1061,7 +1051,7 @@ public class SyncManager implements OnAccountsUpdateListener {
                 pw.println(op.expedited);
                 if (op.extras != null && op.extras.size() > 0) {
                     sb.setLength(0);
-                    SyncOperation.extrasToStringBuilder(op.extras, sb);
+                    SyncOperation.extrasToStringBuilder(op.extras, sb, false /* asKey */);
                     pw.print("    extras: "); pw.println(sb.toString());
                 }
             }
@@ -1376,8 +1366,14 @@ public class SyncManager implements OnAccountsUpdateListener {
         }
 
         public void handleMessage(Message msg) {
+            Long earliestFuturePollTime = null;
             try {
                 waitUntilReadyToRun();
+                // Always do this first so that we be sure that any periodic syncs that
+                // are ready to run have been converted into pending syncs. This allows the
+                // logic that considers the next steps to take based on the set of pending syncs
+                // to also take into account the periodic syncs.
+                earliestFuturePollTime = scheduleReadyPeriodicSyncs();
                 switch (msg.what) {
                     case SyncHandler.MESSAGE_SYNC_FINISHED:
                         if (Log.isLoggable(TAG, Log.VERBOSE)) {
@@ -1486,48 +1482,90 @@ public class SyncManager implements OnAccountsUpdateListener {
                 }
                 manageSyncNotification();
                 manageErrorNotification();
-                manageSyncAlarm();
+                manageSyncAlarm(earliestFuturePollTime);
                 mSyncTimeTracker.update();
             }
         }
 
-        private boolean isSyncAllowed(Account account, String authority, boolean ignoreSettings,
+        /**
-                boolean backgroundDataUsageAllowed) {
+         * Turn any periodic sync operations that are ready to run into pending sync operations.
-            Account[] accounts = mAccounts;
+         * @return the desired start time of the earliest future  periodic sync operation,
+         * in milliseconds since boot
+         */
+        private Long scheduleReadyPeriodicSyncs() {
+            final boolean backgroundDataUsageAllowed =
+                    getConnectivityManager().getBackgroundDataSetting();
+            Long earliestFuturePollTime = null;
+            if (!backgroundDataUsageAllowed || !mSyncStorageEngine.getMasterSyncAutomatically()) {
+                return earliestFuturePollTime;
+            }
+            final long nowAbsolute = System.currentTimeMillis();
+            ArrayList<SyncStorageEngine.AuthorityInfo> infos = mSyncStorageEngine.getAuthorities();
+            for (SyncStorageEngine.AuthorityInfo info : infos) {
+                // skip the sync if the account of this operation no longer exists
+                if (!ArrayUtils.contains(mAccounts, info.account)) {
+                    continue;
+                }
 
-            // skip the sync if the account of this operation no longer exists
+                if (!mSyncStorageEngine.getSyncAutomatically(info.account, info.authority)) {
-            if (!ArrayUtils.contains(accounts, account)) {
+                    continue;
-                return false;
+                }
+
+                if (mSyncStorageEngine.getIsSyncable(info.account, info.authority) == 0) {
+                    continue;
+                }
+
+                SyncStatusInfo status = mSyncStorageEngine.getOrCreateSyncStatus(info);
+                for (int i = 0, N = info.periodicSyncs.size(); i < N; i++) {
+                    final Bundle extras = info.periodicSyncs.get(i).first;
+                    final Long periodInSeconds = info.periodicSyncs.get(i).second;
+                    // find when this periodic sync was last scheduled to run
+                    final long lastPollTimeAbsolute = status.getPeriodicSyncTime(i);
+                    // compute when this periodic sync should next run
+                    long nextPollTimeAbsolute = lastPollTimeAbsolute + periodInSeconds * 1000;
+                    // if it is ready to run then schedule it and mark it as having been scheduled
+                    if (nextPollTimeAbsolute <= nowAbsolute) {
+                        scheduleSyncOperation(
+                                new SyncOperation(info.account, SyncStorageEngine.SOURCE_PERIODIC,
+                                        info.authority, extras, 0 /* delay */));
+                        status.setPeriodicSyncTime(i, nowAbsolute);
+                    } else {
+                        // it isn't ready to run, remember this time if it is earlier than
+                        // earliestFuturePollTime
+                        if (earliestFuturePollTime == null
+                                || nextPollTimeAbsolute < earliestFuturePollTime) {
+                            earliestFuturePollTime = nextPollTimeAbsolute;
+                        }
+                    }
+                }
             }
 
-            // skip the sync if it isn't manual and auto sync is disabled
+            if (earliestFuturePollTime == null) {
-            final boolean syncAutomatically =
+                return null;
-                    mSyncStorageEngine.getSyncAutomatically(account, authority)
-                            && mSyncStorageEngine.getMasterSyncAutomatically();
-            if (!(ignoreSettings || (backgroundDataUsageAllowed && syncAutomatically))) {
-                return false;
             }
 
-            if (mSyncStorageEngine.getIsSyncable(account, authority) <= 0) {
+            // convert absolute time to elapsed time
-                // if not syncable or if the syncable is unknown (< 0), don't allow
+            return SystemClock.elapsedRealtime()
-                return false;
+                    + ((earliestFuturePollTime < nowAbsolute)
-            }
+                      ? 0
-
+                      : (earliestFuturePollTime - nowAbsolute));
-            return true;
         }
 
         private void runStateSyncing() {
             // if the sync timeout has been reached then cancel it
-
             ActiveSyncContext activeSyncContext = mActiveSyncContext;
 
             final long now = SystemClock.elapsedRealtime();
             if (now > activeSyncContext.mTimeoutStartTime + MAX_TIME_PER_SYNC) {
-                SyncOperation nextSyncOperation;
+                Pair<SyncOperation, Long> nextOpAndRunTime;
                 synchronized (mSyncQueue) {
-                    nextSyncOperation = getNextReadyToRunSyncOperation(now);
+                    nextOpAndRunTime = mSyncQueue.nextOperation();
                 }
-                if (nextSyncOperation != null) {
+                SyncOperation curOp = activeSyncContext.mSyncOperation;
+                if (nextOpAndRunTime != null
+                        && nextOpAndRunTime.second <= now
+                        && !nextOpAndRunTime.first.account.equals(curOp.account)
+                        && !nextOpAndRunTime.first.authority.equals(curOp.authority)) {
                     Log.d(TAG, "canceling and rescheduling sync because it ran too long: "
                             + activeSyncContext.mSyncOperation);
                     scheduleSyncOperation(new SyncOperation(activeSyncContext.mSyncOperation));
@@ -1574,27 +1612,48 @@ public class SyncManager implements OnAccountsUpdateListener {
             // found that is runnable (not disabled, etc). If that one is ready to run then
             // start it, otherwise just get out.
             SyncOperation op;
+            int syncableState;
             final boolean backgroundDataUsageAllowed =
                     getConnectivityManager().getBackgroundDataSetting();
+            final boolean masterSyncAutomatically = mSyncStorageEngine.getMasterSyncAutomatically();
+
             synchronized (mSyncQueue) {
                 final long now = SystemClock.elapsedRealtime();
                 while (true) {
-                    op = getNextReadyToRunSyncOperation(now);
+                    Pair<SyncOperation, Long> nextOpAndRunTime = mSyncQueue.nextOperation();
-                    if (op == null) {
+                    if (nextOpAndRunTime == null || nextOpAndRunTime.second > now) {
                         if (isLoggable) {
-                            Log.v(TAG, "runStateIdle: no more sync operations, returning");
+                            Log.v(TAG, "runStateIdle: no more ready sync operations, returning");
                         }
                         return;
                     }
+                    op = nextOpAndRunTime.first;
 
-                    // we are either going to run this sync or drop it so go ahead and remove it
+                    // we are either going to run this sync or drop it so go ahead and
-                    // from the queue now
+                    // remove it from the queue now
                     mSyncQueue.remove(op);
 
-                    final boolean ignoreSettings = op.extras
+                    // drop the sync if the account of this operation no longer exists
-                            .getBoolean(ContentResolver.SYNC_EXTRAS_IGNORE_SETTINGS, false);
+                    if (!ArrayUtils.contains(mAccounts, op.account)) {
-                    if (!isSyncAllowed(op.account, op.authority, ignoreSettings,
+                        continue;
-                            backgroundDataUsageAllowed)) {
+                    }
+
+
+                    // drop this sync request if it isn't syncable, intializing the sync adapter
+                    // if the syncable state is set to "unknown"
+                    syncableState = mSyncStorageEngine.getIsSyncable(op.account, op.authority);
+                    if (syncableState == 0) {
+                        continue;
+                    }
+
+                    // skip the sync if it isn't manual and auto sync or
+                    // background data usage is disabled
+                    if (!op.extras.getBoolean(ContentResolver.SYNC_EXTRAS_IGNORE_SETTINGS, false)
+                            && (syncableState > 0)
+                            && (!masterSyncAutomatically
+                                || !backgroundDataUsageAllowed
+                                || !mSyncStorageEngine.getSyncAutomatically(
+                                       op.account, op.authority))) {
                         continue;
                     }
 
@@ -1608,6 +1667,19 @@ public class SyncManager implements OnAccountsUpdateListener {
                 }
             }
 
+            // convert the op into an initialization sync if the syncable state is "unknown" and
+            // op isn't already an initialization sync. If it is marked syncable then convert
+            // this into a regular sync
+            final boolean initializeIsSet =
+                    op.extras.getBoolean(ContentResolver.SYNC_EXTRAS_INITIALIZE, false);
+            if (syncableState < 0 && !initializeIsSet) {
+                op.extras.putBoolean(ContentResolver.SYNC_EXTRAS_INITIALIZE, true);
+                op = new SyncOperation(op);
+            } else if (syncableState > 0 && initializeIsSet) {
+                op.extras.putBoolean(ContentResolver.SYNC_EXTRAS_INITIALIZE, false);
+                op = new SyncOperation(op);
+            }
+
             // connect to the sync adapter
             SyncAdapterType syncAdapterType = SyncAdapterType.newKey(op.authority, op.account.type);
             RegisteredServicesCache.ServiceInfo<SyncAdapterType> syncAdapterInfo =
@@ -1643,74 +1715,6 @@ public class SyncManager implements OnAccountsUpdateListener {
             // MESSAGE_SERVICE_CONNECTED or MESSAGE_SERVICE_DISCONNECTED message
         }
 
-        private SyncOperation getNextPeriodicSyncOperation() {
-            final boolean backgroundDataUsageAllowed =
-                    getConnectivityManager().getBackgroundDataSetting();
-            SyncStorageEngine.AuthorityInfo best = null;
-            long bestPollTimeAbsolute = Long.MAX_VALUE;
-            Bundle bestExtras = null;
-            ArrayList<SyncStorageEngine.AuthorityInfo> infos = mSyncStorageEngine.getAuthorities();
-            for (SyncStorageEngine.AuthorityInfo info : infos) {
-                if (!isSyncAllowed(info.account, info.authority, false /* ignoreSettings */,
-                        backgroundDataUsageAllowed)) {
-                    continue;
-                }
-                SyncStatusInfo status = mSyncStorageEngine.getStatusByAccountAndAuthority(
-                        info.account, info.authority);
-                int i = 0;
-                for (Pair<Bundle, Long> periodicSync : info.periodicSyncs) {
-                    long lastPollTimeAbsolute = status != null ? status.getPeriodicSyncTime(i) : 0;
-                    final Bundle extras = periodicSync.first;
-                    final Long periodInSeconds = periodicSync.second;
-                    long nextPollTimeAbsolute = lastPollTimeAbsolute + periodInSeconds * 1000;
-                    if (nextPollTimeAbsolute < bestPollTimeAbsolute) {
-                        best = info;
-                        bestPollTimeAbsolute = nextPollTimeAbsolute;
-                        bestExtras = extras;
-                    }
-                    i++;
-                }
-            }
-
-            if (best == null) {
-                return null;
-            }
-
-            final long nowAbsolute = System.currentTimeMillis();
-            final SyncOperation syncOperation = new SyncOperation(best.account,
-                    SyncStorageEngine.SOURCE_PERIODIC,
-                    best.authority, bestExtras, 0 /* delay */);
-            syncOperation.earliestRunTime = SystemClock.elapsedRealtime()
-                    + (bestPollTimeAbsolute - nowAbsolute);
-            if (syncOperation.earliestRunTime < 0) {
-                syncOperation.earliestRunTime = 0;
-            }
-            return syncOperation;
-        }
-
-        public Pair<SyncOperation, Long> bestSyncOperationCandidate() {
-            Pair<SyncOperation, Long> nextOpAndRunTime = mSyncQueue.nextOperation();
-            SyncOperation nextOp = nextOpAndRunTime != null ? nextOpAndRunTime.first : null;
-            Long nextRunTime = nextOpAndRunTime != null ? nextOpAndRunTime.second : null;
-            SyncOperation pollOp = getNextPeriodicSyncOperation();
-            if (nextOp != null
-                    && (pollOp == null || nextOp.expedited
-                        || nextRunTime <= pollOp.earliestRunTime)) {
-                return nextOpAndRunTime;
-            } else if (pollOp != null) {
-                return Pair.create(pollOp, pollOp.earliestRunTime);
-            } else {
-                return null;
-            }
-        }
-
-        private SyncOperation getNextReadyToRunSyncOperation(long now) {
-            Pair<SyncOperation, Long> nextOpAndRunTime = bestSyncOperationCandidate();
-            return nextOpAndRunTime != null && nextOpAndRunTime.second <= now
-                    ? nextOpAndRunTime.first
-                    : null;
-        }
-
         private void runBoundToSyncAdapter(ISyncAdapter syncAdapter) {
             mActiveSyncContext.mSyncAdapter = syncAdapter;
             final SyncOperation syncOperation = mActiveSyncContext.mSyncOperation;
@@ -1757,6 +1761,15 @@ public class SyncManager implements OnAccountsUpdateListener {
                     downstreamActivity = 0;
                     upstreamActivity = 0;
                     clearBackoffSetting(syncOperation);
+                    // if this was an initialization sync and the sync adapter is now
+                    // marked syncable then reschedule the sync. The next time it runs it
+                    // will be made into a regular sync.
+                    if (syncOperation.extras.getBoolean(
+                                ContentResolver.SYNC_EXTRAS_INITIALIZE, false)
+                            && mSyncStorageEngine.getIsSyncable(
+                                syncOperation.account, syncOperation.authority) > 0) {
+                        scheduleSyncOperation(new SyncOperation(syncOperation));
+                    }
                 } else {
                     Log.d(TAG, "failed sync operation " + syncOperation + ", " + syncResult);
                     // the operation failed so increase the backoff time
@@ -1919,7 +1932,7 @@ public class SyncManager implements OnAccountsUpdateListener {
             }
         }
 
-        private void manageSyncAlarm() {
+        private void manageSyncAlarm(Long earliestFuturePollElapsedTime) {
             // in each of these cases the sync loop will be kicked, which will cause this
             // method to be called again
             if (!mDataConnectionIsConnected) return;
@@ -1935,8 +1948,16 @@ public class SyncManager implements OnAccountsUpdateListener {
             ActiveSyncContext activeSyncContext = mActiveSyncContext;
             if (activeSyncContext == null) {
                 synchronized (mSyncQueue) {
-                    Pair<SyncOperation, Long> candidate = bestSyncOperationCandidate();
+                    final Pair<SyncOperation, Long> candidate = mSyncQueue.nextOperation();
-                    alarmTime = candidate != null ? candidate.second : null;
+                    if (earliestFuturePollElapsedTime == null && candidate == null) {
+                        alarmTime = null;
+                    } else if (earliestFuturePollElapsedTime == null) {
+                        alarmTime = candidate.second;
+                    } else if (candidate == null) {
+                        alarmTime = earliestFuturePollElapsedTime;
+                    } else {
+                        alarmTime = Math.min(earliestFuturePollElapsedTime, candidate.second);
+                    }
                 }
             } else {
                 final long notificationTime =
@@ -2077,7 +2098,7 @@ public class SyncManager implements OnAccountsUpdateListener {
                                 SyncStorageEngine.EVENT_STOP, syncOperation.syncSource,
                                 syncOperation.account.name.hashCode());
 
-            mSyncStorageEngine.stopSyncEvent(rowId, syncOperation.extras, elapsedTime,
+            mSyncStorageEngine.stopSyncEvent(rowId, elapsedTime,
                     resultMessage, downstreamActivity, upstreamActivity);
         }
     }

core/java/android/content/SyncOperation.java

@@ -80,7 +80,7 @@ public class SyncOperation implements Comparable {
         sb.append("authority: ").append(authority);
         sb.append(" account: ").append(account);
         sb.append(" extras: ");
-        extrasToStringBuilder(extras, sb);
+        extrasToStringBuilder(extras, sb, false /* asKey */);
         sb.append(" syncSource: ").append(syncSource);
         sb.append(" when: ").append(earliestRunTime);
         sb.append(" expedited: ").append(expedited);
@@ -92,13 +92,19 @@ public class SyncOperation implements Comparable {
         sb.append("authority: ").append(authority);
         sb.append(" account: ").append(account);
         sb.append(" extras: ");
-        extrasToStringBuilder(extras, sb);
+        extrasToStringBuilder(extras, sb, true /* asKey */);
         return sb.toString();
     }
 
-    public static void extrasToStringBuilder(Bundle bundle, StringBuilder sb) {
+    public static void extrasToStringBuilder(Bundle bundle, StringBuilder sb, boolean asKey) {
         sb.append("[");
         for (String key : bundle.keySet()) {
+            // if we are writing this as a key don't consider whether this
+            // is an initialization sync or not when computing the key since
+            // we set this flag appropriately when dispatching the sync request.
+            if (asKey && ContentResolver.SYNC_EXTRAS_INITIALIZE.equals(key)) {
+                continue;
+            }
             sb.append(key).append("=").append(bundle.get(key)).append(" ");
         }
         sb.append("]");

core/java/android/content/SyncQueue.java

@@ -16,6 +16,7 @@
 
 package android.content;
 
+import com.google.android.collect.Lists;
 import com.google.android.collect.Maps;
 
 import android.util.Pair;
@@ -120,14 +121,16 @@ public class SyncQueue {
 
     /**
      * Find the operation that should run next. Operations are sorted by their earliestRunTime,
-     * prioritizing expedited operations. The earliestRunTime is adjusted by the sync adapter's
+     * prioritizing first those with a syncable state of "unknown" that aren't retries then
-     * backoff and delayUntil times, if any.
+     * expedited operations.
+     * The earliestRunTime is adjusted by the sync adapter's backoff and delayUntil times, if any.
      * @return the operation that should run next and when it should run. The time may be in
      * the future. It is expressed in milliseconds since boot.
      */
     public Pair<SyncOperation, Long> nextOperation() {
         SyncOperation best = null;
         long bestRunTime = 0;
+        boolean bestSyncableIsUnknownAndNotARetry = false;
         for (SyncOperation op : mOperationsMap.values()) {
             long opRunTime = op.earliestRunTime;
             if (!op.extras.getBoolean(ContentResolver.SYNC_EXTRAS_IGNORE_BACKOFF, false)) {
@@ -137,12 +140,23 @@ public class SyncQueue {
                         Math.max(opRunTime, delayUntil),
                         backoff != null ? backoff.first : 0);
             }
-            // if the expedited state of both ops are the same then compare their runtime.
+            // we know a sync is a retry if the intialization flag is set, since that will only
-            // Otherwise the candidate is only better than the current best if the candidate
+            // be set by the sync dispatching code, thus if it is set it must have already been
-            // is expedited.
+            // 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
-                    || (best.expedited == op.expedited ? opRunTime < bestRunTime : op.expedited)) {
+                    || ((bestSyncableIsUnknownAndNotARetry == syncableIsUnknownAndNotARetry)
+                        ? (best.expedited == op.expedited
+                           ? opRunTime < bestRunTime
+                           : op.expedited)
+                        : syncableIsUnknownAndNotARetry)) {
                 best = op;
+                bestSyncableIsUnknownAndNotARetry = syncableIsUnknownAndNotARetry;
                 bestRunTime = opRunTime;
             }
         }

core/java/android/content/SyncStorageEngine.java

@@ -59,7 +59,6 @@ import java.util.List;
  */
 public class SyncStorageEngine extends Handler {
     private static final String TAG = "SyncManager";
-    private static final boolean DEBUG = false;
     private static final boolean DEBUG_FILE = false;
 
     private static final long DEFAULT_POLL_FREQUENCY_SECONDS = 60 * 60 * 24; // One day
@@ -241,6 +240,8 @@ public class SyncStorageEngine extends Handler {
     private final RemoteCallbackList<ISyncStatusObserver> mChangeListeners
             = new RemoteCallbackList<ISyncStatusObserver>();
 
+    private int mNextAuthorityId = 0;
+
     // We keep 4 weeks of stats.
     private final DayStats[] mDayStats = new DayStats[7*4];
     private final Calendar mCal;
@@ -301,7 +302,11 @@ public class SyncStorageEngine extends Handler {
         readStatusLocked();
         readPendingOperationsLocked();
         readStatisticsLocked();
-        readLegacyAccountInfoLocked();
+        readAndDeleteLegacyAccountInfoLocked();
+        writeAccountInfoLocked();
+        writeStatusLocked();
+        writePendingOperationsLocked();
+        writeStatisticsLocked();
     }
 
     public static SyncStorageEngine newTestInstance(Context context) {
@@ -365,7 +370,9 @@ public class SyncStorageEngine extends Handler {
             mChangeListeners.finishBroadcast();
         }
 
-        if (DEBUG) Log.v(TAG, "reportChange " + which + " to: " + reports);
+        if (Log.isLoggable(TAG, Log.VERBOSE)) {
+            Log.v(TAG, "reportChange " + which + " to: " + reports);
+        }
 
         if (reports != null) {
             int i = reports.size();
@@ -402,15 +409,19 @@ public class SyncStorageEngine extends Handler {
     }
 
     public void setSyncAutomatically(Account account, String providerName, boolean sync) {
-        boolean wasEnabled;
+        Log.d(TAG, "setSyncAutomatically: " + account + ", provider " + providerName
+                + " -> " + sync);
         synchronized (mAuthorities) {
             AuthorityInfo authority = getOrCreateAuthorityLocked(account, providerName, -1, false);
-            wasEnabled = authority.enabled;
+            if (authority.enabled == sync) {
+                Log.d(TAG, "setSyncAutomatically: already set to " + sync + ", doing nothing");
+                return;
+            }
             authority.enabled = sync;
             writeAccountInfoLocked();
         }
 
-        if (!wasEnabled && sync) {
+        if (sync) {
             ContentResolver.requestSync(account, providerName, new Bundle());
         }
         reportChange(ContentResolver.SYNC_OBSERVER_TYPE_SETTINGS);
@@ -440,7 +451,6 @@ public class SyncStorageEngine extends Handler {
     }
 
     public void setIsSyncable(Account account, String providerName, int syncable) {
-        int oldState;
         if (syncable > 1) {
             syncable = 1;
         } else if (syncable < -1) {
@@ -449,12 +459,15 @@ public class SyncStorageEngine extends Handler {
         Log.d(TAG, "setIsSyncable: " + account + ", provider " + providerName + " -> " + syncable);
         synchronized (mAuthorities) {
             AuthorityInfo authority = getOrCreateAuthorityLocked(account, providerName, -1, false);
-            oldState = authority.syncable;
+            if (authority.syncable == syncable) {
+                Log.d(TAG, "setIsSyncable: already set to " + syncable + ", doing nothing");
+                return;
+            }
             authority.syncable = syncable;
             writeAccountInfoLocked();
         }
 
-        if (oldState <= 0 && syncable > 0) {
+        if (syncable > 0) {
             ContentResolver.requestSync(account, providerName, new Bundle());
         }
         reportChange(ContentResolver.SYNC_OBSERVER_TYPE_SETTINGS);
@@ -550,49 +563,60 @@ public class SyncStorageEngine extends Handler {
                     + " -> period " + period + ", extras " + extras);
         }
         synchronized (mAuthorities) {
-            AuthorityInfo authority = getOrCreateAuthorityLocked(account, providerName, -1, false);
+            try {
-            if (add) {
+                AuthorityInfo authority =
-                boolean alreadyPresent = false;
+                        getOrCreateAuthorityLocked(account, providerName, -1, false);
-                for (int i = 0, N = authority.periodicSyncs.size(); i < N; i++) {
+                if (add) {
-                    Pair<Bundle, Long> syncInfo = authority.periodicSyncs.get(i);
+                    // add this periodic sync if one with the same extras doesn't already
-                    final Bundle existingExtras = syncInfo.first;
+                    // exist in the periodicSyncs array
-                    if (equals(existingExtras, extras)) {
+                    boolean alreadyPresent = false;
-                        if (syncInfo.second == period) {
+                    for (int i = 0, N = authority.periodicSyncs.size(); i < N; i++) {
-                            return;
+                        Pair<Bundle, Long> syncInfo = authority.periodicSyncs.get(i);
+                        final Bundle existingExtras = syncInfo.first;
+                        if (equals(existingExtras, extras)) {
+                            if (syncInfo.second == period) {
+                                return;
+                            }
+                            authority.periodicSyncs.set(i, Pair.create(extras, period));
+                            alreadyPresent = true;
+                            break;
                         }
-                        authority.periodicSyncs.set(i, Pair.create(extras, period));
+                    }
-                        alreadyPresent = true;
+                    // if we added an entry to the periodicSyncs array also add an entry to
-                        break;
+                    // the periodic syncs status to correspond to it
+                    if (!alreadyPresent) {
+                        authority.periodicSyncs.add(Pair.create(extras, period));
+                        SyncStatusInfo status = getOrCreateSyncStatusLocked(authority.ident);
+                        status.setPeriodicSyncTime(authority.periodicSyncs.size() - 1, 0);
+                    }
+                } else {
+                    // remove any periodic syncs that match the authority and extras
+                    SyncStatusInfo status = mSyncStatus.get(authority.ident);
+                    boolean changed = false;
+                    Iterator<Pair<Bundle, Long>> iterator = authority.periodicSyncs.iterator();
+                    int i = 0;
+                    while (iterator.hasNext()) {
+                        Pair<Bundle, Long> syncInfo = iterator.next();
+                        if (equals(syncInfo.first, extras)) {
+                            iterator.remove();
+                            changed = true;
+                            // if we removed an entry from the periodicSyncs array also
+                            // remove the corresponding entry from the status
+                            if (status != null) {
+                                status.removePeriodicSyncTime(i);
+                            }
+                        } else {
+                            i++;
+                        }
+                    }
+                    if (!changed) {
+                        return;
                     }
                 }
-                if (!alreadyPresent) {
+            } finally {
-                    authority.periodicSyncs.add(Pair.create(extras, period));
+                writeAccountInfoLocked();
-                    SyncStatusInfo status = getOrCreateSyncStatusLocked(authority.ident);
+                writeStatusLocked();
-                    status.setPeriodicSyncTime(authority.periodicSyncs.size() - 1, 0);
-                }
-            } else {
-                SyncStatusInfo status = mSyncStatus.get(authority.ident);
-                boolean changed = false;
-                Iterator<Pair<Bundle, Long>> iterator = authority.periodicSyncs.iterator();
-                int i = 0;
-                while (iterator.hasNext()) {
-                    Pair<Bundle, Long> syncInfo = iterator.next();
-                    if (equals(syncInfo.first, extras)) {
-                        iterator.remove();
-                        changed = true;
-                        if (status != null) {
-                            status.removePeriodicSyncTime(i);
-                        }
-                    } else {
-                        i++;
-                    }
-                }
-                if (!changed) {
-                    return;
-                }
             }
-            writeAccountInfoLocked();
-            writeStatusLocked();
         }
 
         reportChange(ContentResolver.SYNC_OBSERVER_TYPE_SETTINGS);
@@ -622,13 +646,14 @@ public class SyncStorageEngine extends Handler {
     }
 
     public void setMasterSyncAutomatically(boolean flag) {
-        boolean old;
         synchronized (mAuthorities) {
-            old = mMasterSyncAutomatically;
+            if (mMasterSyncAutomatically == flag) {
+                return;
+            }
             mMasterSyncAutomatically = flag;
             writeAccountInfoLocked();
         }
-        if (!old && flag) {
+        if (flag) {
             ContentResolver.requestSync(null, null, new Bundle());
         }
         reportChange(ContentResolver.SYNC_OBSERVER_TYPE_SETTINGS);
@@ -651,7 +676,7 @@ public class SyncStorageEngine extends Handler {
 
     public void removeAuthority(Account account, String authority) {
         synchronized (mAuthorities) {
-            removeAuthorityLocked(account, authority);
+            removeAuthorityLocked(account, authority, true /* doWrite */);
         }
     }
 
@@ -692,10 +717,12 @@ public class SyncStorageEngine extends Handler {
 
     public PendingOperation insertIntoPending(PendingOperation op) {
         synchronized (mAuthorities) {
-            if (DEBUG) Log.v(TAG, "insertIntoPending: account=" + op.account
+            if (Log.isLoggable(TAG, Log.VERBOSE)) {
+                Log.v(TAG, "insertIntoPending: account=" + op.account
                     + " auth=" + op.authority
                     + " src=" + op.syncSource
                     + " extras=" + op.extras);
+            }
 
             AuthorityInfo authority = getOrCreateAuthorityLocked(op.account,
                     op.authority,
@@ -721,10 +748,12 @@ public class SyncStorageEngine extends Handler {
     public boolean deleteFromPending(PendingOperation op) {
         boolean res = false;
         synchronized (mAuthorities) {
-            if (DEBUG) Log.v(TAG, "deleteFromPending: account=" + op.account
+            if (Log.isLoggable(TAG, Log.VERBOSE)) {
+                Log.v(TAG, "deleteFromPending: account=" + op.account
                     + " auth=" + op.authority
                     + " src=" + op.syncSource
                     + " extras=" + op.extras);
+            }
             if (mPendingOperations.remove(op)) {
                 if (mPendingOperations.size() == 0
                         || mNumPendingFinished >= PENDING_FINISH_TO_WRITE) {
@@ -737,7 +766,7 @@ public class SyncStorageEngine extends Handler {
                 AuthorityInfo authority = getAuthorityLocked(op.account, op.authority,
                         "deleteFromPending");
                 if (authority != null) {
-                    if (DEBUG) Log.v(TAG, "removing - " + authority);
+                    if (Log.isLoggable(TAG, Log.VERBOSE)) Log.v(TAG, "removing - " + authority);
                     final int N = mPendingOperations.size();
                     boolean morePending = false;
                     for (int i=0; i<N; i++) {
@@ -750,7 +779,7 @@ public class SyncStorageEngine extends Handler {
                     }
 
                     if (!morePending) {
-                        if (DEBUG) Log.v(TAG, "no more pending!");
+                        if (Log.isLoggable(TAG, Log.VERBOSE)) Log.v(TAG, "no more pending!");
                         SyncStatusInfo status = getOrCreateSyncStatusLocked(authority.ident);
                         status.pending = false;
                     }
@@ -767,7 +796,9 @@ public class SyncStorageEngine extends Handler {
     public int clearPending() {
         int num;
         synchronized (mAuthorities) {
-            if (DEBUG) Log.v(TAG, "clearPending");
+            if (Log.isLoggable(TAG, Log.VERBOSE)) {
+                Log.v(TAG, "clearPending");
+            }
             num = mPendingOperations.size();
             mPendingOperations.clear();
             final int N = mSyncStatus.size();
@@ -806,14 +837,16 @@ public class SyncStorageEngine extends Handler {
      */
     public void doDatabaseCleanup(Account[] accounts) {
         synchronized (mAuthorities) {
-            if (DEBUG) Log.w(TAG, "Updating for new accounts...");
+            if (Log.isLoggable(TAG, Log.VERBOSE)) Log.w(TAG, "Updating for new accounts...");
             SparseArray<AuthorityInfo> removing = new SparseArray<AuthorityInfo>();
             Iterator<AccountInfo> accIt = mAccounts.values().iterator();
             while (accIt.hasNext()) {
                 AccountInfo acc = accIt.next();
                 if (!ArrayUtils.contains(accounts, acc.account)) {
                     // This account no longer exists...
-                    if (DEBUG) Log.w(TAG, "Account removed: " + acc.account);
+                    if (Log.isLoggable(TAG, Log.VERBOSE)) {
+                        Log.w(TAG, "Account removed: " + acc.account);
+                    }
                     for (AuthorityInfo auth : acc.authorities.values()) {
                         removing.put(auth.ident, auth);
                     }
@@ -859,11 +892,13 @@ public class SyncStorageEngine extends Handler {
     public void setActiveSync(SyncManager.ActiveSyncContext activeSyncContext) {
         synchronized (mAuthorities) {
             if (activeSyncContext != null) {
-                if (DEBUG) Log.v(TAG, "setActiveSync: account="
+                if (Log.isLoggable(TAG, Log.VERBOSE)) {
+                    Log.v(TAG, "setActiveSync: account="
                         + activeSyncContext.mSyncOperation.account
                         + " auth=" + activeSyncContext.mSyncOperation.authority
                         + " src=" + activeSyncContext.mSyncOperation.syncSource
                         + " extras=" + activeSyncContext.mSyncOperation.extras);
+                }
                 if (mCurrentSync != null) {
                     Log.w(TAG, "setActiveSync called with existing active sync!");
                 }
@@ -878,7 +913,7 @@ public class SyncStorageEngine extends Handler {
                         authority.account, authority.authority,
                         activeSyncContext.mStartTime);
             } else {
-                if (DEBUG) Log.v(TAG, "setActiveSync: null");
+                if (Log.isLoggable(TAG, Log.VERBOSE)) Log.v(TAG, "setActiveSync: null");
                 mCurrentSync = null;
             }
         }
@@ -900,8 +935,10 @@ public class SyncStorageEngine extends Handler {
             long now, int source) {
         long id;
         synchronized (mAuthorities) {
-            if (DEBUG) Log.v(TAG, "insertStartSyncEvent: account=" + accountName
+            if (Log.isLoggable(TAG, Log.VERBOSE)) {
+                Log.v(TAG, "insertStartSyncEvent: account=" + accountName
                     + " auth=" + authorityName + " source=" + source);
+            }
             AuthorityInfo authority = getAuthorityLocked(accountName, authorityName,
                     "insertStartSyncEvent");
             if (authority == null) {
@@ -919,7 +956,7 @@ public class SyncStorageEngine extends Handler {
                 mSyncHistory.remove(mSyncHistory.size()-1);
             }
             id = item.historyId;
-            if (DEBUG) Log.v(TAG, "returning historyId " + id);
+            if (Log.isLoggable(TAG, Log.VERBOSE)) Log.v(TAG, "returning historyId " + id);
         }
 
         reportChange(ContentResolver.SYNC_OBSERVER_TYPE_STATUS);
@@ -944,10 +981,12 @@ public class SyncStorageEngine extends Handler {
         return true;
     }
 
-    public void stopSyncEvent(long historyId, Bundle extras, long elapsedTime, String resultMessage,
+    public void stopSyncEvent(long historyId, long elapsedTime, String resultMessage,
             long downstreamActivity, long upstreamActivity) {
         synchronized (mAuthorities) {
-            if (DEBUG) Log.v(TAG, "stopSyncEvent: historyId=" + historyId);
+            if (Log.isLoggable(TAG, Log.VERBOSE)) {
+                Log.v(TAG, "stopSyncEvent: historyId=" + historyId);
+            }
             SyncHistoryItem item = null;
             int i = mSyncHistory.size();
             while (i > 0) {
@@ -989,14 +1028,6 @@ public class SyncStorageEngine extends Handler {
                     break;
                 case SOURCE_PERIODIC:
                     status.numSourcePeriodic++;
-                    AuthorityInfo authority = mAuthorities.get(item.authorityId);
-                    for (int periodicSyncIndex = 0;
-                            periodicSyncIndex < authority.periodicSyncs.size();
-                            periodicSyncIndex++) {
-                        if (equals(extras, authority.periodicSyncs.get(periodicSyncIndex).first)) {
-                            status.setPeriodicSyncTime(periodicSyncIndex, item.eventTime);
-                        }
-                    }
                     break;
             }
 
@@ -1261,18 +1292,14 @@ public class SyncStorageEngine extends Handler {
         AuthorityInfo authority = account.authorities.get(authorityName);
         if (authority == null) {
             if (ident < 0) {
-                // Look for a new identifier for this authority.
+                ident = mNextAuthorityId;
-                final int N = mAuthorities.size();
+                mNextAuthorityId++;
-                ident = 0;
+                doWrite = true;
-                for (int i=0; i<N; i++) {
-                    if (mAuthorities.valueAt(i).ident > ident) {
-                        break;
-                    }
-                    ident++;
-                }
             }
-            if (DEBUG) Log.v(TAG, "created a new AuthorityInfo for " + accountName
+            if (Log.isLoggable(TAG, Log.VERBOSE)) {
+                Log.v(TAG, "created a new AuthorityInfo for " + accountName
                     + ", provider " + authorityName);
+            }
             authority = new AuthorityInfo(accountName, authorityName, ident);
             account.authorities.put(authorityName, authority);
             mAuthorities.put(ident, authority);
@@ -1284,13 +1311,15 @@ public class SyncStorageEngine extends Handler {
         return authority;
     }
 
-    private void removeAuthorityLocked(Account account, String authorityName) {
+    private void removeAuthorityLocked(Account account, String authorityName, boolean doWrite) {
         AccountInfo accountInfo = mAccounts.get(account);
         if (accountInfo != null) {
             final AuthorityInfo authorityInfo = accountInfo.authorities.remove(authorityName);
             if (authorityInfo != null) {
                 mAuthorities.remove(authorityInfo.ident);
-                writeAccountInfoLocked();
+                if (doWrite) {
+                    writeAccountInfoLocked();
+                }
             }
         }
     }
@@ -1340,7 +1369,11 @@ public class SyncStorageEngine extends Handler {
             readStatusLocked();
             readPendingOperationsLocked();
             readStatisticsLocked();
-            readLegacyAccountInfoLocked();
+            readAndDeleteLegacyAccountInfoLocked();
+            writeAccountInfoLocked();
+            writeStatusLocked();
+            writePendingOperationsLocked();
+            writeStatisticsLocked();
         }
     }
 
@@ -1348,7 +1381,7 @@ public class SyncStorageEngine extends Handler {
      * Read all account information back in to the initial engine state.
      */
     private void readAccountInfoLocked() {
-        boolean writeNeeded = false;
+        int highestAuthorityId = -1;
         FileInputStream fis = null;
         try {
             fis = mAccountInfoFile.openRead();
@@ -1370,11 +1403,14 @@ public class SyncStorageEngine extends Handler {
                 } catch (NumberFormatException e) {
                     version = 0;
                 }
-                if (version < ACCOUNTS_VERSION) {
+                String nextIdString = parser.getAttributeValue(null, "nextAuthorityId");
-                    writeNeeded = true;
+                try {
+                    int id = (nextIdString == null) ? 0 : Integer.parseInt(nextIdString);
+                    mNextAuthorityId = Math.max(mNextAuthorityId, id);
+                } catch (NumberFormatException e) {
+                    // don't care
                 }
-                mMasterSyncAutomatically = listen == null
+                mMasterSyncAutomatically = listen == null || Boolean.parseBoolean(listen);
-                            || Boolean.parseBoolean(listen);
                 eventType = parser.next();
                 AuthorityInfo authority = null;
                 Pair<Bundle, Long> periodicSync = null;
@@ -1385,6 +1421,9 @@ public class SyncStorageEngine extends Handler {
                             if ("authority".equals(tagName)) {
                                 authority = parseAuthority(parser, version);
                                 periodicSync = null;
+                                if (authority.ident > highestAuthorityId) {
+                                    highestAuthorityId = authority.ident;
+                                }
                             }
                         } else if (parser.getDepth() == 3) {
                             if ("periodicSync".equals(tagName) && authority != null) {
@@ -1407,6 +1446,7 @@ public class SyncStorageEngine extends Handler {
             else Log.w(TAG, "Error reading accounts", e);
             return;
         } finally {
+            mNextAuthorityId = Math.max(highestAuthorityId + 1, mNextAuthorityId);
             if (fis != null) {
                 try {
                     fis.close();
@@ -1415,13 +1455,7 @@ public class SyncStorageEngine extends Handler {
             }
         }
 
-        if (maybeMigrateSettingsForRenamedAuthorities()) {
+        maybeMigrateSettingsForRenamedAuthorities();
-            writeNeeded = true;
-        }
-
-        if (writeNeeded) {
-            writeAccountInfoLocked();
-        }
     }
 
     /**
@@ -1463,7 +1497,8 @@ public class SyncStorageEngine extends Handler {
         }
 
         for (AuthorityInfo authorityInfo : authoritiesToRemove) {
-            removeAuthorityLocked(authorityInfo.account, authorityInfo.authority);
+            removeAuthorityLocked(authorityInfo.account, authorityInfo.authority,
+                    false /* doWrite */);
             writeNeeded = true;
         }
 
@@ -1593,6 +1628,7 @@ public class SyncStorageEngine extends Handler {
 
             out.startTag(null, "accounts");
             out.attribute(null, "version", Integer.toString(ACCOUNTS_VERSION));
+            out.attribute(null, "nextAuthorityId", Integer.toString(mNextAuthorityId));
             if (!mMasterSyncAutomatically) {
                 out.attribute(null, "listen-for-tickles", "false");
             }
@@ -1675,7 +1711,7 @@ public class SyncStorageEngine extends Handler {
      * erase it.  Note that we don't deal with pending operations, active
      * sync, or history.
      */
-    private void readLegacyAccountInfoLocked() {
+    private void readAndDeleteLegacyAccountInfoLocked() {
         // Look for old database to initialize from.
         File file = mContext.getDatabasePath("syncmanager.db");
         if (!file.exists()) {
@@ -1792,8 +1828,6 @@ public class SyncStorageEngine extends Handler {
 
             db.close();
 
-            writeAccountInfoLocked();
-            writeStatusLocked();
             (new File(path)).delete();
         }
     }

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

@@ -51,7 +51,7 @@ public class SyncStorageEngineTest extends AndroidTestCase {
         long historyId = engine.insertStartSyncEvent(
                 account, authority, time0, SyncStorageEngine.SOURCE_LOCAL);
         long time1 = time0 + SyncStorageEngine.MILLIS_IN_4WEEKS * 2;
-        engine.stopSyncEvent(historyId, new Bundle(), time1 - time0, "yay", 0, 0);
+        engine.stopSyncEvent(historyId, time1 - time0, "yay", 0, 0);
     }
 
     /**