REGRESSION(r137726): Spring Loaded Pan Scrolling doesn't stop
authoryosin@chromium.org <yosin@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 18 Jan 2013 08:22:36 +0000 (08:22 +0000)
committeryosin@chromium.org <yosin@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 18 Jan 2013 08:22:36 +0000 (08:22 +0000)
https://bugs.webkit.org/show_bug.cgi?id=107205

Reviewed by Hajime Morita.

Source/WebCore:

The bug is caused by forgetting to set true m_panScrollButtonPressed
in AutoscrollController::startPanScroll().

This patch changes state management during pan scroll by replacing
m_panScrollButtonPressed and m_springLoadedPanScrollInProgress by
m_autoscrollType with introducing new AutoscrollController state
AutoscrollForPanCanStop.

Tests: platform/chromium-win/fast/events/panScroll-click.html
       platform/chromium-win/fast/events/panScroll-drag.html

* page/AutoscrollController.cpp:
(WebCore::AutoscrollController::AutoscrollController): Changed to remove initialization of m_panScrollButtonPressed and m_springLoadedPanScrollInProgress.
(WebCore::AutoscrollController::stopAutoscrollTimer): Changed to remove resetting m_panScrollButtonPressed and m_springLoadedPanScrollInProgress.
(WebCore::AutoscrollController::handleMouseReleaseEvent): Changed to handle AutoscrollForPan and AutoscrollForPanCanStop.
(WebCore::AutoscrollController::panScrollInProgress): Changed to check AutoscrollForPanCanStop too.
(WebCore::AutoscrollController::startPanScrolling): Changed to remove setting of m_springLoadedPanScrollInProgress.
(WebCore::AutoscrollController::autoscrollTimerFired): Changed to add case for AutoscrollForPanCanStop.
(WebCore::AutoscrollController::updatePanScrollState): Chagned to use AutoscrollForPan and AutoscrollForPanCanStop.
* page/AutoscrollController.h:
(AutoscrollController): Changed to add AutoscrollForPanCanStop to AutoscrollType.

LayoutTests:

* platform/chromium-win/fast/events/panScroll-click-expected.txt: Added.
* platform/chromium-win/fast/events/panScroll-click.html: Added.
* platform/chromium-win/fast/events/panScroll-drag-expected.txt: Added.
* platform/chromium-win/fast/events/panScroll-drag.html: Added.
* platfrom/chromium/TestExpectations: Skip panScroll-{click,drag}.html for Android, Linux, and Mac.

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

LayoutTests/ChangeLog
LayoutTests/platform/chromium-win/fast/events/panScroll-click-expected.txt [new file with mode: 0644]
LayoutTests/platform/chromium-win/fast/events/panScroll-click.html [new file with mode: 0644]
LayoutTests/platform/chromium-win/fast/events/panScroll-drag-expected.txt [new file with mode: 0644]
LayoutTests/platform/chromium-win/fast/events/panScroll-drag.html [new file with mode: 0644]
LayoutTests/platform/chromium/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/page/AutoscrollController.cpp
Source/WebCore/page/AutoscrollController.h

index 70b4e0f..fcaec4c 100644 (file)
@@ -1,3 +1,16 @@
+2013-01-18  Yoshifumi Inoue  <yosin@chromium.org>
+
+        REGRESSION(r137726): Spring Loaded Pan Scrolling doesn't stop
+        https://bugs.webkit.org/show_bug.cgi?id=107205
+
+        Reviewed by Hajime Morita.
+
+        * platform/chromium-win/fast/events/panScroll-click-expected.txt: Added.
+        * platform/chromium-win/fast/events/panScroll-click.html: Added.
+        * platform/chromium-win/fast/events/panScroll-drag-expected.txt: Added.
+        * platform/chromium-win/fast/events/panScroll-drag.html: Added.
+        * platfrom/chromium/TestExpectations: Skip panScroll-{click,drag}.html for Android, Linux, and Mac.
+
 2013-01-17  Rafael Weinstein  <rafaelw@chromium.org>
 
         Ensure the parser adopts foster-parented children into the document of their parent.
diff --git a/LayoutTests/platform/chromium-win/fast/events/panScroll-click-expected.txt b/LayoutTests/platform/chromium-win/fast/events/panScroll-click-expected.txt
new file mode 100644 (file)
index 0000000..3b89e0a
--- /dev/null
@@ -0,0 +1,9 @@
+For manual testing, hold middle button in scrollable and move aroudn mouse pointer for scrolling, then release middle button to stop scrolling.
+Check pan scroll by click mouse
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS scrollable.scrollTop > 0
+PASS autoscroll stopped
+
diff --git a/LayoutTests/platform/chromium-win/fast/events/panScroll-click.html b/LayoutTests/platform/chromium-win/fast/events/panScroll-click.html
new file mode 100644 (file)
index 0000000..f9a4a73
--- /dev/null
@@ -0,0 +1,123 @@
+<html>
+<head>
+<style type="text/css">
+#draggable {
+  padding: 5pt;
+  border: 3px solid #00cc00;
+  background: #00cccc;
+  width: 80px;
+  cursor: hand;
+}
+
+#scrollable {
+    height: 200px;
+    overflow: auto;
+    border: solid 3px #cc0000;
+    font-size: 80px;
+}
+</style>
+<script>
+function $(id) { return document.getElementById(id); }
+var MIDDLE_BUTTON = 1;
+var PAN_SCROLL_RADIUS = 15; // from WebCore/platform/ScrollView.h
+
+function finishTest() {
+    $('container').innerHTML = '';
+    window.testRunner.notifyDone();
+}
+
+function testIt() {
+    var scrollable = $('scrollable');
+
+    if (!window.eventSender)
+        return;
+
+    // Start pan scroll by click
+    eventSender.mouseMoveTo(scrollable.offsetLeft + 5, scrollable.offsetTop + 5);
+    eventSender.mouseDown(MIDDLE_BUTTON);
+    eventSender.mouseUp(MIDDLE_BUTTON);
+    eventSender.mouseMoveTo(scrollable.offsetLeft + 5, scrollable.offsetTop + PAN_SCROLL_RADIUS + 6);
+
+    var retryCount = 0;
+    var lastScrollTop = 0;
+
+    function checkScrolled()
+    {
+        if (scrollable.scrollTop > 0) {
+            testPassed('scrollable.scrollTop > 0');
+            // Stop spring loaded pan scroll
+            eventSender.mouseDown(MIDDLE_BUTTON);
+            eventSender.mouseUp(MIDDLE_BUTTON);
+            retryCount = 0;
+            window.setTimeout(checkStopped, 50);
+            return;
+        }
+
+        ++retryCount;
+        if (retryCount > 10) {
+            testFailed('No autoscroll');
+            finishTest();
+            return;
+        }
+
+        // Autoscroll is occurred evey 0.05 sec.
+        window.setTimeout(checkScrolled, 50);
+    }
+
+    function checkStopped()
+    {
+        if (lastScrollTop == scrollable.scrollTop) {
+            testPassed('autoscroll stopped');
+            finishTest();
+            return;
+        }
+
+        ++retryCount;
+        if (retryCount > 10) {
+            testFailed('still autoscroll');
+            finishTest();
+            return;
+        }
+
+        lastScrollTop = scrollable.scrollTop;
+        window.setTimeout(checkStopped, 50);
+    }
+
+    checkScrolled();
+}
+
+function setUpTest()
+{
+    var scrollable = $('scrollable');
+    for (var i = 0; i < 100; ++i) {
+      var line = document.createElement('div');
+      line.innerHTML = "line " + i;
+      scrollable.appendChild(line);
+    }
+
+    if (!window.eventSender) {
+        console.log('Please run within DumpRenderTree');
+        return;
+    }
+
+    window.jsTestIsAsync = true;
+    window.setTimeout(testIt, 0);
+}
+</script>
+</head>
+<body>
+For manual testing, hold middle button in scrollable and move aroudn mouse pointer for scrolling, then release middle button to stop scrolling.
+<div id="container">
+Scrollable
+<div id="scrollable">
+</div>
+</div>
+<div id="console"></div>
+<script src="../../../../fast/js/resources/js-test-pre.js"></script>
+<script>
+description('Check pan scroll by click mouse');
+setUpTest();
+</script>
+<script src="../../../../fast/js/resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/platform/chromium-win/fast/events/panScroll-drag-expected.txt b/LayoutTests/platform/chromium-win/fast/events/panScroll-drag-expected.txt
new file mode 100644 (file)
index 0000000..c1c117e
--- /dev/null
@@ -0,0 +1,9 @@
+For manual testing, hold middle button in scrollable and move aroudn mouse pointer for scrolling, then release middle button to stop scrolling.
+Check pan scroll by drag mouse
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS scrollable.scrollTop > 0
+PASS autoscroll stopped
+
diff --git a/LayoutTests/platform/chromium-win/fast/events/panScroll-drag.html b/LayoutTests/platform/chromium-win/fast/events/panScroll-drag.html
new file mode 100644 (file)
index 0000000..3c7b9c6
--- /dev/null
@@ -0,0 +1,121 @@
+<html>
+<head>
+<style type="text/css">
+#draggable {
+  padding: 5pt;
+  border: 3px solid #00cc00;
+  background: #00cccc;
+  width: 80px;
+  cursor: hand;
+}
+
+#scrollable {
+    height: 200px;
+    overflow: auto;
+    border: solid 3px #cc0000;
+    font-size: 80px;
+}
+</style>
+<script>
+function $(id) { return document.getElementById(id); }
+var MIDDLE_BUTTON = 1;
+var PAN_SCROLL_RADIUS = 15; // from WebCore/platform/ScrollView.h
+
+function finishTest() {
+    $('container').innerHTML = '';
+    window.testRunner.notifyDone();
+}
+
+function testIt() {
+    var scrollable = $('scrollable');
+
+    if (!window.eventSender)
+        return;
+
+    // Start pan scroll by drag
+    eventSender.mouseMoveTo(scrollable.offsetLeft + 5, scrollable.offsetTop + 5);
+    eventSender.mouseDown(MIDDLE_BUTTON);
+    eventSender.mouseMoveTo(scrollable.offsetLeft + 5, scrollable.offsetTop + PAN_SCROLL_RADIUS + 6);
+
+    var retryCount = 0;
+    var lastScrollTop = 0;
+
+    function checkScrolled()
+    {
+        if (scrollable.scrollTop > 0) {
+            testPassed('scrollable.scrollTop > 0');
+            // Stop spring loaded pan scroll
+            eventSender.mouseUp(MIDDLE_BUTTON);
+            retryCount = 0;
+            window.setTimeout(checkStopped, 50);
+            return;
+        }
+
+        ++retryCount;
+        if (retryCount > 10) {
+            testFailed('No autoscroll');
+            finishTest();
+            return;
+        }
+
+        // Autoscroll is occurred evey 0.05 sec.
+        window.setTimeout(checkScrolled, 50);
+    }
+
+    function checkStopped()
+    {
+        if (lastScrollTop == scrollable.scrollTop) {
+            testPassed('autoscroll stopped');
+            finishTest();
+            return;
+        }
+
+        ++retryCount;
+        if (retryCount > 10) {
+            testFailed('still autoscroll');
+            finishTest();
+            return;
+        }
+
+        lastScrollTop = scrollable.scrollTop;
+        window.setTimeout(checkStopped, 50);
+    }
+
+    checkScrolled();
+}
+
+function setUpTest()
+{
+    var scrollable = $('scrollable');
+    for (var i = 0; i < 100; ++i) {
+      var line = document.createElement('div');
+      line.innerHTML = "line " + i;
+      scrollable.appendChild(line);
+    }
+
+    if (!window.eventSender) {
+        console.log('Please run within DumpRenderTree');
+        return;
+    }
+
+    window.jsTestIsAsync = true;
+    window.setTimeout(testIt, 0);
+}
+</script>
+</head>
+<body>
+For manual testing, hold middle button in scrollable and move aroudn mouse pointer for scrolling, then release middle button to stop scrolling.
+<div id="container">
+Scrollable
+<div id="scrollable">
+</div>
+</div>
+<div id="console"></div>
+<script src="../../../../fast/js/resources/js-test-pre.js"></script>
+<script>
+description('Check pan scroll by drag mouse');
+setUpTest();
+</script>
+<script src="../../../../fast/js/resources/js-test-post.js"></script>
+</body>
+</html>
index 94c2a32..5126f24 100644 (file)
@@ -2073,6 +2073,9 @@ webkit.org/b/104991 [ Android Linux Mac ] platform/chromium-win/fast/events/panS
 webkit.org/b/104991 [ Android Linux Mac ] platform/chromium-win/fast/events/panScroll-event-fired.html [ Skip ]
 webkit.org/b/104991 [ Android Linux Mac ] platform/chromium-win/fast/events/panScroll-nested-divs.html [ Skip ]
 
+webkit.org/b/107205 [ Android Linux Mac ] platform/chromium-win/fast/events/panScroll-click.html [ Skip ]
+webkit.org/b/107205 [ Android Linux Mac ] platform/chromium-win/fast/events/panScroll-drag.html [ Skip ]
+
 crbug.com/31623 [ SnowLeopard Win ] http/tests/appcache/remove-cache.html [ Failure Pass Timeout ]
 
 # V8's implementation of getOwnPropertyNames has different results for built-in
index 2277f87..1d1f72c 100644 (file)
@@ -1,3 +1,32 @@
+2013-01-18  Yoshifumi Inoue  <yosin@chromium.org>
+
+        REGRESSION(r137726): Spring Loaded Pan Scrolling doesn't stop
+        https://bugs.webkit.org/show_bug.cgi?id=107205
+
+        Reviewed by Hajime Morita.
+
+        The bug is caused by forgetting to set true m_panScrollButtonPressed
+        in AutoscrollController::startPanScroll().
+
+        This patch changes state management during pan scroll by replacing
+        m_panScrollButtonPressed and m_springLoadedPanScrollInProgress by
+        m_autoscrollType with introducing new AutoscrollController state
+        AutoscrollForPanCanStop.
+
+        Tests: platform/chromium-win/fast/events/panScroll-click.html
+               platform/chromium-win/fast/events/panScroll-drag.html
+
+        * page/AutoscrollController.cpp:
+        (WebCore::AutoscrollController::AutoscrollController): Changed to remove initialization of m_panScrollButtonPressed and m_springLoadedPanScrollInProgress.
+        (WebCore::AutoscrollController::stopAutoscrollTimer): Changed to remove resetting m_panScrollButtonPressed and m_springLoadedPanScrollInProgress.
+        (WebCore::AutoscrollController::handleMouseReleaseEvent): Changed to handle AutoscrollForPan and AutoscrollForPanCanStop.
+        (WebCore::AutoscrollController::panScrollInProgress): Changed to check AutoscrollForPanCanStop too.
+        (WebCore::AutoscrollController::startPanScrolling): Changed to remove setting of m_springLoadedPanScrollInProgress.
+        (WebCore::AutoscrollController::autoscrollTimerFired): Changed to add case for AutoscrollForPanCanStop.
+        (WebCore::AutoscrollController::updatePanScrollState): Chagned to use AutoscrollForPan and AutoscrollForPanCanStop.
+        * page/AutoscrollController.h:
+        (AutoscrollController): Changed to add AutoscrollForPanCanStop to AutoscrollType.
+
 2013-01-17  Takashi Sakamoto  <tasak@google.com>
 
         Heap-use-after-free in WebCore::LiveNodeListBase::invalidateCache
index 438e99d..2aeb8c4 100644 (file)
@@ -53,10 +53,6 @@ AutoscrollController::AutoscrollController()
     : m_autoscrollTimer(this, &AutoscrollController::autoscrollTimerFired)
     , m_autoscrollRenderer(0)
     , m_autoscrollType(NoAutoscroll)
-#if ENABLE(PAN_SCROLLING)
-    , m_panScrollButtonPressed(false)
-    , m_springLoadedPanScrollInProgress(false)
-#endif
 {
 }
 
@@ -89,10 +85,6 @@ void AutoscrollController::stopAutoscrollTimer(bool rendererIsBeingDestroyed)
     m_autoscrollTimer.stop();
     m_autoscrollType = NoAutoscroll;
     m_autoscrollRenderer = 0;
-#if ENABLE(PAN_SCROLLING)
-    m_panScrollButtonPressed = false;
-    m_springLoadedPanScrollInProgress = false;
-#endif
 
     if (!scrollable)
         return;
@@ -157,15 +149,20 @@ void AutoscrollController::didPanScrollStop()
 
 void AutoscrollController::handleMouseReleaseEvent(const PlatformMouseEvent& mouseEvent)
 {
-    if (mouseEvent.button() == MiddleButton)
-        m_panScrollButtonPressed = false;
-    if (m_springLoadedPanScrollInProgress)
+    switch (m_autoscrollType) {
+    case AutoscrollForPan:
+        if (mouseEvent.button() == MiddleButton)
+            m_autoscrollType = AutoscrollForPanCanStop;
+        break;
+    case AutoscrollForPanCanStop:
         stopAutoscrollTimer();
+        break;
+    }
 }
 
 bool AutoscrollController::panScrollInProgress() const
 {
-    return m_autoscrollType == AutoscrollForPan;
+    return m_autoscrollType == AutoscrollForPan || m_autoscrollType == AutoscrollForPanCanStop;
 }
 
 void AutoscrollController::startPanScrolling(RenderBox* scrollable, const IntPoint& lastKnownMousePosition)
@@ -177,7 +174,6 @@ void AutoscrollController::startPanScrolling(RenderBox* scrollable, const IntPoi
     m_autoscrollType = AutoscrollForPan;
     m_autoscrollRenderer = scrollable;
     m_panScrollStartPos = lastKnownMousePosition;
-    m_springLoadedPanScrollInProgress = false;
 
     if (FrameView* view = scrollable->frame()->view())
         view->addPanScrollIcon(lastKnownMousePosition);
@@ -210,6 +206,7 @@ void AutoscrollController::autoscrollTimerFired(Timer<AutoscrollController>*)
     case NoAutoscroll:
         break;
 #if ENABLE(PAN_SCROLLING)
+    case AutoscrollForPanCanStop:
     case AutoscrollForPan:
         // we verify that the main frame hasn't received the order to stop the panScroll
         if (Frame* mainFrame = getMainFrame(frame)) {
@@ -241,8 +238,8 @@ void AutoscrollController::updatePanScrollState(FrameView* view, const IntPoint&
     bool north = m_panScrollStartPos.y() > (lastKnownMousePosition.y() + ScrollView::noPanScrollRadius);
     bool south = m_panScrollStartPos.y() < (lastKnownMousePosition.y() - ScrollView::noPanScrollRadius);
 
-    if ((east || west || north || south) && m_panScrollButtonPressed)
-        m_springLoadedPanScrollInProgress = true;
+    if (m_autoscrollType == AutoscrollForPan && (east || west || north || south))
+        m_autoscrollType = AutoscrollForPanCanStop;
 
     if (north) {
         if (east)
index f671f15..02e35e1 100644 (file)
@@ -42,6 +42,7 @@ enum AutoscrollType {
     NoAutoscroll,
     AutoscrollForSelection,
 #if ENABLE(PAN_SCROLLING)
+    AutoscrollForPanCanStop,
     AutoscrollForPan,
 #endif
 };
@@ -76,8 +77,6 @@ private:
     AutoscrollType m_autoscrollType;
 #if ENABLE(PAN_SCROLLING)
     IntPoint m_panScrollStartPos;
-    bool m_panScrollButtonPressed;
-    bool m_springLoadedPanScrollInProgress;
 #endif
 };