Fix scale of screen.width, window.outerWidth and @media device-width when page scale...
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 10 Jan 2013 21:10:00 +0000 (21:10 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 10 Jan 2013 21:10:00 +0000 (21:10 +0000)
https://bugs.webkit.org/show_bug.cgi?id=106460

Patch by John Mellor <johnme@chromium.org> on 2013-01-10
Reviewed by Kenneth Rohde Christiansen.

Source/WebCore:

screen.width, window.outerWidth and @media device-width should be in
density independent (UI) pixels, not physical screen pixels.

This already works on most ports (including iOS). However Chrome for
Android currently internally sizes its windows in physical screen pixels
instead of density independent pixels, and this leaks through in the
sizes provided to WebCore.

This patch scales these sizes appropriately before they reach JavaScript,
on platforms where the applyPageScaleFactorInCompositor setting is false
(i.e. just Chrome for Android). Once Chrome for Android switches over to
the same coordinate space, the applyPageScaleFactorInCompositor setting
will be removed, along with any code (such as this) that depends on it.

The effect of this patch on Chrome for Android when viewing a page with
a width=device-width viewport on a Galaxy Nexus (720x1280 @ dPR 2) is:
    @media device-width      changes from 720 to 360
    screen.width             changes from 720 to 360
    screen.availWidth        changes from 720 to 360
    window.outerWidth        changes from 720 to 360
    window.innerWidth                    remains 360 (at overview zoom)
    @media width                         remains 360
    document.documentElement.clientWidth remains 360
    document.documentElement.offsetWidth remains 360
    document.documentElement.scrollWidth remains 360
    document.body.clientWidth            remains 360
    document.body.offsetWidth            remains 360
    document.body.scrollWidth            remains 360
And similarly for heights (though they are slightly less than 640, since
toolbars etc. get subtracted from the available height).

No new tests, as applyDeviceScaleFactorInCompositor appears to always be
true in DumpRenderTree, so this situation cannot occur there.

* page/Settings.in:
    Added applyDeviceScaleFactorInCompositor setting, which allows
    detecting whether we need to normalize the scale.
* platform/chromium/PlatformScreenChromium.cpp:
(WebCore::toUserSpace):
    Static function for normalizing screen rect scale.
(WebCore::screenRect):
    Uses toUserSpace.
(WebCore::screenAvailableRect):
    Uses toUserSpace.
* rendering/TextAutosizer.cpp:
(WebCore::TextAutosizer::processSubtree):
    Use applyDeviceScaleFactorInCompositor instead of
    applyPageScaleFactorInCompositor, to scale the window rect correctly
    on platforms other than Chrome for Android.

Source/WebKit/chromium:

1. Exposes the existing applyDeviceScaleFactorInCompositor from
WebSettings on Settings (and stores the value there instead), so it can
be accessed from WebCore.

2. Changes ChromeClientImpl::windowRect to return values in density
independent (UI) pixels pixels instead of physical screen pixels (see
explanation in Source/WebCore/ChangeLog).

* src/ChromeClientImpl.cpp:
(WebKit::ChromeClientImpl::windowRect):
    Normalizes window rect scale.
* src/WebSettingsImpl.cpp:
(WebKit::WebSettingsImpl::WebSettingsImpl):
(WebKit::WebSettingsImpl::setApplyDeviceScaleFactorInCompositor):
(WebKit::WebSettingsImpl::applyDeviceScaleFactorInCompositor):
(WebKit):
* src/WebSettingsImpl.h:
(WebSettingsImpl):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@139356 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/page/Settings.in
Source/WebCore/platform/chromium/PlatformScreenChromium.cpp
Source/WebCore/rendering/TextAutosizer.cpp
Source/WebKit/chromium/ChangeLog
Source/WebKit/chromium/src/ChromeClientImpl.cpp
Source/WebKit/chromium/src/WebSettingsImpl.cpp
Source/WebKit/chromium/src/WebSettingsImpl.h

index 5d3432b..a451889 100644 (file)
@@ -1,3 +1,60 @@
+2013-01-10  John Mellor  <johnme@chromium.org>
+
+        Fix scale of screen.width, window.outerWidth and @media device-width when page scale not applied in compositor.
+        https://bugs.webkit.org/show_bug.cgi?id=106460
+
+        Reviewed by Kenneth Rohde Christiansen.
+
+        screen.width, window.outerWidth and @media device-width should be in
+        density independent (UI) pixels, not physical screen pixels.
+
+        This already works on most ports (including iOS). However Chrome for
+        Android currently internally sizes its windows in physical screen pixels
+        instead of density independent pixels, and this leaks through in the
+        sizes provided to WebCore.
+
+        This patch scales these sizes appropriately before they reach JavaScript,
+        on platforms where the applyPageScaleFactorInCompositor setting is false
+        (i.e. just Chrome for Android). Once Chrome for Android switches over to
+        the same coordinate space, the applyPageScaleFactorInCompositor setting
+        will be removed, along with any code (such as this) that depends on it.
+
+        The effect of this patch on Chrome for Android when viewing a page with
+        a width=device-width viewport on a Galaxy Nexus (720x1280 @ dPR 2) is:
+            @media device-width      changes from 720 to 360
+            screen.width             changes from 720 to 360
+            screen.availWidth        changes from 720 to 360
+            window.outerWidth        changes from 720 to 360
+            window.innerWidth                    remains 360 (at overview zoom)
+            @media width                         remains 360
+            document.documentElement.clientWidth remains 360
+            document.documentElement.offsetWidth remains 360
+            document.documentElement.scrollWidth remains 360
+            document.body.clientWidth            remains 360
+            document.body.offsetWidth            remains 360
+            document.body.scrollWidth            remains 360
+        And similarly for heights (though they are slightly less than 640, since
+        toolbars etc. get subtracted from the available height).
+
+        No new tests, as applyDeviceScaleFactorInCompositor appears to always be
+        true in DumpRenderTree, so this situation cannot occur there.
+
+        * page/Settings.in:
+            Added applyDeviceScaleFactorInCompositor setting, which allows
+            detecting whether we need to normalize the scale.
+        * platform/chromium/PlatformScreenChromium.cpp:
+        (WebCore::toUserSpace):
+            Static function for normalizing screen rect scale.
+        (WebCore::screenRect):
+            Uses toUserSpace.
+        (WebCore::screenAvailableRect):
+            Uses toUserSpace.
+        * rendering/TextAutosizer.cpp:
+        (WebCore::TextAutosizer::processSubtree):
+            Use applyDeviceScaleFactorInCompositor instead of
+            applyPageScaleFactorInCompositor, to scale the window rect correctly
+            on platforms other than Chrome for Android.
+
 2013-01-10  Florin Malita  <fmalita@chromium.org>
 
         Rename GraphicsContext::addRoundedRectClip
index 53b982b..6b0f38d 100644 (file)
@@ -160,6 +160,7 @@ cookieEnabled initial=true
 windowFocusRestricted initial=true
 
 diagnosticLoggingEnabled initial=false
+applyDeviceScaleFactorInCompositor initial=true
 applyPageScaleFactorInCompositor initial=false
 plugInSnapshottingEnabled initial=false
 frameFlatteningEnabled initial=false
index 4fb6915..a298ec9 100644 (file)
 #include "PlatformScreen.h"
 
 #include "FloatRect.h"
+#include "Frame.h"
+#include "FrameView.h"
 #include "HostWindow.h"
+#include "Page.h"
 #include "ScrollView.h"
+#include "Settings.h"
 #include "Widget.h"
 #include <public/Platform.h>
 #include <public/WebScreenInfo.h>
@@ -77,12 +81,24 @@ bool screenIsMonochrome(Widget* widget)
     return client->screenInfo().isMonochrome;
 }
 
+// On Chrome for Android, the screenInfo rects are in physical screen pixels
+// instead of density independent (UI) pixels, and must be scaled down.
+static FloatRect toUserSpace(FloatRect rect, Widget* widget)
+{
+    if (widget->isFrameView()) {
+        Page* page = static_cast<FrameView*>(widget)->frame()->page();
+        if (page && !page->settings()->applyDeviceScaleFactorInCompositor())
+            rect.scale(1 / page->deviceScaleFactor());
+    }
+    return rect;
+}
+
 FloatRect screenRect(Widget* widget)
 {
     PlatformPageClient client = toPlatformPageClient(widget);
     if (!client)
         return FloatRect();
-    return IntRect(client->screenInfo().rect);
+    return toUserSpace(IntRect(client->screenInfo().rect), widget);
 }
 
 FloatRect screenAvailableRect(Widget* widget)
@@ -90,7 +106,7 @@ FloatRect screenAvailableRect(Widget* widget)
     PlatformPageClient client = toPlatformPageClient(widget);
     if (!client)
         return FloatRect();
-    return IntRect(client->screenInfo().availableRect);
+    return toUserSpace(IntRect(client->screenInfo().availableRect), widget);
 }
 
 void screenColorProfile(ColorProfile& toProfile)
index 97e2d18..199b973 100644 (file)
@@ -85,7 +85,7 @@ bool TextAutosizer::processSubtree(RenderObject* layoutRoot)
     if (windowInfo.windowSize.isEmpty()) {
         bool includeScrollbars = !InspectorInstrumentation::shouldApplyScreenWidthOverride(mainFrame);
         windowInfo.windowSize = mainFrame->view()->visibleContentRect(includeScrollbars).size();
-        if (!m_document->settings()->applyPageScaleFactorInCompositor())
+        if (!m_document->settings()->applyDeviceScaleFactorInCompositor())
             windowInfo.windowSize.scale(1 / m_document->page()->deviceScaleFactor());
     }
 
index 052bb97..4ca4417 100644 (file)
@@ -1,3 +1,29 @@
+2013-01-10  John Mellor  <johnme@chromium.org>
+
+        Fix scale of screen.width, window.outerWidth and @media device-width when page scale not applied in compositor.
+        https://bugs.webkit.org/show_bug.cgi?id=106460
+
+        Reviewed by Kenneth Rohde Christiansen.
+
+        1. Exposes the existing applyDeviceScaleFactorInCompositor from
+        WebSettings on Settings (and stores the value there instead), so it can
+        be accessed from WebCore.
+
+        2. Changes ChromeClientImpl::windowRect to return values in density
+        independent (UI) pixels pixels instead of physical screen pixels (see
+        explanation in Source/WebCore/ChangeLog).
+
+        * src/ChromeClientImpl.cpp:
+        (WebKit::ChromeClientImpl::windowRect):
+            Normalizes window rect scale.
+        * src/WebSettingsImpl.cpp:
+        (WebKit::WebSettingsImpl::WebSettingsImpl):
+        (WebKit::WebSettingsImpl::setApplyDeviceScaleFactorInCompositor):
+        (WebKit::WebSettingsImpl::applyDeviceScaleFactorInCompositor):
+        (WebKit):
+        * src/WebSettingsImpl.h:
+        (WebSettingsImpl):
+
 2013-01-10  Sheriff Bot  <webkit.review.bot@gmail.com>
 
         Unreviewed.  Rolled DEPS.
index 83074f5..f4c8f52 100644 (file)
@@ -170,17 +170,19 @@ void ChromeClientImpl::setWindowRect(const FloatRect& r)
 
 FloatRect ChromeClientImpl::windowRect()
 {
-    WebRect rect;
-    if (m_webView->client())
-        rect = m_webView->client()->rootWindowRect();
-    else {
-        // These numbers will be fairly wrong. The window's x/y coordinates will
-        // be the top left corner of the screen and the size will be the content
-        // size instead of the window size.
-        rect.width = m_webView->size().width;
-        rect.height = m_webView->size().height;
+    if (m_webView->client()) {
+        // On Chrome for Android, rootWindowRect is in physical screen pixels
+        // instead of density independent (UI) pixels, and must be scaled down.
+        FloatRect rect = FloatRect(m_webView->client()->rootWindowRect());
+        if (!m_webView->page()->settings()->applyDeviceScaleFactorInCompositor())
+            rect.scale(1 / m_webView->client()->screenInfo().deviceScaleFactor);
+        return rect;
     }
-    return FloatRect(rect);
+
+    // These numbers will be fairly wrong. The window's x/y coordinates will
+    // be the top left corner of the screen and the size will be the content
+    // size instead of the window size.
+    return FloatRect(0, 0, m_webView->size().width, m_webView->size().height);
 }
 
 FloatRect ChromeClientImpl::pageRect()
index 08deb3d..4cf1585 100644 (file)
@@ -54,7 +54,6 @@ WebSettingsImpl::WebSettingsImpl(Settings* settings)
     , m_renderVSyncEnabled(true)
     , m_renderVSyncNotificationEnabled(false)
     , m_viewportEnabled(false)
-    , m_applyDeviceScaleFactorInCompositor(false)
     , m_gestureTapHighlightEnabled(true)
     , m_autoZoomFocusedNodeToLegibleScale(false)
     , m_deferredImageDecodingEnabled(false)
@@ -142,7 +141,7 @@ bool WebSettingsImpl::deviceSupportsTouch()
 
 void WebSettingsImpl::setApplyDeviceScaleFactorInCompositor(bool applyDeviceScaleFactorInCompositor)
 {
-    m_applyDeviceScaleFactorInCompositor = applyDeviceScaleFactorInCompositor;
+    m_settings->setApplyDeviceScaleFactorInCompositor(applyDeviceScaleFactorInCompositor);
 }
 
 void WebSettingsImpl::setApplyPageScaleFactorInCompositor(bool applyPageScaleFactorInCompositor)
@@ -724,6 +723,11 @@ void WebSettingsImpl::setGestureTapHighlightEnabled(bool enableHighlight)
     m_gestureTapHighlightEnabled = enableHighlight;
 }
 
+bool WebSettingsImpl::applyDeviceScaleFactorInCompositor() const
+{
+    return m_settings->applyDeviceScaleFactorInCompositor();
+}
+
 bool WebSettingsImpl::applyPageScaleFactorInCompositor() const
 {
     return m_settings->applyPageScaleFactorInCompositor();
index 7430b48..5a96413 100644 (file)
@@ -175,7 +175,7 @@ public:
     bool showPaintRects() const { return m_showPaintRects; }
     bool renderVSyncEnabled() const { return m_renderVSyncEnabled; }
     bool renderVSyncNotificationEnabled() const { return m_renderVSyncNotificationEnabled; }
-    bool applyDeviceScaleFactorInCompositor() const { return m_applyDeviceScaleFactorInCompositor; }
+    bool applyDeviceScaleFactorInCompositor() const;
     bool applyPageScaleFactorInCompositor() const;
     bool autoZoomFocusedNodeToLegibleScale() const { return m_autoZoomFocusedNodeToLegibleScale; }
     bool gestureTapHighlightEnabled() const { return m_gestureTapHighlightEnabled; }
@@ -193,7 +193,6 @@ private:
     bool m_renderVSyncEnabled;
     bool m_renderVSyncNotificationEnabled;
     bool m_viewportEnabled;
-    bool m_applyDeviceScaleFactorInCompositor;
     bool m_gestureTapHighlightEnabled;
     bool m_autoZoomFocusedNodeToLegibleScale;
     bool m_deferredImageDecodingEnabled;