MD Settings: Stop calling chrome.settingsPrivate.setDefaultZoomLevel on startup.

chrome/browser/resources/settings/appearance_page/appearance_page.js

Index: chrome/browser/resources/settings/appearance_page/appearance_page.js
diff --git a/chrome/browser/resources/settings/appearance_page/appearance_page.js b/chrome/browser/resources/settings/appearance_page/appearance_page.js
index 5b7462bd9315bb65beb987e947d9e0919873dba2..82ee396eee94aaf148a118abea317b99da6aa315 100644
--- a/chrome/browser/resources/settings/appearance_page/appearance_page.js
+++ b/chrome/browser/resources/settings/appearance_page/appearance_page.js
@@ -31,26 +31,29 @@ Polymer({
       notify: true,
     },
 
-    /**
-     * @private
-     */
+    /** @private */
     allowResetTheme_: {
       notify: true,
       type: Boolean,
       value: false,
     },
 
-    /**
-     * @private
-     */
+    /** @private */
     defaultZoomLevel_: {
       notify: true,
       type: Object,
-      value: function() {
-        return {
-          type: chrome.settingsPrivate.PrefType.NUMBER,
-        };
-      },
+      value: {type: chrome.settingsPrivate.PrefType.NUMBER},

[Dan Beam, 2016/10/11 19:03:23]

can we just omit the default value and use:

defaultZoomLevel_: {
  ...
  observer: 'zoomLevelChanged_',
},

and

zoomLevelChanged_: function(newZoomLevel, oldZoomLevel) {
  if (!oldZoomLevel)  { // or === undefined or something
    // Ignore the first time this is called.
    return;
  }
}

[dpapad, 2016/10/11 19:33:06]

Unfortunately that only works when you are setting the entire object, but not
when setting defaultZoomLevel.value directly (which is what the
settings-dropdown-menu does).

[Dan Beam, 2016/10/11 19:34:48]

and the problem with that is what?

[Dan Beam, 2016/10/11 19:35:50]

i.e. why not just change this code:

[dpapad, 2016/10/11 19:43:24]

The problem is that settings-dropdown-menu does not do that, so when you change
the dropdown option with the mouse, nothing happens (no observer fires), see 

https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/contro....

Perhaps I am misunderstanding the suggestion? FWIW, I tried it before
responding. Yes, it fixed the issue of not calling chrome.settingsPrivate on
startup, at the expense of never calling it (even when user changes the value).

[Dan Beam, 2016/10/12 23:56:20]

does this work?

this.defaultZoomLevel_ = {...};
this.notifyPath('defaultZoomLevel_.value');

+    },
+
+    /**
+     * Needed such that the |defaultZoomLevel_| pref can be initialized
+     * without unnecessarily invoking
+     * chrome.settingsPrivate.setDefaultZoomPercent().
+     * @private
+     */
+    defaultZoomLevelInitialized_: {

[Dan Beam, 2016/10/11 19:03:23]

fyi, some polymer peeps asked me to use vanilla prototype properties instead of
properties: {} recently, so I suppose we should start doing that for things we
don't observe

[dpapad, 2016/10/11 19:33:06]

Done. I was thinking about this myself.

+      type: Boolean,
+      value: false,
     },
 
     /**
@@ -213,6 +216,14 @@ Polymer({
     // The |percent| may be undefined on startup.
     if (percent === undefined)
       return;
+
+    if (!this.defaultZoomLevelInitialized_) {
+      // Simply mark this pref as initialized, no actual change was requested by
+      // the user.
+      this.defaultZoomLevelInitialized_ = true;
+      return;
+    }
+
     chrome.settingsPrivate.setDefaultZoomPercent(percent);
   },