DevTools: get rid of deviceMetics settings.

Source/devtools/front_end/sdk/OverridesSupport.js

Index: Source/devtools/front_end/sdk/OverridesSupport.js
diff --git a/Source/devtools/front_end/sdk/OverridesSupport.js b/Source/devtools/front_end/sdk/OverridesSupport.js
index 1b61b386811ec7636b99b4644de14515637d0762..bdcac356aacd6c675f9b1e9ad93ba85317e6b478 100644
--- a/Source/devtools/front_end/sdk/OverridesSupport.js
+++ b/Source/devtools/front_end/sdk/OverridesSupport.js
@@ -118,180 +118,6 @@ WebInspector.OverridesSupport.DeviceMetrics.parseSetting = function(value)
 }
 
 /**
- * @return {?WebInspector.OverridesSupport.DeviceMetrics}
- */
-WebInspector.OverridesSupport.DeviceMetrics._parseUserInput = function(widthString, heightString, deviceScaleFactorString, textAutosizing)
-{
-    function isUserInputValid(value, isInteger)
-    {
-        if (!value)
-            return true;
-        return isInteger ? /^[\d]+$/.test(value) : /^[\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);
-
-    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;
-
-    return new WebInspector.OverridesSupport.DeviceMetrics(width, height, deviceScaleFactor, textAutosizing);
-}
-
-/**
- * @param {!Element} widthInput
- * @param {!Element} heightInput
- * @param {!Element} deviceScaleFactorInput
- * @param {!Element} textAutosizingInput
- */
-WebInspector.OverridesSupport.DeviceMetrics.applyOverrides = function(widthInput, heightInput, deviceScaleFactorInput, textAutosizingInput)
-{
-    if (WebInspector.OverridesSupport.DeviceMetrics._applyOverridesTimer)
-        clearTimeout(WebInspector.OverridesSupport.DeviceMetrics._applyOverridesTimer);
-    WebInspector.OverridesSupport.DeviceMetrics._applyOverridesTimer = setTimeout(onTimer, 50);
-
-    function onTimer()
-    {
-        delete WebInspector.OverridesSupport.DeviceMetrics._applyOverridesTimer;
-        var metrics = WebInspector.OverridesSupport.DeviceMetrics._parseUserInput(widthInput.value.trim(), heightInput.value.trim(), deviceScaleFactorInput.value.trim(), textAutosizingInput.checked);
-
-        function setValid(condition, element)
-        {
-            if (condition)
-                element.classList.remove("error-input");
-            else
-                element.classList.add("error-input");
-        }
-
-        setValid(metrics && metrics.isWidthValid(), widthInput);
-        setValid(metrics && metrics.isHeightValid(), heightInput);
-        setValid(metrics && metrics.isDeviceScaleFactorValid(), deviceScaleFactorInput);
-
-        if (!metrics)
-            return;
-
-        if (metrics.isValid()) {
-            var value = metrics.toSetting();
-            if (value !== WebInspector.overridesSupport.settings.deviceMetrics.get())
-                WebInspector.overridesSupport.settings.deviceMetrics.set(value);
-        }
-    }
-}
-
-WebInspector.OverridesSupport.DeviceMetrics.prototype = {
-    /**
-     * @return {boolean}
-     */
-    isValid: function()
-    {
-        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 {string}
-     */
-    toSetting: function()
-    {
-        if (!this.isValid())
-            return "";
-
-        return this.width + "x" + this.height + "x" + this.deviceScaleFactor + "x" + (this.textAutosizing ? "1" : "0");
-    },
-
-    /**
-     * @return {string}
-     */
-    widthToInput: function()
-    {
-        return this.isWidthValid() ? String(this.width) : "";
-    },
-
-    /**
-     * @return {string}
-     */
-    heightToInput: function()
-    {
-        return this.isHeightValid() ? String(this.height) : "";
-    },
-
-    /**
-     * @return {string}
-     */
-    deviceScaleFactorToInput: function()
-    {
-        return this.isDeviceScaleFactorValid() ? String(this.deviceScaleFactor) : "";
-    },
-
-    /**
-     * Compute the font scale factor.
-     *
-     * Chromium on Android uses a device scale adjustment for fonts used in text autosizing for
-     * improved legibility. This function computes this adjusted value for text autosizing.
-     *
-     * For a description of the Android device scale adjustment algorithm, see:
-     *     chrome/browser/chrome_content_browser_client.cc, GetFontScaleMultiplier(...)
-     *
-     * @return {number} font scale factor.
-     */
-    fontScaleFactor: function()
-    {
-        if (this.isValid()) {
-            // FIXME: this works bad with zero width/height. Create utility function with parameters instead.
-            var minWidth = Math.min(this.width, this.height) / (this.deviceScaleFactor || 1);
-
-            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
@@ -458,11 +284,15 @@ WebInspector.OverridesSupport.prototype = {
      */
     emulateDevice: function(deviceMetrics, userAgent)
     {
+        var metrics = WebInspector.OverridesSupport.DeviceMetrics.parseSetting(deviceMetrics)

[dgozman, 2014/05/28 09:39:10]

nit: semicolon missing

[pfeldman, 2014/05/28 15:07:45]

Done.

         this._deviceMetricsChangedListenerMuted = true;
         this._userAgentChangedListenerMuted = true;
-        this.settings.deviceMetrics.set(deviceMetrics);
         this.settings.userAgent.set(userAgent);
-        this.settings.overrideDeviceMetrics.set(true);
+        this.settings.overrideDeviceResolution.set(true);
+        this.settings.deviceWidth.set(metrics.width);
+        this.settings.deviceHeight.set(metrics.height);
+        this.settings.deviceScaleFactor.set(metrics.deviceScaleFactor);
+        this.settings.deviceTextAutosizing.set(metrics.textAutosizing);
         this.settings.overrideUserAgent.set(true);
         this.settings.emulateTouchEvents.set(true);
         this.settings.emulateViewport.set(true);
@@ -476,14 +306,13 @@ WebInspector.OverridesSupport.prototype = {
     {
         this._deviceMetricsChangedListenerMuted = true;
         this._userAgentChangedListenerMuted = true;
-        this.settings.overrideDeviceMetrics.set(false);
+        this.settings.overrideDeviceResolution.set(false);
         this.settings.overrideUserAgent.set(false);
         this.settings.emulateTouchEvents.set(false);
         this.settings.overrideDeviceOrientation.set(false);
         this.settings.overrideGeolocation.set(false);
         this.settings.overrideCSSMedia.set(false);
         this.settings.emulateViewport.set(false);
-        this.settings.deviceMetrics.set("");
         delete this._deviceMetricsChangedListenerMuted;
         delete this._userAgentChangedListenerMuted;
         this._deviceMetricsChanged();
@@ -509,7 +338,7 @@ WebInspector.OverridesSupport.prototype = {
         if (this.settings.overrideCSSMedia.get())
             this._cssMediaChanged();
 
-        if (this.settings.overrideDeviceMetrics.get())
+        if (this.settings.overrideDeviceResolution.get() || this.settings.emulateViewport.get())
             this._deviceMetricsChanged();
 
         if (this.settings.overrideUserAgent.get())
@@ -531,12 +360,8 @@ WebInspector.OverridesSupport.prototype = {
 
     _onPageResizerAvailableSizeChanged: function()
     {
-        var metrics = WebInspector.OverridesSupport.DeviceMetrics.parseSetting(this.settings.deviceMetrics.get());
-        if (!metrics.isValid())
-            return;
-
         var available = this._pageResizer.availableDipSize();
-        if (available.width > metrics.width && available.height > metrics.height)
+        if (available.width > this.settings.deviceWidth.get() && available.height > this.settings.deviceHeight.get())
             return;
 
         this._deviceMetricsChanged();
@@ -544,21 +369,9 @@ WebInspector.OverridesSupport.prototype = {
 
     _onPageResizerResizeRequested: function(event)
     {
-        if (!this.settings.overrideDeviceMetrics.get())
-            return;
-
         var size = /** @type {!Size} */ (event.data);
-        var metrics = WebInspector.OverridesSupport.DeviceMetrics.parseSetting(this.settings.deviceMetrics.get());
-        if (!metrics.isValid())
-            return;
-
-        metrics.width = size.width;
-        metrics.height = size.height;
-        var value = metrics.toSetting();
-        if (this.settings.deviceMetrics.get() === value)
-            return;
-
-        this.settings.deviceMetrics.set(metrics.toSetting());
+        this.settings.deviceWidth.set(size.width);

[dgozman, 2014/05/28 09:39:10]

We should have a 'change multiple settings at once' mechanism to avoid
unnecessary listeners triggering a lot of code.
Something like WebInspector.Setting.{begin,end}BunchUpdate?

[pfeldman, 2014/05/28 15:07:45]

Agreed. But apart from that, I'll make sure it only changes if it is different.

+        this.settings.deviceHeight.set(size.height);
     },
 
     _deviceMetricsChanged: function()
@@ -568,8 +381,8 @@ WebInspector.OverridesSupport.prototype = {
         if (this._deviceMetricsChangedListenerMuted)
             return;
 
-        var metricsOverrideEnabled = this.settings.overrideDeviceMetrics.get();
-        if (!metricsOverrideEnabled) {
+        var overrideDeviceResolution = this.settings.overrideDeviceResolution.get();
+        if (!overrideDeviceResolution && !this.settings.emulateViewport.get()) {
             if (this._pageResizer)
                 this._pageResizer.disable();
             PageAgent.clearDeviceMetricsOverride(apiCallback.bind(this));
@@ -577,19 +390,15 @@ WebInspector.OverridesSupport.prototype = {
             return;
         }
 
-        var metrics = WebInspector.OverridesSupport.DeviceMetrics.parseSetting(this.settings.deviceMetrics.get());
-        if (!metrics.isValid())
-            return;
-
-        var dipWidth = Math.round(metrics.width);
-        var dipHeight = Math.round(metrics.height);
+        var dipWidth = this.settings.deviceWidth.get();
+        var dipHeight = this.settings.deviceHeight.get();
 
         // Disable override without checks.
         if (this.isInspectingDevice())
             return;
 
-        var overrideWidth = dipWidth;
-        var overrideHeight = dipHeight;
+        var overrideWidth = overrideDeviceResolution ? dipWidth : 0;
+        var overrideHeight = overrideDeviceResolution ? dipHeight : 0;
         if (this._pageResizer) {
             var available = this._pageResizer.availableDipSize();
             if (available.width >= dipWidth && available.height >= dipHeight) {
@@ -597,7 +406,7 @@ WebInspector.OverridesSupport.prototype = {
                 // When we have enough space, no page size override is required. This will speed things up and remove lag.
                 overrideWidth = 0;
                 overrideHeight = 0;
-            } else {
+            } else if (overrideDeviceResolution) {

[dgozman, 2014/05/28 09:39:10]

Adding this means it's possible to have |available < 0|.

[pfeldman, 2014/05/28 15:07:45]

Done.

                 this._pageResizer.enable(Math.min(dipWidth, available.width), Math.min(dipHeight, available.height), 0);
             }
         }
@@ -626,9 +435,9 @@ WebInspector.OverridesSupport.prototype = {
             }
 
             PageAgent.setDeviceMetricsOverride(
-                overrideWidth, overrideHeight, metrics.deviceScaleFactor,
+                overrideWidth, overrideHeight, this.settings.deviceScaleFactor.get(),
                 this.settings.emulateViewport.get(), this._pageResizer ? false : this.settings.deviceFitWindow.get(),
-                metrics.textAutosizing, metrics.fontScaleFactor(),
+                this.settings.deviceTextAutosizing.get(), this._fontScaleFactor(),
                 apiCallback.bind(this));
         }
 
@@ -647,10 +456,11 @@ WebInspector.OverridesSupport.prototype = {
                 return;
             }
 
-            var viewportEnabled =  this.settings.emulateViewport.get();
-            this._updateDeviceMetricsWarningMessage(this._deviceMetricsOverrideEnabled !== metricsOverrideEnabled || (metricsOverrideEnabled && this._emulateViewportEnabled != viewportEnabled) ?
+            var overrideDeviceResolution = this.settings.overrideDeviceResolution.get();
+            var viewportEnabled = this.settings.emulateViewport.get();
+            this._updateDeviceMetricsWarningMessage(this._overrideDeviceResolution !== overrideDeviceResolution || this._emulateViewportEnabled != viewportEnabled ?
                 WebInspector.UIString("You might need to reload the page for proper user agent spoofing and viewport rendering.") : "");
-            this._deviceMetricsOverrideEnabled = metricsOverrideEnabled;
+            this._overrideDeviceResolution = overrideDeviceResolution;
             this._emulateViewportEnabled = viewportEnabled;
             this._deviceMetricsOverrideAppliedForTest();
         }
@@ -716,7 +526,7 @@ WebInspector.OverridesSupport.prototype = {
      */
     showMetricsRulers: function()
     {
-        var rulersInPageResizer = this._pageResizer && this.settings.overrideDeviceMetrics.get();
+        var rulersInPageResizer = this._pageResizer && this.settings.overrideDeviceResolution.get();
         return WebInspector.settings.showMetricsRulers.get() && !rulersInPageResizer;
     },
 
@@ -739,7 +549,7 @@ WebInspector.OverridesSupport.prototype = {
     {
         var hasActiveOverrides =
             this.settings.overrideUserAgent.get() ||
-            (this.settings.overrideDeviceMetrics.get() && !this.isInspectingDevice()) ||
+            ((this.settings.overrideDeviceResolution.get() || this.settings.emulateViewport.get()) && !this.isInspectingDevice()) ||
             this.settings.overrideGeolocation.get() ||
             this.settings.overrideDeviceOrientation.get() ||
             (this.settings.emulateTouchEvents.get() && !this.hasTouchInputs()) ||
@@ -795,8 +605,13 @@ WebInspector.OverridesSupport.prototype = {
         this.settings = {};
         this.settings.overrideUserAgent = WebInspector.settings.createSetting("overrideUserAgent", false);
         this.settings.userAgent = WebInspector.settings.createSetting("userAgent", "");
-        this.settings.overrideDeviceMetrics = WebInspector.settings.createSetting("overrideDeviceMetrics", false);
-        this.settings.deviceMetrics = WebInspector.settings.createSetting("deviceMetrics", "");
+
+        this.settings.overrideDeviceResolution = WebInspector.settings.createSetting("overrideDeviceResolution", false);
+        this.settings.deviceWidth = WebInspector.settings.createSetting("deviceWidth", 800);
+        this.settings.deviceHeight = WebInspector.settings.createSetting("deviceHeight", 600);
+        this.settings.deviceScaleFactor = WebInspector.settings.createSetting("deviceScaleFactor", 1);

[dgozman, 2014/05/28 09:39:10]

I'd use window.devicePixelRatio as a default value.

[pfeldman, 2014/05/28 15:07:45]

Done.

+        this.settings.deviceTextAutosizing = WebInspector.settings.createSetting("deviceTextAutosizing", true);
+
         this.settings.deviceFitWindow = WebInspector.settings.createSetting("deviceFitWindow", true);
         this.settings.emulateViewport = WebInspector.settings.createSetting("emulateViewport", false);
         this.settings.emulateTouchEvents = WebInspector.settings.createSetting("emulateTouchEvents", false);
@@ -812,8 +627,11 @@ WebInspector.OverridesSupport.prototype = {
         this.settings.overrideUserAgent.addChangeListener(this._userAgentChanged, this);
         this.settings.userAgent.addChangeListener(this._userAgentChanged, this);
 
-        this.settings.overrideDeviceMetrics.addChangeListener(this._deviceMetricsChanged, this);
-        this.settings.deviceMetrics.addChangeListener(this._deviceMetricsChanged, this);
+        this.settings.overrideDeviceResolution.addChangeListener(this._deviceMetricsChanged, this);
+        this.settings.deviceWidth.addChangeListener(this._deviceMetricsChanged, this);
+        this.settings.deviceHeight.addChangeListener(this._deviceMetricsChanged, this);
+        this.settings.deviceScaleFactor.addChangeListener(this._deviceMetricsChanged, this);
+        this.settings.deviceTextAutosizing.addChangeListener(this._deviceMetricsChanged, this);
         this.settings.emulateViewport.addChangeListener(this._deviceMetricsChanged, this);
         this.settings.deviceFitWindow.addChangeListener(this._deviceMetricsChanged, this);
 
@@ -836,6 +654,14 @@ WebInspector.OverridesSupport.prototype = {
         }
     },
 
+    swapDimensions: function()
+    {
+        var width = WebInspector.OverridesSupport.settings.deviceWidth.get();

[dgozman, 2014/05/28 09:39:10]

Use WebInspector.overridesSupport instance, not a class.

[pfeldman, 2014/05/28 15:07:45]

Done.

+        var height = WebInspector.OverridesSupport.settings.deviceHeight.get();
+        WebInspector.OverridesSupport.settings.deviceWidth.set(height);
+        WebInspector.OverridesSupport.settings.deviceHeight.set(width);
+    },
+
     /**
      * @param {!WebInspector.Target} target
      */
@@ -860,6 +686,45 @@ WebInspector.OverridesSupport.prototype = {
         return !!this._target && this._target.hasTouchInputs;
     },
 
+    /**
+     * Compute the font scale factor.
+     *
+     * Chromium on Android uses a device scale adjustment for fonts used in text autosizing for
+     * improved legibility. This function computes this adjusted value for text autosizing.
+     *
+     * For a description of the Android device scale adjustment algorithm, see:
+     *     chrome/browser/chrome_content_browser_client.cc, GetFontScaleMultiplier(...)
+     *
+     * @return {number} font scale factor.
+     */
+    _fontScaleFactor: function()
+    {
+        if (!this.settings.overrideDeviceResolution.get())
+            return 1;
+
+        var width = this.settings.deviceWidth.get();
+        var height = this.settings.deviceWidth.get();
+        var deviceScaleFactor = this.settings.deviceScaleFactor.get();
+
+        // FIXME: this works bad with zero width/height. Create utility function with parameters instead.

[dgozman, 2014/05/28 09:39:10]

You can easily fix this TODO by passing (overrideWidth | dipWidth) at call site
now instead of using setting value.

[pfeldman, 2014/05/28 15:07:45]

Done.

+        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);
+
+        return ratio * (kMaxFSM - kMinFSM) + kMinFSM;
+    },
+
     __proto__: WebInspector.Object.prototype
 }