Unreviewed, rolling out r145898.
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 17 Mar 2013 05:26:04 +0000 (05:26 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 17 Mar 2013 05:26:04 +0000 (05:26 +0000)
http://trac.webkit.org/changeset/145898
https://bugs.webkit.org/show_bug.cgi?id=112512

Causing flakiness on webkit_unit_tests on android bots on
chromium.webkit and chromium.linux (Requested by jochen__ on
#webkit).

Patch by Sheriff Bot <webkit.review.bot@gmail.com> on 2013-03-16

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

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@146005 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 8d10e79..28ef88f 100644 (file)
@@ -1,3 +1,21 @@
+2013-03-16  Sheriff Bot  <webkit.review.bot@gmail.com>
+
+        Unreviewed, rolling out r145898.
+        http://trac.webkit.org/changeset/145898
+        https://bugs.webkit.org/show_bug.cgi?id=112512
+
+        Causing flakiness on webkit_unit_tests on android bots on
+        chromium.webkit and chromium.linux (Requested by jochen__ on
+        #webkit).
+
+        * src/WebViewImpl.cpp:
+        (WebKit::WebViewImpl::setPageScaleFactor):
+        (WebKit::WebViewImpl::applyScrollAndScale):
+        * src/WebViewImpl.h:
+        (WebViewImpl):
+        * tests/WebFrameTest.cpp:
+        * tests/WebViewTest.cpp:
+
 2013-03-16  Hans Wennborg  <hans@chromium.org>
 
         Remove redundant checks from WebInputEvent::isGestureEventType
index 4212090..640aa6b 100644 (file)
@@ -2992,22 +2992,11 @@ void WebViewImpl::setPageScaleFactor(float scaleFactor, const WebPoint& origin)
     if (!scaleFactor)
         scaleFactor = 1;
 
-    IntPoint newScrollOffset = origin;
+    IntPoint scrollOffset = origin;
     scaleFactor = clampPageScaleFactorToLimits(scaleFactor);
-    newScrollOffset = clampOffsetAtScale(newScrollOffset, scaleFactor);
+    scrollOffset = clampOffsetAtScale(scrollOffset, scaleFactor);
 
-    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);
+    page()->setPageScaleFactor(scaleFactor, IntPoint(scrollOffset));
 
     m_pageScaleFactorIsSet = true;
 
@@ -4177,21 +4166,6 @@ 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())
@@ -4199,9 +4173,7 @@ void WebViewImpl::applyScrollAndScale(const WebSize& scrollDelta, float pageScal
 
     if (pageScaleDelta == 1) {
         TRACE_EVENT_INSTANT2("webkit", "WebViewImpl::applyScrollAndScale::scrollBy", "x", scrollDelta.width, "y", scrollDelta.height);
-        WebSize webScrollOffset = mainFrame()->scrollOffset();
-        IntPoint scrollOffset(webScrollOffset.width + scrollDelta.width, webScrollOffset.height + scrollDelta.height);
-        updateMainFrameScrollPosition(scrollOffset, false);
+        mainFrameImpl()->frameView()->scrollBy(scrollDelta);
     } else {
         // The page scale changed, so apply a scale and scroll in a single
         // operation.
index 283d4cf..e1706d9 100644 (file)
@@ -611,8 +611,6 @@ 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 3dc7400..df5b6da 100644 (file)
@@ -2497,119 +2497,4 @@ 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 63b8f12..46088e6 100644 (file)
@@ -45,7 +45,6 @@
 #include "WebFrameClient.h"
 #include "WebFrameImpl.h"
 #include "WebInputEvent.h"
-#include "WebSettings.h"
 #include "WebViewClient.h"
 #include "WebViewImpl.h"
 #include <gtest/gtest.h>
@@ -482,7 +481,6 @@ 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);
@@ -499,10 +497,6 @@ 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);