Fix RenderGeometryMap assertion when layers are scrolled during layout
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 6 Nov 2012 17:57:08 +0000 (17:57 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 6 Nov 2012 17:57:08 +0000 (17:57 +0000)
https://bugs.webkit.org/show_bug.cgi?id=101292

Reviewed by Beth Dakin.

Source/WebCore:

When we set RenderLayer scroll positions as part of layout, we don't want
to update compositing layers right away. Updating compositing layers
requires that the entire layer tree has been updated from renderers,
so that the geometry of all RenderLayers can be trusted. When this state
was violated, RenderGeometryMap asserts.

Fix by bailing from updateCompositingLayersAfterScroll() if FrameView
tells us that we're doing layout. A full update of the compositing layers
will happen later anyway.

Test: compositing/geometry/geometry-map-scroll-during-layout-assertion.html

* rendering/RenderLayer.cpp:
(WebCore::frameViewFromLayer):
(WebCore::RenderLayer::updateCompositingLayersAfterScroll):

LayoutTests:

Test that hit the assertion before the fix.

* compositing/geometry/geometry-map-scroll-during-layout-assertion-expected.txt: Added.
* compositing/geometry/geometry-map-scroll-during-layout-assertion.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/compositing/geometry/geometry-map-scroll-during-layout-assertion-expected.txt [new file with mode: 0644]
LayoutTests/compositing/geometry/geometry-map-scroll-during-layout-assertion.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderLayer.cpp

index a1b1806..37c8f87 100644 (file)
@@ -1,3 +1,15 @@
+2012-11-05  Simon Fraser  <simon.fraser@apple.com>
+
+        Fix RenderGeometryMap assertion when layers are scrolled during layout
+        https://bugs.webkit.org/show_bug.cgi?id=101292
+
+        Reviewed by Beth Dakin.
+
+        Test that hit the assertion before the fix.
+
+        * compositing/geometry/geometry-map-scroll-during-layout-assertion-expected.txt: Added.
+        * compositing/geometry/geometry-map-scroll-during-layout-assertion.html: Added.
+
 2012-11-06  Joshua Bell  <jsbell@chromium.org>
 
         [Chromium] Unreviewed gardening - more rebaselines following bug 101115.
diff --git a/LayoutTests/compositing/geometry/geometry-map-scroll-during-layout-assertion-expected.txt b/LayoutTests/compositing/geometry/geometry-map-scroll-during-layout-assertion-expected.txt
new file mode 100644 (file)
index 0000000..0c82888
--- /dev/null
@@ -0,0 +1,3 @@
+This test should not hit an assertion in debug builds.
+
+
diff --git a/LayoutTests/compositing/geometry/geometry-map-scroll-during-layout-assertion.html b/LayoutTests/compositing/geometry/geometry-map-scroll-during-layout-assertion.html
new file mode 100644 (file)
index 0000000..9e58b12
--- /dev/null
@@ -0,0 +1,48 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <style>
+        #wpadminbar {
+            height: 28px;
+            position: fixed;
+            top: 0;
+            left: 0;
+            width: 100%;
+            min-width: 600px;
+            z-index: 99999;
+            background-color: gray;
+        }
+        
+        #overflow {
+            overflow: scroll;
+            height: 300px;
+        }
+        
+        #contents {
+            height: 500px;
+            background-color: green;
+        }
+    </style>
+    <script>
+        if (window.testRunner)
+            testRunner.dumpAsText();
+
+        function doTest()
+        {
+            document.getElementById('overflow').scrollTop = 200;
+            var contents = document.getElementById('contents');
+            contents.style.height = '200' + 'px';
+        }
+        window.addEventListener('load', doTest, false);
+    </script>
+</head>
+<body>
+    <p>This test should not hit an assertion in debug builds.</p>
+    <div id="wpadminbar" class="" role="navigation"></div>
+    <div id="overflow" style="width: 430px; margin-left: 100px; margin-top: 100px;">
+        <div id="contents">
+        </div>
+    </div>
+
+</body>
+</html>
index ead7047..f6a62c5 100644 (file)
@@ -1,3 +1,26 @@
+2012-11-05  Simon Fraser  <simon.fraser@apple.com>
+
+        Fix RenderGeometryMap assertion when layers are scrolled during layout
+        https://bugs.webkit.org/show_bug.cgi?id=101292
+
+        Reviewed by Beth Dakin.
+
+        When we set RenderLayer scroll positions as part of layout, we don't want
+        to update compositing layers right away. Updating compositing layers
+        requires that the entire layer tree has been updated from renderers,
+        so that the geometry of all RenderLayers can be trusted. When this state
+        was violated, RenderGeometryMap asserts.
+        
+        Fix by bailing from updateCompositingLayersAfterScroll() if FrameView
+        tells us that we're doing layout. A full update of the compositing layers
+        will happen later anyway.
+
+        Test: compositing/geometry/geometry-map-scroll-during-layout-assertion.html
+
+        * rendering/RenderLayer.cpp:
+        (WebCore::frameViewFromLayer):
+        (WebCore::RenderLayer::updateCompositingLayersAfterScroll):
+
 2012-11-06  Michael Saboff  <msaboff@apple.com>
 
         quoteCSSString() always creates a 16 bit string
index 3a8c028..1b862c3 100644 (file)
@@ -1885,10 +1885,25 @@ void RenderLayer::scrollRectToVisible(const LayoutRect& rect, const ScrollAlignm
         frameView->resumeScheduledEvents();
 }
 
+static FrameView* frameViewFromLayer(const RenderLayer* layer)
+{
+    Frame* frame = layer->renderer()->frame();
+    if (!frame)
+        return 0;
+
+    return frame->view();
+}
+
 void RenderLayer::updateCompositingLayersAfterScroll()
 {
 #if USE(ACCELERATED_COMPOSITING)
     if (compositor()->inCompositingMode()) {
+        // If we're in the middle of layout, we'll just update compositiong once layout has finished.
+        if (FrameView* frameView = frameViewFromLayer(this)) {
+            if (frameView->isInLayout())
+                return;
+        }
+
         // Our stacking context is guaranteed to contain all of our descendants that may need
         // repositioning, so update compositing layers from there.
         if (RenderLayer* compositingAncestor = stackingContext()->enclosingCompositingLayer()) {