REGRESSION (r172832): Poor 2-finger scrolling performance at theverge.com articles
authorbdakin@apple.com <bdakin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 4 Sep 2014 20:48:03 +0000 (20:48 +0000)
committerbdakin@apple.com <bdakin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 4 Sep 2014 20:48:03 +0000 (20:48 +0000)
(all tiles repaint)
https://bugs.webkit.org/show_bug.cgi?id=136433
-and corresponding-
rdar://problem/18193942

Reviewed by Tim Horton.

Source/WebCore:

We should ensure that we are only setting scroll elasticity for layers that return
true for scrollsOverflow(). When overflow:scroll is set on the root element, we
wound up setting the ScrollElasticity for the root, which messed up with the
special way that the root is meant to scroll. Even though overflow:scroll has been
set on the root, scrollsOverflow() is still false because we knew not to set
hasOverflowClip() since it’s the root, which is why this check works.
* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::setHasHorizontalScrollbar):
(WebCore::RenderLayer::setHasVerticalScrollbar):

Really, we should have never even called RenderLayer::setHasVerticalScrollbar or
RenderLayer::setHasHorizontalScrollbar since it’s wrong to be creating a scrollbar
on RenderLayer for the root. We should make sure, in addition to the other
requirements consulted, that the renderer has an overflow clip before we create
the scrollbars.
(WebCore::RenderLayer::updateScrollbarsAfterStyleChange):

LayoutTests:

* platform/mac-wk2/tiled-drawing/scrolling/root-overflow-with-mousewheel-expected.txt: Added.
* platform/mac-wk2/tiled-drawing/scrolling/root-overflow-with-mousewheel.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/root-overflow-with-mousewheel-expected.txt [new file with mode: 0644]
LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/root-overflow-with-mousewheel.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderLayer.cpp

index 4f84c83..76401c2 100644 (file)
@@ -1,3 +1,16 @@
+2014-09-04  Beth Dakin  <bdakin@apple.com>
+
+        REGRESSION (r172832): Poor 2-finger scrolling performance at theverge.com articles 
+        (all tiles repaint)
+        https://bugs.webkit.org/show_bug.cgi?id=136433
+        -and corresponding-
+        rdar://problem/18193942
+
+        Reviewed by Tim Horton.
+
+        * platform/mac-wk2/tiled-drawing/scrolling/root-overflow-with-mousewheel-expected.txt: Added.
+        * platform/mac-wk2/tiled-drawing/scrolling/root-overflow-with-mousewheel.html: Added.
+
 2014-09-04  Eva Balazsfalvi  <evab.u-szeged@partner.samsung.com>
 
         Remove CSS_FILTERS flag
diff --git a/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/root-overflow-with-mousewheel-expected.txt b/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/root-overflow-with-mousewheel-expected.txt
new file mode 100644 (file)
index 0000000..f985b46
--- /dev/null
@@ -0,0 +1 @@
+Success!
diff --git a/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/root-overflow-with-mousewheel.html b/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/root-overflow-with-mousewheel.html
new file mode 100644 (file)
index 0000000..35a7f8e
--- /dev/null
@@ -0,0 +1,53 @@
+<html>
+<head>
+<style>
+html {
+    height: 2000px;
+    overflow: scroll;
+}
+</style>
+
+<script>
+function checkForScroll()
+{
+    var pageScrollPositionAfter = document.body.scrollTop;
+    var finishLog = document.getElementById("finishLog");
+    if (pageScrollPositionAfter > 50)
+        finishLog.innerHTML = "Success!";
+    else
+        finishLog.innerHTML = "Failure. Try running the test manually. If this proves to be flakey, and we might have to skip it.";
+    testRunner.notifyDone();
+}
+
+function scrollTest()
+{
+    eventSender.mouseMoveTo(50, 50);
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'began', 'none');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'changed', 'none');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'changed', 'none');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, 'ended', 'none');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'none', 'begin');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'none', 'continue');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'none', 'continue');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'none', 'continue');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'none', 'continue');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, 'none', 'end');
+    checkForScroll();
+}
+
+function onLoad() {
+    window.addEventListener('mousewheel', function() { }, false);
+
+    if (window.eventSender) {
+        testRunner.dumpAsText();
+        testRunner.waitUntilDone();
+        setTimeout(scrollTest, 0);
+    }
+}
+</script>
+</head>
+
+<body onload="onLoad();">
+    <pre id="finishLog">This test passes if you can scroll the page.</pre>
+</body>
+</html>
index bfd69f1..cdc4df0 100644 (file)
@@ -1,3 +1,30 @@
+2014-09-04  Beth Dakin  <bdakin@apple.com>
+
+        REGRESSION (r172832): Poor 2-finger scrolling performance at theverge.com articles 
+        (all tiles repaint)
+        https://bugs.webkit.org/show_bug.cgi?id=136433
+        -and corresponding-
+        rdar://problem/18193942
+
+        Reviewed by Tim Horton.
+
+        We should ensure that we are only setting scroll elasticity for layers that return 
+        true for scrollsOverflow(). When overflow:scroll is set on the root element, we 
+        wound up setting the ScrollElasticity for the root, which messed up with the 
+        special way that the root is meant to scroll. Even though overflow:scroll has been 
+        set on the root, scrollsOverflow() is still false because we knew not to set 
+        hasOverflowClip() since it’s the root, which is why this check works.  
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::setHasHorizontalScrollbar):
+        (WebCore::RenderLayer::setHasVerticalScrollbar):
+
+        Really, we should have never even called RenderLayer::setHasVerticalScrollbar or 
+        RenderLayer::setHasHorizontalScrollbar since it’s wrong to be creating a scrollbar 
+        on RenderLayer for the root. We should make sure, in addition to the other 
+        requirements consulted, that the renderer has an overflow clip before we create 
+        the scrollbars.
+        (WebCore::RenderLayer::updateScrollbarsAfterStyleChange):
+
 2014-09-04  Antti Koivisto  <antti@apple.com>
 
         Remove ResourceResponse::m_suggestedFilename
index 8c2de9f..8109f9f 100644 (file)
@@ -2991,7 +2991,8 @@ void RenderLayer::setHasHorizontalScrollbar(bool hasScrollbar)
     if (hasScrollbar) {
         m_hBar = createScrollbar(HorizontalScrollbar);
 #if ENABLE(RUBBER_BANDING)
-        ScrollableArea::setHorizontalScrollElasticity(renderer().frame().settings().rubberBandingForOverflowScrollEnabled() ? ScrollElasticityAutomatic : ScrollElasticityNone);
+        ScrollElasticity elasticity = scrollsOverflow() && renderer().frame().settings().rubberBandingForOverflowScrollEnabled() ? ScrollElasticityAutomatic : ScrollElasticityNone;
+        ScrollableArea::setHorizontalScrollElasticity(elasticity);
 #endif
     } else {
         destroyScrollbar(HorizontalScrollbar);
@@ -3021,7 +3022,8 @@ void RenderLayer::setHasVerticalScrollbar(bool hasScrollbar)
     if (hasScrollbar) {
         m_vBar = createScrollbar(VerticalScrollbar);
 #if ENABLE(RUBBER_BANDING)
-        ScrollableArea::setVerticalScrollElasticity((renderer().frame().settings().rubberBandingForOverflowScrollEnabled() ? ScrollElasticityAutomatic : ScrollElasticityNone));
+        ScrollElasticity elasticity = scrollsOverflow() && renderer().frame().settings().rubberBandingForOverflowScrollEnabled() ? ScrollElasticityAutomatic : ScrollElasticityNone;
+        ScrollableArea::setVerticalScrollElasticity(elasticity);
 #endif
     } else {
         destroyScrollbar(VerticalScrollbar);
@@ -6364,8 +6366,8 @@ void RenderLayer::updateScrollbarsAfterStyleChange(const RenderStyle* oldStyle)
     EOverflow overflowY = box->style().overflowY();
 
     // To avoid doing a relayout in updateScrollbarsAfterLayout, we try to keep any automatic scrollbar that was already present.
-    bool needsHorizontalScrollbar = (hasHorizontalScrollbar() && overflowDefinesAutomaticScrollbar(overflowX)) || overflowRequiresScrollbar(overflowX);
-    bool needsVerticalScrollbar = (hasVerticalScrollbar() && overflowDefinesAutomaticScrollbar(overflowY)) || overflowRequiresScrollbar(overflowY);
+    bool needsHorizontalScrollbar = box->hasOverflowClip() && ((hasHorizontalScrollbar() && overflowDefinesAutomaticScrollbar(overflowX)) || overflowRequiresScrollbar(overflowX));
+    bool needsVerticalScrollbar = box->hasOverflowClip() && ((hasVerticalScrollbar() && overflowDefinesAutomaticScrollbar(overflowY)) || overflowRequiresScrollbar(overflowY));
     setHasHorizontalScrollbar(needsHorizontalScrollbar);
     setHasVerticalScrollbar(needsVerticalScrollbar);