Settings People Revamp: Cache the selected Sync individual data types.

chrome/browser/resources/settings/people_page/sync_page.js

Index: chrome/browser/resources/settings/people_page/sync_page.js
diff --git a/chrome/browser/resources/settings/people_page/sync_page.js b/chrome/browser/resources/settings/people_page/sync_page.js
index c50b263903c447012519667dfd7d8f6da42fe7ef..73d073f4e0c8a70dfdad3587fe873f44a7c82e5a 100644
--- a/chrome/browser/resources/settings/people_page/sync_page.js
+++ b/chrome/browser/resources/settings/people_page/sync_page.js
@@ -67,6 +67,14 @@ Polymer({
     },
 
     /**
+     * Caches the individually selected synced data types. This is used to
+     * be able to restore the selections after checking and unchecking Sync All.

[dpapad, 2016/06/28 22:23:32]

@private

[tommycli, 2016/06/29 00:04:07]

Done.

+     */
+    cachedSyncPrefs_: {
+      type: Object,
+    },
+
+    /**
      * Whether the "create passphrase" inputs should be shown. These inputs
      * give the user the opportunity to use a custom passphrase instead of
      * authenticating with his Google credentials.
@@ -188,18 +196,34 @@ Polymer({
    * @private
    */
   onSyncAllDataTypesChanged_: function(event) {
+    var SyncPrefsIndividualDataTypes = [

[dpapad, 2016/06/28 22:23:33]

Can this array be moved outside this function as a local constant to the
surrounding function scope? Seems unnecessary to redefine this array every time
onSyncAllataTypesChanged_ executes.

[tommycli, 2016/06/29 00:04:07]

Done.

+      'appsSynced',
+      'extensionsSynced',
+      'preferencesSynced',
+      'autofillSynced',
+      'typedUrlsSynced',
+      'themesSynced',
+      'bookmarksSynced',
+      'passwordsSynced',
+      'tabsSynced',
+      'paymentsIntegrationEnabled',
+    ];
+
     if (event.target.checked) {
       this.set('syncPrefs.syncAllDataTypes', true);
-      this.set('syncPrefs.appsSynced', true);
-      this.set('syncPrefs.extensionsSynced', true);
-      this.set('syncPrefs.preferencesSynced', true);
-      this.set('syncPrefs.autofillSynced', true);
-      this.set('syncPrefs.typedUrlsSynced', true);
-      this.set('syncPrefs.themesSynced', true);
-      this.set('syncPrefs.bookmarksSynced', true);
-      this.set('syncPrefs.passwordsSynced', true);
-      this.set('syncPrefs.tabsSynced', true);
-      this.set('syncPrefs.paymentsIntegrationEnabled', true);
+
+      // Cache the previously selected preference before checking every box.
+      this.cachedSyncPrefs_ = {};
+      for (dataType of SyncPrefsIndividualDataTypes) {
+        this.cachedSyncPrefs_[dataType] = this.syncPrefs[dataType];

[dpapad, 2016/06/28 22:23:33]

What is the key/value pair of syncPrefs look like? Is this shallow copy
sufficient? If some of the values are complex objects themselves, then the
cachedSyncPrefs and syncPrefs will hold references to the exact same object. If
all values are primitives, then let's add a comment explaining that the shallow
copy to store prefs is good enough.

[tommycli, 2016/06/29 00:04:07]

Done. I added a comment.

+
+        this.set(['syncPrefs', dataType], true);
+      }
+    } else if (this.cachedSyncPrefs_) {
+      // Restore the previously selected preference.
+      for (dataType of SyncPrefsIndividualDataTypes) {
+        this.set(['syncPrefs', dataType], this.cachedSyncPrefs_[dataType]);
+      }
     }
 
     this.onSingleSyncDataTypeChanged_();