MD Settings: [further] robustify default zoom level handling

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

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 /**
  * 'settings-appearance-page' is the settings page containing appearance
  * settings.
  *
  * Example:
  *
  *    <iron-animated-pages>
  *      <settings-appearance-page prefs="{{prefs}}">
  *      </settings-appearance-page>
  *      ... other pages ...
  *    </iron-animated-pages>
  */
 Polymer({
   is: 'settings-appearance-page',
 
   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.
      * @type {!DropdownMenuOptionList}
      */
     fontSizeOptions_: {
       readOnly: true,
       type: Array,
       value: function() {
         return [
@@ skipping 31 matching lines @@
         5 / 2,
         3,
         4,
         5,
       ],
     },
 
     /** @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} */
   themeUrl_: '',
 
   observers: [
     'themeChanged_(prefs.extensions.theme.id.value, useSystemTheme_)',
 
 <if expr="is_linux and not chromeos">
     // NOTE: this pref only exists on Linux.
     'useSystemThemePrefChanged_(prefs.extensions.theme.use_system.value)',
 </if>
   ],
 
   created: function() {
     this.browserProxy_ = settings.AppearanceBrowserProxyImpl.getInstance();
   },
 
   ready: function() {
     this.$.defaultFontSize.menuOptions = this.fontSizeOptions_;
     // 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));
   },
 
   /**
    * @param {number} zoom
    * @return {number} A zoom easier read by users.
    * @private
    */
   formatZoom_: function(zoom) {
     return Math.round(zoom * 100);
@@ skipping 116 matching lines @@
     chrome.settingsPrivate.setDefaultZoom(parseFloat(this.$.zoomLevel.value));
   },
 
   /**
    * @param {boolean} bookmarksBarVisible if bookmarks bar option is visible.
    * @return {string} 'first' if the argument is false or empty otherwise.
    * @private
    */
   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;
+  },
 });