MD Settings: handle zoom as floats to fix ghost zoom level issue

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 e8bac2aaf030ae0cba4341dc42c4fdadd3c4c5c0..bde7e1c8e9b5e54768aabeb6f92cb1e6842cbc34 100644
--- a/chrome/browser/resources/settings/appearance_page/appearance_page.js
+++ b/chrome/browser/resources/settings/appearance_page/appearance_page.js
@@ -56,27 +56,28 @@ Polymer({
      * List of options for the page zoom drop-down menu.
      * @type {!DropdownMenuOptionList}

[dpapad, 2016/10/15 00:58:07]

Type annotation no longer accurate. Should be !Array<number>

[Dan Beam, 2016/10/17 19:20:26]

Done.

      */
-    pageZoomOptions_: {
+    pageZoomLevels_: {
       readOnly: true,
       type: Array,
       value: [
-        {value: 25, name: '25%'},
-        {value: 33, name: '33%'},
-        {value: 50, name: '50%'},
-        {value: 67, name: '67%'},
-        {value: 75, name: '75%'},
-        {value: 80, name: '80%'},
-        {value: 90, name: '90%'},
-        {value: 100, name: '100%'},
-        {value: 110, name: '110%'},
-        {value: 125, name: '125%'},
-        {value: 150, name: '150%'},
-        {value: 175, name: '175%'},
-        {value: 200, name: '200%'},
-        {value: 250, name: '250%'},
-        {value: 300, name: '300%'},
-        {value: 400, name: '400%'},
-        {value: 500, name: '500%'},
+        // TODO(dbeam): get these dynamically from C++ instead.
+        1 / 4,
+        1 / 3,

[michaelpg, 2016/10/17 08:46:08]

when it comes to repeating decimals, is this safe?

JSON pref store => double => JavaScript number => <option> string => JavaScript
number => double => JSON pref store

[Dan Beam, 2016/10/17 19:20:26]

safe enough, yes
https://cs.chromium.org/chromium/src/content/common/page_zoom.cc?l=16

[michaelpg, 2016/10/17 19:58:15]

ok, that's good. what about the JS side? we populate the <select> with option
values generated in JS (implicitly converted to strings), and set the value with
a Number coming from C++ (implicitly converted to a string).

i guess... is Number.prototype.toString guaranteed to return the same string for
two equivalent rational numbers, one produced in JS and one produced in C++ and
sent to JS via some binding? is the answer implementation or architecture
dependent?

on my current machine, (2/3).toString() isn't accurate: "0.6666666666666666" --
even though with the same number of digits I can safely represent
0.6666666666666667 in a variable that prints the 7.

could CallJavascriptFunction('foo', FundamentalValue(2.0/3)) call foo with an
argument like 0.6666666666666667, resulting in the select not finding a correct
option?

tl;dr using arbitrary real numbers converted to strings as keys scares me

[Dan Beam, 2016/10/17 21:01:20]

i understand your concerns.

i think getting these numbers from zoom::kPresetZoomFactors in C++ and
DCHECK()ing that ZoomValuesEqual() one of them when setting from JS is best way
to go here.

+        1 / 2,
+        2 / 3,
+        3 / 4,
+        4 / 5,
+        9 / 10,
+        1,
+        11 / 10,
+        5 / 4,
+        3 / 2,
+        7 / 4,
+        2,
+        5 / 2,
+        3,
+        4,
+        5,
       ],
     },
 
@@ -118,6 +119,14 @@ Polymer({
   },
 
   /**
+   * @param {number} zoom
+   * @return {number} A zoom easier read by users.

[dpapad, 2016/10/15 00:58:07]

@private

[michaelpg, 2016/10/17 08:46:08]

{string} maybe

[Dan Beam, 2016/10/17 19:20:26]

Done.

[Dan Beam, 2016/10/17 19:20:26]

it's not a string

+   */
+  formatZoom_: function(zoom) {
+    return Math.round(zoom * 100);

[michaelpg, 2016/10/17 08:46:08]

return (zoom * 100).toFixed();

[Dan Beam, 2016/10/17 19:20:26]

what's the functional difference?  especially for the small set we're operating
on?

[michaelpg, 2016/10/17 19:58:14]

toFixed returns a string. it's mathematically equivalent, but if we do return a
number I think "format" is the wrong name.

+  },
+
+  /**
    * @param {boolean} isNtp Whether to use the NTP as the home page.
    * @param {string} homepage If not using NTP, use this URL.
    * @return {string} The sub-label.