[Chromium] Compositor is applying scroll offset using 'programmatic' API
authorjknotten@chromium.org <jknotten@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 15 Mar 2013 12:32:55 +0000 (12:32 +0000)
committerjknotten@chromium.org <jknotten@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 15 Mar 2013 12:32:55 +0000 (12:32 +0000)
https://bugs.webkit.org/show_bug.cgi?id=109712

Reviewed by James Robinson.

Ensure that the compositor uses non-programmatic APIs to scroll and
scale.

* src/WebViewImpl.cpp:
(WebKit::WebViewImpl::setPageScaleFactor):
(WebKit::WebViewImpl::updateMainFrameScrollPosition):
(WebKit):
(WebKit::WebViewImpl::applyScrollAndScale):
* src/WebViewImpl.h:
(WebViewImpl):
* tests/WebFrameTest.cpp:
* tests/WebViewTest.cpp:

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

Source/WebKit/chromium/ChangeLog
Source/WebKit/chromium/src/WebViewImpl.cpp
Source/WebKit/chromium/src/WebViewImpl.h
Source/WebKit/chromium/tests/WebFrameTest.cpp
Source/WebKit/chromium/tests/WebViewTest.cpp

index 95e1dc2..be36416 100644 (file)
@@ -1,3 +1,23 @@
+2013-03-15  John Knottenbelt  <jknotten@chromium.org>
+
+        [Chromium] Compositor is applying scroll offset using 'programmatic' API
+        https://bugs.webkit.org/show_bug.cgi?id=109712
+
+        Reviewed by James Robinson.
+
+        Ensure that the compositor uses non-programmatic APIs to scroll and
+        scale.
+
+        * src/WebViewImpl.cpp:
+        (WebKit::WebViewImpl::setPageScaleFactor):
+        (WebKit::WebViewImpl::updateMainFrameScrollPosition):
+        (WebKit):
+        (WebKit::WebViewImpl::applyScrollAndScale):
+        * src/WebViewImpl.h:
+        (WebViewImpl):
+        * tests/WebFrameTest.cpp:
+        * tests/WebViewTest.cpp:
+
 2013-03-14  Dana Jansens  <danakj@chromium.org>
 
         [chromium] Make zoom filter independent of the layer size.
index 640aa6b..4212090 100644 (file)
@@ -2992,11 +2992,22 @@ void WebViewImpl::setPageScaleFactor(float scaleFactor, const WebPoint& origin)
     if (!scaleFactor)
         scaleFactor = 1;
 
-    IntPoint scrollOffset = origin;
+    IntPoint newScrollOffset = origin;
     scaleFactor = clampPageScaleFactorToLimits(scaleFactor);
-    scrollOffset = clampOffsetAtScale(scrollOffset, scaleFactor);
+    newScrollOffset = clampOffsetAtScale(newScrollOffset, scaleFactor);
 
-    page()->setPageScaleFactor(scaleFactor, IntPoint(scrollOffset));
+    Frame* frame = page()->mainFrame();
+    FrameView* view = frame->view();
+    IntPoint oldScrollOffset = view->scrollPosition();
+
+    // Adjust the page scale in two steps. First, without change to scroll
+    // position, and then with a user scroll to the desired position.
+    // We do this because Page::setPageScaleFactor calls
+    // FrameView::setScrollPosition which is a programmatic scroll
+    // and we need this method to perform only user scrolls.
+    page()->setPageScaleFactor(scaleFactor, oldScrollOffset);
+    if (newScrollOffset != oldScrollOffset)
+        updateMainFrameScrollPosition(newScrollOffset, false);
 
     m_pageScaleFactorIsSet = true;
 
@@ -4166,6 +4177,21 @@ WebInputHandler* WebViewImpl::createInputHandler()
     return handler;
 }
 
+void WebViewImpl::updateMainFrameScrollPosition(const IntPoint& scrollPosition, bool programmaticScroll)
+{
+    FrameView* frameView = page()->mainFrame()->view();
+    if (!frameView)
+        return;
+
+    if (frameView->scrollPosition() == scrollPosition)
+        return;
+
+    bool oldProgrammaticScroll = frameView->inProgrammaticScroll();
+    frameView->setInProgrammaticScroll(programmaticScroll);
+    frameView->notifyScrollPositionChanged(scrollPosition);
+    frameView->setInProgrammaticScroll(oldProgrammaticScroll);
+}
+
 void WebViewImpl::applyScrollAndScale(const WebSize& scrollDelta, float pageScaleDelta)
 {
     if (!mainFrameImpl() || !mainFrameImpl()->frameView())
@@ -4173,7 +4199,9 @@ void WebViewImpl::applyScrollAndScale(const WebSize& scrollDelta, float pageScal
 
     if (pageScaleDelta == 1) {
         TRACE_EVENT_INSTANT2("webkit", "WebViewImpl::applyScrollAndScale::scrollBy", "x", scrollDelta.width, "y", scrollDelta.height);
-        mainFrameImpl()->frameView()->scrollBy(scrollDelta);
+        WebSize webScrollOffset = mainFrame()->scrollOffset();
+        IntPoint scrollOffset(webScrollOffset.width + scrollDelta.width, webScrollOffset.height + scrollDelta.height);
+        updateMainFrameScrollPosition(scrollOffset, false);
     } else {
         // The page scale changed, so apply a scale and scroll in a single
         // operation.
index e1706d9..283d4cf 100644 (file)
@@ -611,6 +611,8 @@ private:
 
     void resetSavedScrollAndScaleState();
 
+    void updateMainFrameScrollPosition(const WebCore::IntPoint& scrollPosition, bool programmaticScroll);
+
     friend class WebView;  // So WebView::Create can call our constructor
     friend class WTF::RefCounted<WebViewImpl>;
     friend void setCurrentInputEventForTest(const WebInputEvent*);
index df5b6da..3dc7400 100644 (file)
@@ -2497,4 +2497,119 @@ TEST_F(WebFrameTest, DidAccessInitialDocumentNavigator)
     m_webView = 0;
 }
 
+class TestMainFrameUserOrProgrammaticScrollFrameClient : public WebFrameClient {
+public:
+    TestMainFrameUserOrProgrammaticScrollFrameClient() { reset(); }
+    void reset()
+    {
+        m_didScrollMainFrame = false;
+        m_wasProgrammaticScroll = false;
+    }
+    bool wasUserScroll() const { return m_didScrollMainFrame && !m_wasProgrammaticScroll; }
+    bool wasProgrammaticScroll() const { return m_didScrollMainFrame && m_wasProgrammaticScroll; }
+
+    // WebFrameClient:
+    virtual void didChangeScrollOffset(WebFrame* frame) OVERRIDE
+    {
+        if (frame->parent())
+            return;
+        EXPECT_FALSE(m_didScrollMainFrame);
+        WebCore::FrameView* view = static_cast<WebFrameImpl*>(frame)->frameView();
+        // FrameView can be scrolled in FrameView::setFixedVisibleContentRect
+        // which is called from Frame::createView (before the frame is associated
+        // with the the view).
+        if (view) {
+            m_didScrollMainFrame = true;
+            m_wasProgrammaticScroll = view->inProgrammaticScroll();
+        }
+    }
+private:
+    bool m_didScrollMainFrame;
+    bool m_wasProgrammaticScroll;
+};
+
+TEST_F(WebFrameTest, CompositorScrollIsUserScrollLongPage)
+{
+    TestMainFrameUserOrProgrammaticScrollFrameClient client;
+
+    std::string pageData = "data:text/html,<div style=\"height:2000px\">A long div</div>";
+
+    // Make sure we initialize to minimum scale, even if the window size
+    // only becomes available after the load begins.
+    m_webView = FrameTestHelpers::createWebViewAndLoad(pageData, true, &client);
+    m_webView->settings()->setApplyDeviceScaleFactorInCompositor(true);
+    m_webView->settings()->setApplyPageScaleFactorInCompositor(true);
+    m_webView->resize(WebSize(1000, 1000));
+    m_webView->layout();
+
+    WebViewImpl* webViewImpl = static_cast<WebViewImpl*>(m_webView);
+    EXPECT_FALSE(client.wasUserScroll());
+    EXPECT_FALSE(client.wasProgrammaticScroll());
+
+    // Do a compositor scroll, verify that this is counted as a user scroll.
+    webViewImpl->applyScrollAndScale(WebSize(0, 1), 1.1f);
+    EXPECT_TRUE(client.wasUserScroll());
+    client.reset();
+
+    EXPECT_FALSE(client.wasUserScroll());
+    EXPECT_FALSE(client.wasProgrammaticScroll());
+
+    // The page scale 1.0f and scroll.
+    webViewImpl->applyScrollAndScale(WebSize(0, 1), 1.0f);
+    EXPECT_TRUE(client.wasUserScroll());
+    client.reset();
+
+    // No scroll event if there is no scroll delta.
+    webViewImpl->applyScrollAndScale(WebSize(), 1.0f);
+    EXPECT_FALSE(client.wasUserScroll());
+    EXPECT_FALSE(client.wasProgrammaticScroll());
+    client.reset();
+
+    // Non zero page scale and scroll.
+    webViewImpl->applyScrollAndScale(WebSize(9, 13), 0.6f);
+    EXPECT_TRUE(client.wasUserScroll());
+    client.reset();
+
+    // Programmatic scroll.
+    WebFrameImpl* frameImpl = webViewImpl->mainFrameImpl();
+    frameImpl->executeScript(WebScriptSource("window.scrollTo(0, 20);"));
+    EXPECT_TRUE(client.wasProgrammaticScroll());
+    client.reset();
+
+    // Programmatic scroll to same offset. No scroll event should be generated.
+    frameImpl->executeScript(WebScriptSource("window.scrollTo(0, 20);"));
+    EXPECT_FALSE(client.wasProgrammaticScroll());
+    EXPECT_FALSE(client.wasUserScroll());
+    client.reset();
+
+    m_webView->close();
+    m_webView = 0;
+}
+
+TEST_F(WebFrameTest, CompositorScrollIsUserScrollShortPage)
+{
+    TestMainFrameUserOrProgrammaticScrollFrameClient client;
+
+    // Short page tests.
+    m_webView = FrameTestHelpers::createWebViewAndLoad("data:text/html,<span>Very short page</span>", true, &client);
+    m_webView->settings()->setApplyDeviceScaleFactorInCompositor(true);
+    m_webView->settings()->setApplyPageScaleFactorInCompositor(true);
+    m_webView->resize(WebSize(1000, 1000));
+    m_webView->layout();
+
+    WebViewImpl* webViewImpl = static_cast<WebViewImpl*>(m_webView);
+    EXPECT_FALSE(client.wasUserScroll());
+    EXPECT_FALSE(client.wasProgrammaticScroll());
+
+    // Non zero page scale and scroll.
+    webViewImpl->applyScrollAndScale(WebSize(9, 13), 2.0f);
+    EXPECT_FALSE(client.wasProgrammaticScroll());
+    EXPECT_TRUE(client.wasUserScroll());
+    client.reset();
+
+    m_webView->close();
+    m_webView = 0;
+}
+
+
 } // namespace
index 46088e6..63b8f12 100644 (file)
@@ -45,6 +45,7 @@
 #include "WebFrameClient.h"
 #include "WebFrameImpl.h"
 #include "WebInputEvent.h"
+#include "WebSettings.h"
 #include "WebViewClient.h"
 #include "WebViewImpl.h"
 #include <gtest/gtest.h>
@@ -481,6 +482,7 @@ TEST_F(WebViewTest, ResetScrollAndScaleState)
 {
     URLTestHelpers::registerMockedURLFromBaseURL(WebString::fromUTF8(m_baseURL.c_str()), WebString::fromUTF8("hello_world.html"));
     WebViewImpl* webViewImpl = static_cast<WebViewImpl*>(FrameTestHelpers::createWebViewAndLoad(m_baseURL + "hello_world.html"));
+    webViewImpl->settings()->setApplyPageScaleFactorInCompositor(true);
     webViewImpl->resize(WebSize(640, 480));
     EXPECT_EQ(0, webViewImpl->mainFrame()->scrollOffset().width);
     EXPECT_EQ(0, webViewImpl->mainFrame()->scrollOffset().height);
@@ -497,6 +499,10 @@ TEST_F(WebViewTest, ResetScrollAndScaleState)
     EXPECT_EQ(1.5f, webViewImpl->pageScaleFactor());
     EXPECT_EQ(16, webViewImpl->mainFrame()->scrollOffset().width);
     EXPECT_EQ(24, webViewImpl->mainFrame()->scrollOffset().height);
+    // WebViewImpl::setPageScaleFactor is performing user scrolls, which will set the
+    // wasScrolledByUser flag on the main frame, and prevent restoreScrollPositionAndViewState
+    // from restoring the scrolling position.
+    webViewImpl->page()->mainFrame()->view()->setWasScrolledByUser(false);
     webViewImpl->page()->mainFrame()->loader()->history()->restoreScrollPositionAndViewState();
     EXPECT_EQ(2.0f, webViewImpl->pageScaleFactor());
     EXPECT_EQ(116, webViewImpl->mainFrame()->scrollOffset().width);