Attachment 143427 Details for Bug 82949 – Patch for landing

[patch] Patch for landing

bug-82949-20120522182840.patch (text/plain), 6.10 KB, created by Adam Barth on 2012-05-22 18:28:41 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Adam Barth

Created: 2012-05-22 18:28:41 PDT

Size: 6.10 KB

patch

obsolete

>Index: Source/WebKit/chromium/ChangeLog
>===================================================================
>--- Source/WebKit/chromium/ChangeLog	(revision 118092)
>+++ Source/WebKit/chromium/ChangeLog	(working copy)
>@@ -1,3 +1,28 @@
>+2012-05-22  Alexandre Elias  <aelias@google.com>
>+
>+        [chromium] Apply viewport tag initial-scale only once
>+        https://bugs.webkit.org/show_bug.cgi?id=82949
>+
>+        Reviewed by Adam Barth.
>+
>+        First, check that isPageScaleFactorSet return false before setting
>+        initial-scale. We need to call dispatchViewportPropertiesDidChange()
>+        when the viewport width changes, since that's an input to the viewport
>+        tag calculation. When this happens, we shouldn't pop back to initial
>+        scale.
>+
>+        Second, check that isNewNavigation is true when deciding to clear
>+        isPageScaleFactorIsSet in didCommitLoad. We only want to clear it on
>+        the very first commit, otherwise we'll pop back to initial scale if
>+        the user zooms in before the load is complete.
>+
>+        New test WebFrameTest::FixedLayoutInitializeAtMinimumPageScale.
>+
>+        * src/ChromeClientImpl.cpp:
>+        (WebKit::ChromeClientImpl::dispatchViewportPropertiesDidChange):
>+        * src/WebViewImpl.cpp:
>+        (WebKit::WebViewImpl::didCommitLoad):
>+
> 2012-05-22  Dana Jansens  <danakj@chromium.org>
> 
>         [chromium] Don't force the visibleLayerRect to be empty for animating layers whose front face is not visible
>Index: Source/WebKit/chromium/src/ChromeClientImpl.cpp
>===================================================================
>--- Source/WebKit/chromium/src/ChromeClientImpl.cpp	(revision 118092)
>+++ Source/WebKit/chromium/src/ChromeClientImpl.cpp	(working copy)
>@@ -665,14 +665,14 @@ void ChromeClientImpl::dispatchViewportP
>     int layoutHeight = computed.layoutSize.height();
>     m_webView->setFixedLayoutSize(IntSize(layoutWidth, layoutHeight));
> 
>-    // FIXME: Investigate the impact this has on layout/rendering if any.
>-    // This exposes the correct device scale to javascript and media queries.
>+    bool needInitializePageScale = !m_webView->isPageScaleFactorSet();
>     if (useDefaultDeviceScaleFactor && settings->defaultDeviceScaleFactor())
>         m_webView->setDeviceScaleFactor(settings->defaultDeviceScaleFactor());
>     else
>         m_webView->setDeviceScaleFactor(computed.devicePixelRatio);
>     m_webView->setPageScaleFactorLimits(computed.minimumScale, computed.maximumScale);
>-    m_webView->setPageScaleFactorPreservingScrollOffset(computed.initialScale * computed.devicePixelRatio);
>+    if (needInitializePageScale)
>+        m_webView->setPageScaleFactorPreservingScrollOffset(computed.initialScale * computed.devicePixelRatio);
> #endif
> }
> 
>Index: Source/WebKit/chromium/src/WebViewImpl.cpp
>===================================================================
>--- Source/WebKit/chromium/src/WebViewImpl.cpp	(revision 118092)
>+++ Source/WebKit/chromium/src/WebViewImpl.cpp	(working copy)
>@@ -3102,7 +3102,7 @@ void WebViewImpl::didCommitLoad(bool* is
>     m_newNavigationLoader = 0;
> #endif
>     m_observedNewNavigation = false;
>-    if (!isNavigationWithinPage)
>+    if (*isNewNavigation && !isNavigationWithinPage)
>         m_pageScaleFactorIsSet = false;
> 
>     m_gestureAnimation.clear();
>Index: Source/WebKit/chromium/tests/WebFrameTest.cpp
>===================================================================
>--- Source/WebKit/chromium/tests/WebFrameTest.cpp	(revision 118092)
>+++ Source/WebKit/chromium/tests/WebFrameTest.cpp	(working copy)
>@@ -34,6 +34,7 @@
> 
> #include "Frame.h"
> #include "FrameTestHelpers.h"
>+#include "FrameView.h"
> #include "ResourceError.h"
> #include "WebDocument.h"
> #include "WebFindOptions.h"
>@@ -46,6 +47,7 @@
> #include "WebSecurityPolicy.h"
> #include "WebSettings.h"
> #include "WebViewClient.h"
>+#include "WebFrameImpl.h"
> #include "WebViewImpl.h"
> #include "v8.h"
> #include <gtest/gtest.h>
>@@ -243,6 +245,43 @@ TEST_F(WebFrameTest, DeviceScaleFactorUs
>     // Force the layout to happen before leaving the test.
>     webView->mainFrame()->contentAsText(1024).utf8();
> }
>+
>+TEST_F(WebFrameTest, FixedLayoutInitializeAtMinimumPageScale)
>+{
>+    registerMockedHttpURLLoad("fixed_layout.html");
>+
>+    FixedLayoutTestWebViewClient client;
>+    client.m_screenInfo.horizontalDPI = 160;
>+    int viewportWidth = 640;
>+    int viewportHeight = 480;
>+    client.m_windowRect = WebRect(0, 0, viewportWidth, viewportHeight);
>+
>+    // Make sure we initialize to minimum scale, even if the window size
>+    // only becomes available after the load begins.
>+    WebViewImpl* webViewImpl = static_cast<WebViewImpl*>(FrameTestHelpers::createWebViewAndLoad(m_baseURL + "fixed_layout.html", true, 0, &client));
>+    webViewImpl->enableFixedLayoutMode(true);
>+    webViewImpl->settings()->setViewportEnabled(true);
>+    webViewImpl->resize(WebSize(viewportWidth, viewportHeight));
>+
>+    int defaultFixedLayoutWidth = 980;
>+    float minimumPageScaleFactor = viewportWidth / (float) defaultFixedLayoutWidth;
>+    EXPECT_EQ(minimumPageScaleFactor, webViewImpl->pageScaleFactor());
>+
>+    // Assume the user has pinch zoomed to page scale factor 2.
>+    float userPinchPageScaleFactor = 2;
>+    webViewImpl->setPageScaleFactorPreservingScrollOffset(userPinchPageScaleFactor);
>+    webViewImpl->mainFrameImpl()->frameView()->layout();
>+
>+    // Make sure we don't reset to initial scale if the page continues to load.
>+    bool isNewNavigation;
>+    webViewImpl->didCommitLoad(&isNewNavigation, false);
>+    webViewImpl->didChangeContentsSize();
>+    EXPECT_EQ(userPinchPageScaleFactor, webViewImpl->pageScaleFactor());
>+
>+    // Make sure we don't reset to initial scale if the viewport size changes.
>+    webViewImpl->resize(WebSize(viewportWidth, viewportHeight + 100));
>+    EXPECT_EQ(userPinchPageScaleFactor, webViewImpl->pageScaleFactor());
>+}
> #endif
> 
> #if ENABLE(GESTURE_EVENTS)
>Index: Source/WebKit/chromium/tests/data/fixed_layout.html
>===================================================================
>--- Source/WebKit/chromium/tests/data/fixed_layout.html	(revision 0)
>+++ Source/WebKit/chromium/tests/data/fixed_layout.html	(revision 0)
>@@ -0,0 +1 @@
>+<body>Ordinary non-mobile page</body>

Actions: View | Formatted Diff | Diff

Attachments on bug 82949: 135194 | 135234 | 135238 | 142438 | 143427 | 143433