Merge "Make sync settings restore more robust" into mnc-dev

3 files changed, 100 insertions, 87 deletions

diff --git a/core/java/com/android/server/backup/AccountSyncSettingsBackupHelper.java b/core/java/com/android/server/backup/AccountSyncSettingsBackupHelper.java
index 3f4b980..c0215a8 100644
--- a/core/java/com/android/server/backup/AccountSyncSettingsBackupHelper.java
+++ b/core/java/com/android/server/backup/AccountSyncSettingsBackupHelper.java
@@ -28,8 +28,6 @@ import android.app.backup.BackupHelper;
 import android.content.ContentResolver;
 import android.content.Context;
 import android.content.SyncAdapterType;
-import android.content.SyncStatusObserver;
-import android.os.Bundle;
 import android.os.ParcelFileDescriptor;
 import android.util.Log;
 
@@ -47,8 +45,6 @@ import java.util.Arrays;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
-import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.TimeUnit;
 
 /**
  * Helper for backing up account sync settings (whether or not a service should be synced). The
@@ -270,6 +266,10 @@ public class AccountSyncSettingsBackupHelper implements BackupHelper {
                     // yet won't be restored.
                     if (currentAccounts.contains(account)) {
                         restoreExistingAccountSyncSettingsFromJSON(accountJSON);
+                    } else {
+                        // TODO:
+                        // Stash the data to a file that the SyncManager can read from to restore
+                        // settings at a later date.
                     }
                 }
             } finally {
@@ -300,6 +300,31 @@ public class AccountSyncSettingsBackupHelper implements BackupHelper {
     /**
      * Restore account sync settings using the given JSON. This function won't work if the account
      * doesn't exist yet.
+     * This function will only be called during Setup Wizard, where we are guaranteed that there
+     * are no active syncs.
+     * There are 2 pieces of data to restore -
+     *      isSyncable (corresponds to {@link ContentResolver#getIsSyncable(Account, String)}
+     *      syncEnabled (corresponds to {@link ContentResolver#getSyncAutomatically(Account, String)}
+     * <strong>The restore favours adapters that were enabled on the old device, and doesn't care
+     * about adapters that were disabled.</strong>
+     *
+     * syncEnabled=true in restore data.
+     * syncEnabled will be true on this device. isSyncable will be left as the default in order to
+     * give the enabled adapter the chance to run an initialization sync.
+     *
+     * syncEnabled=false in restore data.
+     * syncEnabled will be false on this device. isSyncable will be set to 2, unless it was 0 on the
+     * old device in which case it will be set to 0 on this device. This is because isSyncable=0 is
+     * a rare state and was probably set to 0 for good reason (historically isSyncable is a way by
+     * which adapters control their own sync state independently of sync settings which is
+     * toggleable by the user).
+     * isSyncable=2 is a new isSyncable state we introduced specifically to allow adapters that are
+     * disabled after a restore to run initialization logic when the adapter is later enabled.
+     * See com.android.server.content.SyncStorageEngine#setSyncAutomatically
+     *
+     * The end result is that an adapter that the user had on will be turned on and get an
+     * initialization sync, while an adapter that the user had off will be off until the user
+     * enables it on this device at which point it will get an initialization sync.
      */
     private void restoreExistingAccountSyncSettingsFromJSON(JSONObject accountJSON)
             throws JSONException {
@@ -307,72 +332,27 @@ public class AccountSyncSettingsBackupHelper implements BackupHelper {
         JSONArray authorities = accountJSON.getJSONArray(KEY_ACCOUNT_AUTHORITIES);
         String accountName = accountJSON.getString(KEY_ACCOUNT_NAME);
         String accountType = accountJSON.getString(KEY_ACCOUNT_TYPE);
+
         final Account account = new Account(accountName, accountType);
         for (int i = 0; i < authorities.length(); i++) {
             JSONObject authority = (JSONObject) authorities.get(i);
             final String authorityName = authority.getString(KEY_AUTHORITY_NAME);
-            boolean syncEnabled = authority.getBoolean(KEY_AUTHORITY_SYNC_ENABLED);
-
-            // Cancel any active syncs.
-            if (ContentResolver.isSyncActive(account, authorityName)) {
-                ContentResolver.cancelSync(account, authorityName);
-            }
-
-            boolean overwriteSync = true;
-            Bundle initializationExtras = createSyncInitializationBundle();
-            int currentSyncState = ContentResolver.getIsSyncable(account, authorityName);
-            if (currentSyncState < 0) {
-                // Requesting a sync is an asynchronous operation, so we setup a countdown latch to
-                // wait for it to finish. Initialization syncs are generally very brief and
-                // shouldn't take too much time to finish.
-                final CountDownLatch latch = new CountDownLatch(1);
-                Object syncStatusObserverHandle = ContentResolver.addStatusChangeListener(
-                        ContentResolver.SYNC_OBSERVER_TYPE_ACTIVE, new SyncStatusObserver() {
-                            @Override
-                            public void onStatusChanged(int which) {
-                                if (!ContentResolver.isSyncActive(account, authorityName)) {
-                                    latch.countDown();
-                                }
-                            }
-                        });
-
-                // If we set sync settings for a sync that hasn't been initialized yet, we run the
-                // risk of having our changes overwritten later on when the sync gets initialized.
-                // To prevent this from happening we will manually initiate the sync adapter. We
-                // also explicitly pass in a Bundle with SYNC_EXTRAS_INITIALIZE to prevent a data
-                // sync from running after the initialization sync. Two syncs will be scheduled, but
-                // the second one (data sync) will override the first one (initialization sync) and
-                // still behave as an initialization sync because of the Bundle.
-                ContentResolver.requestSync(account, authorityName, initializationExtras);
-
-                boolean done = false;
-                try {
-                    done = latch.await(SYNC_REQUEST_LATCH_TIMEOUT_SECONDS, TimeUnit.SECONDS);
-                } catch (InterruptedException e) {
-                    Log.e(TAG, "CountDownLatch interrupted\n" + e);
-                    done = false;
-                }
-                if (!done) {
-                    overwriteSync = false;
-                    Log.i(TAG, "CountDownLatch timed out, skipping '" + authorityName
-                            + "' authority.");
-                }
-                ContentResolver.removeStatusChangeListener(syncStatusObserverHandle);
-            }
-
-            if (overwriteSync) {
-                ContentResolver.setSyncAutomatically(account, authorityName, syncEnabled);
-                Log.i(TAG, "Set sync automatically for '" + authorityName + "': " + syncEnabled);
+            boolean wasSyncEnabled = authority.getBoolean(KEY_AUTHORITY_SYNC_ENABLED);
+            int wasSyncable = authority.getInt(KEY_AUTHORITY_SYNC_STATE);
+
+            ContentResolver.setSyncAutomaticallyAsUser(
+                    account, authorityName, wasSyncEnabled, 0 /* user Id */);
+
+            if (!wasSyncEnabled) {
+                ContentResolver.setIsSyncable(
+                        account,
+                        authorityName,
+                        wasSyncable == 0 ?
+                                0 /* not syncable */ : 2 /* syncable but needs initialization */);
             }
         }
     }
 
-    private Bundle createSyncInitializationBundle() {
-        Bundle extras = new Bundle();
-        extras.putBoolean(ContentResolver.SYNC_EXTRAS_INITIALIZE, true);
-        return extras;
-    }
-
     @Override
     public void writeNewStateDescription(ParcelFileDescriptor newState) {
 
diff --git a/services/core/java/com/android/server/content/SyncManager.java b/services/core/java/com/android/server/content/SyncManager.java
index f222dba..2eb8797 100644
--- a/services/core/java/com/android/server/content/SyncManager.java
+++ b/services/core/java/com/android/server/content/SyncManager.java
@@ -801,7 +801,7 @@ public class SyncManager {
             for (String authority : syncableAuthorities) {
                 int isSyncable = getIsSyncable(account.account, account.userId,
                         authority);
-                if (isSyncable == 0) {
+                if (isSyncable == AuthorityInfo.NOT_SYNCABLE) {
                     continue;
                 }
                 final RegisteredServicesCache.ServiceInfo<SyncAdapterType> syncAdapterInfo;
@@ -813,8 +813,9 @@ public class SyncManager {
                 final boolean allowParallelSyncs = syncAdapterInfo.type.allowParallelSyncs();
                 final boolean isAlwaysSyncable = syncAdapterInfo.type.isAlwaysSyncable();
                 if (isSyncable < 0 && isAlwaysSyncable) {
-                    mSyncStorageEngine.setIsSyncable(account.account, account.userId, authority, 1);
-                    isSyncable = 1;
+                    mSyncStorageEngine.setIsSyncable(
+                            account.account, account.userId, authority, AuthorityInfo.SYNCABLE);
+                    isSyncable = AuthorityInfo.SYNCABLE;
                 }
                 if (onlyThoseWithUnkownSyncableState && isSyncable >= 0) {
                     continue;
diff --git a/services/core/java/com/android/server/content/SyncStorageEngine.java b/services/core/java/com/android/server/content/SyncStorageEngine.java
index d68b615..cca0c16 100644
--- a/services/core/java/com/android/server/content/SyncStorageEngine.java
+++ b/services/core/java/com/android/server/content/SyncStorageEngine.java
@@ -301,6 +301,30 @@ public class SyncStorageEngine extends Handler {
     }
 
     public static class AuthorityInfo {
+        // Legal values of getIsSyncable
+        /**
+         * Default state for a newly installed adapter. An uninitialized adapter will receive an
+         * initialization sync which are governed by a different set of rules to that of regular
+         * syncs.
+         */
+        public static final int NOT_INITIALIZED = -1;
+        /**
+         * The adapter will not receive any syncs. This is behaviourally equivalent to
+         * setSyncAutomatically -> false. However setSyncAutomatically is surfaced to the user
+         * while this is generally meant to be controlled by the developer.
+         */
+        public static final int NOT_SYNCABLE = 0;
+        /**
+         * The adapter is initialized and functioning. This is the normal state for an adapter.
+         */
+        public static final int SYNCABLE = 1;
+        /**
+         * The adapter is syncable but still requires an initialization sync. For example an adapter
+         * than has been restored from a previous device will be in this state. Not meant for
+         * external use.
+         */
+        public static final int SYNCABLE_NOT_INITIALIZED = 2;
+
         final EndPoint target;
         final int ident;
         boolean enabled;
@@ -349,12 +373,11 @@ public class SyncStorageEngine extends Handler {
         }
 
         private void defaultInitialisation() {
-            syncable = -1; // default to "unknown"
+            syncable = NOT_INITIALIZED; // default to "unknown"
             backoffTime = -1; // if < 0 then we aren't in backoff mode
             backoffDelay = -1; // if < 0 then we aren't in backoff mode
             PeriodicSync defaultSync;
-            // Old version is one sync a day. Empty bundle gets replaced by any addPeriodicSync()
-            // call.
+            // Old version is one sync a day.
             if (target.target_provider) {
                 defaultSync =
                         new PeriodicSync(target.account, target.provider,
@@ -663,6 +686,12 @@ public class SyncStorageEngine extends Handler {
                 }
                 return;
             }
+            // If the adapter was syncable but missing its initialization sync, set it to
+            // uninitialized now. This is to give it a chance to run any one-time initialization
+            // logic.
+            if (sync && authority.syncable == AuthorityInfo.SYNCABLE_NOT_INITIALIZED) {
+                authority.syncable = AuthorityInfo.NOT_INITIALIZED;
+            }
             authority.enabled = sync;
             writeAccountInfoLocked();
         }
@@ -682,7 +711,7 @@ public class SyncStorageEngine extends Handler {
                         new EndPoint(account, providerName, userId),
                         "get authority syncable");
                 if (authority == null) {
-                    return -1;
+                    return AuthorityInfo.NOT_INITIALIZED;
                 }
                 return authority.syncable;
             }
@@ -696,7 +725,7 @@ public class SyncStorageEngine extends Handler {
                     return authorityInfo.syncable;
                 }
             }
-            return -1;
+            return AuthorityInfo.NOT_INITIALIZED;
         }
     }
 
@@ -720,7 +749,8 @@ public class SyncStorageEngine extends Handler {
     }
 
     public void setIsTargetServiceActive(ComponentName cname, int userId, boolean active) {
-        setSyncableStateForEndPoint(new EndPoint(cname, userId), active ? 1 : 0);
+        setSyncableStateForEndPoint(new EndPoint(cname, userId), active ?
+                AuthorityInfo.SYNCABLE : AuthorityInfo.NOT_SYNCABLE);
     }
 
     /**
@@ -733,10 +763,8 @@ public class SyncStorageEngine extends Handler {
         AuthorityInfo aInfo;
         synchronized (mAuthorities) {
             aInfo = getOrCreateAuthorityLocked(target, -1, false);
-            if (syncable > 1) {
-                syncable = 1;
-            } else if (syncable < -1) {
-                syncable = -1;
+            if (syncable < AuthorityInfo.NOT_INITIALIZED) {
+                syncable = AuthorityInfo.NOT_INITIALIZED;
             }
             if (Log.isLoggable(TAG, Log.VERBOSE)) {
                 Log.d(TAG, "setIsSyncable: " + aInfo.toString() + " -> " + syncable);
@@ -750,7 +778,7 @@ public class SyncStorageEngine extends Handler {
             aInfo.syncable = syncable;
             writeAccountInfoLocked();
         }
-        if (syncable > 0) {
+        if (syncable == AuthorityInfo.SYNCABLE) {
             requestSync(aInfo, SyncOperation.REASON_IS_SYNCABLE, new Bundle());
         }
         reportChange(ContentResolver.SYNC_OBSERVER_TYPE_SETTINGS);
@@ -2012,7 +2040,7 @@ public class SyncStorageEngine extends Handler {
             int userId = user == null ? 0 : Integer.parseInt(user);
             if (accountType == null && packageName == null) {
                 accountType = "com.google";
-                syncable = "unknown";
+                syncable = String.valueOf(AuthorityInfo.NOT_INITIALIZED);
             }
             authority = mAuthorities.get(id);
             if (Log.isLoggable(TAG_FILE, Log.VERBOSE)) {
@@ -2052,11 +2080,19 @@ public class SyncStorageEngine extends Handler {
             }
             if (authority != null) {
                 authority.enabled = enabled == null || Boolean.parseBoolean(enabled);
-                if ("unknown".equals(syncable)) {
-                    authority.syncable = -1;
-                } else {
-                    authority.syncable =
-                            (syncable == null || Boolean.parseBoolean(syncable)) ? 1 : 0;
+                try {
+                    authority.syncable = (syncable == null) ?
+                            AuthorityInfo.NOT_INITIALIZED : Integer.parseInt(syncable);
+                } catch (NumberFormatException e) {
+                    // On L we stored this as {"unknown", "true", "false"} so fall back to this
+                    // format.
+                    if ("unknown".equals(syncable)) {
+                        authority.syncable = AuthorityInfo.NOT_INITIALIZED;
+                    } else {
+                        authority.syncable = Boolean.parseBoolean(syncable) ?
+                                AuthorityInfo.SYNCABLE : AuthorityInfo.NOT_SYNCABLE;
+                    }
+
                 }
             } else {
                 Log.w(TAG, "Failure adding authority: account="
@@ -2190,11 +2226,7 @@ public class SyncStorageEngine extends Handler {
                     out.attribute(null, "package", info.service.getPackageName());
                     out.attribute(null, "class", info.service.getClassName());
                 }
-                if (authority.syncable < 0) {
-                    out.attribute(null, "syncable", "unknown");
-                } else {
-                    out.attribute(null, "syncable", Boolean.toString(authority.syncable != 0));
-                }
+                out.attribute(null, "syncable", Integer.toString(authority.syncable));
                 for (PeriodicSync periodicSync : authority.periodicSyncs) {
                     out.startTag(null, "periodicSync");
                     out.attribute(null, "period", Long.toString(periodicSync.period));