Some non-scrollable elements are added to non-fast-scrollable region
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 8 Oct 2012 22:15:44 +0000 (22:15 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 8 Oct 2012 22:15:44 +0000 (22:15 +0000)
https://bugs.webkit.org/show_bug.cgi?id=97927

Patch by Sami Kyostila <skyostil@chromium.org> on 2012-10-08
Reviewed by James Robinson.

Source/WebCore:

Only RenderLayers that can actually be scrolled should be added to the
ScrollingCoordinator's non-fast scrollable region. Otherwise we may
needlessly fall back to main thread scrolling.

Test: ScrollingCoordinatorChromiumTest.clippedBodyTest

* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::updateScrollbarsAfterLayout):
(WebCore::RenderLayer::updateScrollbarsAfterStyleChange):

Source/WebKit/chromium:

Test that a non-scrollable RenderLayer isn't added to the non-fast scrollable
region.

The test in this patch consists of a web page where both the html and
body elements clip horizontal overflow. This results in a hierarchy
where the body element gets a scrollable RenderLayer. However, that
layer isn't (interactively) scrollable because while the scroll geometry
is calculated based on the amount of overflow, the scroll bars are
hidden because of the "overflow-x: hidden" style.

Previously this layer would have made the entire page part of the
non-fast scrollable region. With this patch,
RenderLayer::allowScrolling() is used as a pre-requisite for expanding
the region, thus avoiding the problem.

* tests/ScrollingCoordinatorChromiumTest.cpp:
(WebKit::TEST_F):
(WebKit):
* tests/data/clipped-body.html: Added.

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

Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderLayer.cpp
Source/WebKit/chromium/ChangeLog
Source/WebKit/chromium/tests/ScrollingCoordinatorChromiumTest.cpp
Source/WebKit/chromium/tests/data/clipped-body.html [new file with mode: 0644]

index f67f242..5e16b39 100644 (file)
@@ -1,3 +1,20 @@
+2012-10-08  Sami Kyostila  <skyostil@chromium.org>
+
+        Some non-scrollable elements are added to non-fast-scrollable region
+        https://bugs.webkit.org/show_bug.cgi?id=97927
+
+        Reviewed by James Robinson.
+
+        Only RenderLayers that can actually be scrolled should be added to the
+        ScrollingCoordinator's non-fast scrollable region. Otherwise we may
+        needlessly fall back to main thread scrolling.
+
+        Test: ScrollingCoordinatorChromiumTest.clippedBodyTest
+
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::updateScrollbarsAfterLayout):
+        (WebCore::RenderLayer::updateScrollbarsAfterStyleChange):
+
 2012-10-08  Simon Fraser  <simon.fraser@apple.com>
 
         Move layerTreeAsText to window.internals
 2012-10-08  Simon Fraser  <simon.fraser@apple.com>
 
         Move layerTreeAsText to window.internals
index d9e0bfc..a1947e4 100644 (file)
@@ -2629,7 +2629,7 @@ void RenderLayer::updateScrollbarsAfterLayout()
         m_vBar->setProportion(clientHeight, m_scrollSize.height());
     }
 
         m_vBar->setProportion(clientHeight, m_scrollSize.height());
     }
 
-    updateScrollableAreaSet((hasHorizontalOverflow || hasVerticalOverflow) && scrollsOverflow());
+    updateScrollableAreaSet((hasHorizontalOverflow || hasVerticalOverflow) && scrollsOverflow() && allowsScrolling());
 }
 
 void RenderLayer::updateScrollInfoAfterLayout()
 }
 
 void RenderLayer::updateScrollInfoAfterLayout()
@@ -4847,7 +4847,7 @@ void RenderLayer::updateScrollbarsAfterStyleChange(const RenderStyle* oldStyle)
     }
 
     if (!m_scrollDimensionsDirty)
     }
 
     if (!m_scrollDimensionsDirty)
-        updateScrollableAreaSet((hasHorizontalOverflow() || hasVerticalOverflow()) && scrollsOverflow());
+        updateScrollableAreaSet((hasHorizontalOverflow() || hasVerticalOverflow()) && scrollsOverflow() && allowsScrolling());
 }
 
 void RenderLayer::styleChanged(StyleDifference, const RenderStyle* oldStyle)
 }
 
 void RenderLayer::styleChanged(StyleDifference, const RenderStyle* oldStyle)
index 2943332..772e5a4 100644 (file)
@@ -1,3 +1,30 @@
+2012-10-08  Sami Kyostila  <skyostil@chromium.org>
+
+        Some non-scrollable elements are added to non-fast-scrollable region
+        https://bugs.webkit.org/show_bug.cgi?id=97927
+
+        Reviewed by James Robinson.
+
+        Test that a non-scrollable RenderLayer isn't added to the non-fast scrollable
+        region.
+
+        The test in this patch consists of a web page where both the html and
+        body elements clip horizontal overflow. This results in a hierarchy
+        where the body element gets a scrollable RenderLayer. However, that
+        layer isn't (interactively) scrollable because while the scroll geometry
+        is calculated based on the amount of overflow, the scroll bars are
+        hidden because of the "overflow-x: hidden" style.
+
+        Previously this layer would have made the entire page part of the
+        non-fast scrollable region. With this patch,
+        RenderLayer::allowScrolling() is used as a pre-requisite for expanding
+        the region, thus avoiding the problem.
+
+        * tests/ScrollingCoordinatorChromiumTest.cpp:
+        (WebKit::TEST_F):
+        (WebKit):
+        * tests/data/clipped-body.html: Added.
+
 2012-10-08  Sailesh Agrawal  <sail@chromium.org>
 
         Mac Chromium: Ignore system numpad modifier
 2012-10-08  Sailesh Agrawal  <sail@chromium.org>
 
         Mac Chromium: Ignore system numpad modifier
index b4fa4a1..d705701 100644 (file)
@@ -183,6 +183,15 @@ TEST_F(ScrollingCoordinatorChromiumTest, wheelEventHandler)
     ASSERT_TRUE(rootScrollLayer->haveWheelEventHandlers());
 }
 
     ASSERT_TRUE(rootScrollLayer->haveWheelEventHandlers());
 }
 
+TEST_F(ScrollingCoordinatorChromiumTest, clippedBodyTest)
+{
+    registerMockedHttpURLLoad("clipped-body.html");
+    navigateTo(m_baseURL + "clipped-body.html");
+
+    WebLayer* rootScrollLayer = getRootScrollLayer();
+    ASSERT_EQ(0u, rootScrollLayer->nonFastScrollableRegion().size());
+}
+
 #if ENABLE(ACCELERATED_OVERFLOW_SCROLLING)
 TEST_F(ScrollingCoordinatorChromiumTest, touchOverflowScrolling)
 {
 #if ENABLE(ACCELERATED_OVERFLOW_SCROLLING)
 TEST_F(ScrollingCoordinatorChromiumTest, touchOverflowScrolling)
 {
diff --git a/Source/WebKit/chromium/tests/data/clipped-body.html b/Source/WebKit/chromium/tests/data/clipped-body.html
new file mode 100644 (file)
index 0000000..4ebf169
--- /dev/null
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html>
+
+<head>
+  <style>
+    html, body {
+      overflow-x: hidden;
+    }
+    #content {
+      background: silver;
+      width: 1000px;
+      height: 1000px;
+    }
+  </style>
+<head>
+
+<body>
+  <div id="content"></div>
+</body>
+
+</html>