MD Settings: [further] robustify default zoom level handling

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 49f9fc1a5fdec41fe40e61283174fa23907823fa..690888a112d7da597a8c386ca1e03c0197f6f486 100644
--- a/chrome/browser/resources/settings/appearance_page/appearance_page.js
+++ b/chrome/browser/resources/settings/appearance_page/appearance_page.js
@@ -20,19 +20,22 @@ Polymer({
   behaviors: [I18nBehavior],
 
   properties: {
-    /** @private {!settings.AppearanceBrowserProxy} */
-    browserProxy_: Object,
+    /**
+     * Dictionary defining page visibility.
+     * @type {!AppearancePageVisibility}
+     */
+    pageVisibility: Object,
 
     prefs: {
       type: Object,
       notify: true,
     },
 
+    /** @private {!settings.AppearanceBrowserProxy} */
+    browserProxy_: Object,

[dpapad, 2016/11/10 02:20:49]

Nit: I recall that we are OK not putting browserProxy_ objects inside properties
now.

[Dan Beam, 2016/11/10 02:25:29]

done in next patch

+
     /** @private */
-    useSystemTheme_: {
-      type: Boolean,
-      value: false,  // Can only be true on Linux, but value exists everywhere.
-    },
+    defaultZoom_: Number,
 
     /**
      * List of options for the font size drop-down menu.
@@ -84,11 +87,11 @@ Polymer({
     /** @private */
     themeSublabel_: String,
 
-    /**
-     * Dictionary defining page visibility.
-     * @type {!AppearancePageVisibility}
-     */
-    pageVisibility: Object,
+    /** @private */
+    useSystemTheme_: {
+      type: Boolean,
+      value: false,  // Can only be true on Linux, but value exists everywhere.
+    },
   },
 
   /** @private {string} */
@@ -112,7 +115,7 @@ Polymer({
     // TODO(dschuyler): Look into adding a listener for the
     // default zoom percent.
     chrome.settingsPrivate.getDefaultZoom(function(zoom) {
-      this.$.zoomLevel.value = zoom;
+      this.defaultZoom_ = zoom;
     }.bind(this));
   },
 
@@ -249,5 +252,16 @@ Polymer({
    */
   getFirst_: function(bookmarksBarVisible) {
     return !bookmarksBarVisible ? 'first' : '';
-  }
+  },
+
+  /**
+   * @see content::ZoomValuesEqual().
+   * @param {number} zoom1
+   * @param {number} zoom2
+   * @return {boolean}
+   * @private
+   */
+  zoomValuesEqual_: function(zoom1, zoom2) {

[dpapad, 2016/11/10 02:20:49]

Nit (optional): I think only passing one param in this function, and using
this.zoomLevel_ internally instead of 2nd param, makes it clearer that this
binding triggers when/if this.zoomLevel_ changes (even without looking at the
HTML).

[Dan Beam, 2016/11/10 02:25:29]

ehhhhhhhhhhhh, but then it's less like a straight-up port of
content::ZoomValuesEqual().

honestly, most people need to look at the HTML to be sure either way (because
that's where the computed binding that invokes this lives anyways)

+    return Math.abs(zoom1 - zoom2) <= 0.001;
+  },
 });