Refactor how font scale factor appears in the emulation panel

Source/devtools/front_end/OverridesSupport.js

 /*
  * Copyright (C) 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 48 matching lines @@
 
 WebInspector.OverridesSupport.Events = {
     OverridesWarningUpdated: "OverridesWarningUpdated",
 }
 
 /**
  * @constructor
  * @param {number} width
  * @param {number} height
  * @param {number} deviceScaleFactor
- * @param {number} fontScaleFactor
  * @param {boolean} textAutosizing
+ * @param {boolean} useAndroidFontMetrics
  */
-WebInspector.OverridesSupport.DeviceMetrics = function(width, height, deviceScaleFactor, fontScaleFactor, textAutosizing)
+WebInspector.OverridesSupport.DeviceMetrics = function(width, height, deviceScaleFactor, textAutosizing, useAndroidFontMetrics)
 {
     this.width = width;
     this.height = height;
     this.deviceScaleFactor = deviceScaleFactor;
-    this.fontScaleFactor = fontScaleFactor;
     this.textAutosizing = textAutosizing;
+    this.useAndroidFontMetrics = useAndroidFontMetrics;
 }
 
 /**
- * Android computes a font scale factor fudged for improved legibility. This value is
- * not computed on non-Android builds so we duplicate the Android-specific logic here.
- * For a description of this algorithm, see:
- *     chrome/browser/chrome_content_browser_client.cc, GetFontScaleMultiplier(...)
- *
- * @param {number} width
- * @param {number} height
- * @param {number} deviceScaleFactor
- * @return {number} fontScaleFactor for Android.
- */
-// FIXME(crbug.com/313410): Unify this calculation with the c++ side so we don't duplicate this logic.
-WebInspector.OverridesSupport.DeviceMetrics._computeFontScaleFactorForAndroid = function(width, height, deviceScaleFactor)
-{
-    var minWidth = Math.min(width, height) / deviceScaleFactor;
-
-    var kMinFSM = 1.05;
-    var kWidthForMinFSM = 320;
-    var kMaxFSM = 1.3;
-    var kWidthForMaxFSM = 800;
-
-    if (minWidth <= kWidthForMinFSM)
-        return kMinFSM;
-    if (minWidth >= kWidthForMaxFSM)
-        return kMaxFSM;
-
-    // The font scale multiplier varies linearly between kMinFSM and kMaxFSM.
-    var ratio = (minWidth - kWidthForMinFSM) / (kWidthForMaxFSM - kWidthForMinFSM);
-    var fontScaleFactor = ratio * (kMaxFSM - kMinFSM) + kMinFSM;
-    return Math.round(fontScaleFactor * 1000) / 1000;
-}
-
-/**
  * @return {WebInspector.OverridesSupport.DeviceMetrics}
  */
 WebInspector.OverridesSupport.DeviceMetrics.parseSetting = function(value)
 {
     var width = 0;
     var height = 0;
     var deviceScaleFactor = 1;
-    var fontScaleFactor = 1;
     var textAutosizing = false;
+    var useAndroidFontMetrics = false;
     if (value) {
         var splitMetrics = value.split("x");
         if (splitMetrics.length === 5) {
             width = parseInt(splitMetrics[0], 10);
             height = parseInt(splitMetrics[1], 10);
             deviceScaleFactor = parseFloat(splitMetrics[2]);
-            fontScaleFactor = parseFloat(splitMetrics[3]);
-            if (fontScaleFactor == 0)
-                fontScaleFactor = WebInspector.OverridesSupport.DeviceMetrics._computeFontScaleFactorForAndroid(width, height, deviceScaleFactor);
+            useAndroidFontMetrics = splitMetrics[3] == 1;
             textAutosizing = splitMetrics[4] == 1;
         }
     }
-    return new WebInspector.OverridesSupport.DeviceMetrics(width, height, deviceScaleFactor, fontScaleFactor, textAutosizing);
+    return new WebInspector.OverridesSupport.DeviceMetrics(width, height, deviceScaleFactor, textAutosizing, useAndroidFontMetrics);
 }
 
 /**
  * @return {?WebInspector.OverridesSupport.DeviceMetrics}
  */
-WebInspector.OverridesSupport.DeviceMetrics.parseUserInput = function(widthString, heightString, deviceScaleFactorString, fontScaleFactorString, textAutosizing)
+WebInspector.OverridesSupport.DeviceMetrics.parseUserInput = function(widthString, heightString, deviceScaleFactorString, textAutosizing, useAndroidFontMetrics)
 {
     function isUserInputValid(value, isInteger)
     {
         if (!value)
             return true;
         return isInteger ? /^[0]*[1-9][\d]*$/.test(value) : /^[0]*([1-9][\d]*(\.\d+)?|\.\d+)$/.test(value);
     }
 
     if (!widthString ^ !heightString)
         return null;
 
     var isWidthValid = isUserInputValid(widthString, true);
     var isHeightValid = isUserInputValid(heightString, true);
     var isDeviceScaleFactorValid = isUserInputValid(deviceScaleFactorString, false);
-    var isFontScaleFactorValid = isUserInputValid(fontScaleFactorString, false);
 
-    if (!isWidthValid && !isHeightValid && !isDeviceScaleFactorValid && !isFontScaleFactorValid)
+    if (!isWidthValid && !isHeightValid && !isDeviceScaleFactorValid)
         return null;
 
     var width = isWidthValid ? parseInt(widthString || "0", 10) : -1;
     var height = isHeightValid ? parseInt(heightString || "0", 10) : -1;
     var deviceScaleFactor = isDeviceScaleFactorValid ? parseFloat(deviceScaleFactorString) : -1;
-    var fontScaleFactor = isFontScaleFactorValid ? parseFloat(fontScaleFactorString) : -1;
 
-    return new WebInspector.OverridesSupport.DeviceMetrics(width, height, deviceScaleFactor, fontScaleFactor, textAutosizing);
+    return new WebInspector.OverridesSupport.DeviceMetrics(width, height, deviceScaleFactor, textAutosizing, useAndroidFontMetrics);
 }
 
 WebInspector.OverridesSupport.DeviceMetrics.prototype = {
     /**
      * @return {boolean}
      */
     isValid: function()
     {
-        return this.isWidthValid() && this.isHeightValid() && this.isDeviceScaleFactorValid() && this.isFontScaleFactorValid();
+        return this.isWidthValid() && this.isHeightValid() && this.isDeviceScaleFactorValid();
     },
 
     /**
      * @return {boolean}
      */
     isWidthValid: function()
     {
         return this.width >= 0;
     },
 
     /**
      * @return {boolean}
      */
     isHeightValid: function()
     {
         return this.height >= 0;
     },
 
     /**
      * @return {boolean}
      */
     isDeviceScaleFactorValid: function()
     {
         return this.deviceScaleFactor > 0;
     },
 
     /**
-     * @return {boolean}
-     */
-    isFontScaleFactorValid: function()
-    {
-        return this.fontScaleFactor > 0;
-    },
-
-    /**
-     * @return {boolean}
-     */
-    isTextAutosizingValid: function()
-    {
-        return true;
-    },
-
-    /**
      * @return {string}
      */
     toSetting: function()
     {
         if (!this.isValid())
             return "";
 
-        return this.width && this.height ? this.width + "x" + this.height + "x" + this.deviceScaleFactor + "x" + this.fontScaleFactor + "x" + (this.textAutosizing ? "1" : "0") : "";
+        return this.width && this.height ? this.width + "x" + this.height + "x" + this.deviceScaleFactor + "x" + (this.useAndroidFontMetrics ? "1" : "0") + "x" + (this.textAutosizing ? "1" : "0") : "";
     },
 
     /**
      * @return {string}
      */
     widthToInput: function()
     {
         return this.isWidthValid() && this.width ? String(this.width) : "";
     },
 
     /**
      * @return {string}
      */
     heightToInput: function()
     {
         return this.isHeightValid() && this.height ? String(this.height) : "";
     },
 
     /**
      * @return {string}
      */
     deviceScaleFactorToInput: function()
     {
         return this.isDeviceScaleFactorValid() && this.deviceScaleFactor ? String(this.deviceScaleFactor) : "";
     },
 
     /**
-     * @return {string}
+     * Compute the font scale factor.
+     *
+     * Android uses a font scale factor for text autosizing that is fudged for improved

[skobes, 2013/11/15 19:30:40]

Note that:

(1) I renamed the fudge factor to "device scale adjustment" so that it is more
clearly a separate entity from the font scale factor (otherwise "font scale
factor" means two different things depending on what layer of the code you are
in)

(2) Currently we only apply it when there is no meta viewport, though there is
an unresolved discussion on https://codereview.chromium.org/49913005/ about
changing that.  Not sure how closely you want to emulate that behavior in the
inspector.

[pdr., 2013/11/15 23:13:16]

I'll update this wording in the comment. The rename from font scale factor to
device scale adjustment will require a two-sided patch, so lets do that in
another patch :)
My understanding is that we'll use your updated code in TextAutosizer.cpp. The
inspector just enables that codepath during emulation.

[skobes, 2013/11/15 23:26:03]

Are you sure?  It looks like the inspector hooks into
Settings::textAutosizingFontScaleFactor but not Settings::deviceScaleAdjustment.

+     * legibility. This function computes this fudged value if useAndroidFontMetrics is true.
+     *
+     * For a description of the Android font scale factor algorithm, see:
+     *     chrome/browser/chrome_content_browser_client.cc, GetFontScaleMultiplier(...)
+     *
+     * @return {number} font scale factor for Android if useAndroidFontMetrics, or 1.
      */
-    fontScaleFactorToInput: function()
+    fontScaleFactor: function()
     {
-        return this.isFontScaleFactorValid() && this.fontScaleFactor ? String(this.fontScaleFactor) : "";
+        if (this.useAndroidFontMetrics && this.isValid()) {
+            var minWidth = Math.min(this.width, this.height) / this.deviceScaleFactor;
+
+            var kMinFSM = 1.05;
+            var kWidthForMinFSM = 320;
+            var kMaxFSM = 1.3;
+            var kWidthForMaxFSM = 800;
+
+            if (minWidth <= kWidthForMinFSM)
+                return kMinFSM;
+            if (minWidth >= kWidthForMaxFSM)
+                return kMaxFSM;
+
+            // The font scale multiplier varies linearly between kMinFSM and kMaxFSM.
+            var ratio = (minWidth - kWidthForMinFSM) / (kWidthForMaxFSM - kWidthForMinFSM);
+
+            return ratio * (kMaxFSM - kMinFSM) + kMinFSM;
+        }
+
+        return 1;
     }
 }
 
 /**
  * @constructor
  * @param {number} latitude
  * @param {number} longitude
  */
 WebInspector.OverridesSupport.GeolocationPosition = function(latitude, longitude, error)
 {
@@ skipping 192 matching lines @@
 
         var dipWidth = Math.round(metrics.width / metrics.deviceScaleFactor);
         var dipHeight = Math.round(metrics.height / metrics.deviceScaleFactor);
 
         // Disable override without checks.
         if (dipWidth && dipHeight && WebInspector.isInspectingDevice()) {
             this._updateWarningMessage(WebInspector.UIString("Screen emulation on the device is not available."));
             return;
         }
 
-        PageAgent.setDeviceMetricsOverride(dipWidth, dipHeight, metrics.deviceScaleFactor, WebInspector.settings.emulateViewport.get(), WebInspector.settings.deviceFitWindow.get(), metrics.textAutosizing, metrics.fontScaleFactor, apiCallback.bind(this));
+        PageAgent.setDeviceMetricsOverride(dipWidth, dipHeight, metrics.deviceScaleFactor, WebInspector.settings.emulateViewport.get(), WebInspector.settings.deviceFitWindow.get(), metrics.textAutosizing, metrics.fontScaleFactor(), apiCallback.bind(this));
 
         /**
          * param {?Protocol.Error} error
          */
         function apiCallback(error)
         {
             if (error) {
                 this._updateWarningMessage(WebInspector.UIString("Screen emulation is not available on this page."));
                 return;
             }
@@ skipping 84 matching lines @@
     },
 
     __proto__: WebInspector.Object.prototype
 }
 
 
 /**
  * @type {WebInspector.OverridesSupport}
  */
 WebInspector.overridesSupport;