Changeset 139146 in webkit

Timestamp:

Jan 8, 2013 6:57:50 PM (11 years ago)

Author:

Alexandru Chiculita

Message:

Assert in RenderGeometryMap::mapToContainer
​https://bugs.webkit.org/show_bug.cgi?id=106068

Reviewed by Simon Fraser.

Source/WebCore:

The assert was due to a pending layout, so the values used to compute the layer bounding boxes were incorrect.
That was because of the Document::setVisualUpdatesAllowed mechanism, which triggers a compositor update
and a repaint, but before this patch didn't check whether a layout was pending or not.

Added a check in Document::setVisualUpdatesAllowed for pending layouts and bailed when such case happened.
A layout will come anyway and trigger the correct updates. Couldn't not force an inline layout at that time
as this function is sometimes called really soon, when the WebKit parts are not fully created yet and updates were
calling back into some client callbacks that were not ready.

Also added an assert in RenderLayerCompositor::updateCompositingLayers to check for other cases that might
try to update the layers with a layout pending. That one led to finding an issue in the RenderMarquee, which
was updating on a timer callback. It might happen that a layout is pending while this timer fires and it
tries to update the scroll position of the layers while a layout is still due.

There was already a protection to bail if a layout is pending in RenderMarquee::timerFired, so I've just broadened the scope
to the whole RenderView to catch all the layout requests.

Tests: compositing/geometry/assert-layout-not-done.html

compositing/geometry/assert-marquee-timer.html

• dom/Document.cpp:

(WebCore::Document::setVisualUpdatesAllowed):

• rendering/RenderLayerCompositor.cpp:

(WebCore::RenderLayerCompositor::updateCompositingLayers):

• rendering/RenderMarquee.cpp:

(WebCore::RenderMarquee::timerFired):

LayoutTests:

• compositing/geometry/assert-layout-not-done-expected.txt: Added.
• compositing/geometry/assert-layout-not-done.html: Added. Testing for the case when compositor is triggered before the first layout.
• compositing/geometry/assert-marquee-timer-expected.txt: Added.
• compositing/geometry/assert-marquee-timer.html: Added. Tested for the case when the marquee might trigger compositor updates, while a layout is still pending.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/compositing/geometry/assert-layout-not-done-expected.txt (added)
• LayoutTests/compositing/geometry/assert-layout-not-done.html (added)
• LayoutTests/compositing/geometry/assert-marquee-timer-expected.txt (added)
• LayoutTests/compositing/geometry/assert-marquee-timer.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/dom/Document.cpp (modified) (1 diff)
• Source/WebCore/rendering/RenderLayerCompositor.cpp (modified) (1 diff)
• Source/WebCore/rendering/RenderMarquee.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2013-01-08  Alexandru Chiculita  <achicu@adobe.com>
+
+        Assert in RenderGeometryMap::mapToContainer
+        https://bugs.webkit.org/show_bug.cgi?id=106068
+
+        Reviewed by Simon Fraser.
+
+        * compositing/geometry/assert-layout-not-done-expected.txt: Added.
+        * compositing/geometry/assert-layout-not-done.html: Added. Testing for the case when compositor is triggered before the first layout.
+        * compositing/geometry/assert-marquee-timer-expected.txt: Added.
+        * compositing/geometry/assert-marquee-timer.html: Added. Tested for the case when the marquee might trigger compositor updates, while a layout is still pending.
+
 2013-01-08  Brandon Jones  <bajones@chromium.org>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-01-08  Alexandru Chiculita  <achicu@adobe.com>
+
+        Assert in RenderGeometryMap::mapToContainer
+        https://bugs.webkit.org/show_bug.cgi?id=106068
+
+        Reviewed by Simon Fraser.
+
+        The assert was due to a pending layout, so the values used to compute the layer bounding boxes were incorrect.
+        That was because of the Document::setVisualUpdatesAllowed mechanism, which triggers a compositor update
+        and a repaint, but before this patch didn't check whether a layout was pending or not.
+
+        Added a check in Document::setVisualUpdatesAllowed for pending layouts and bailed when such case happened.
+        A layout will come anyway and trigger the correct updates. Couldn't not force an inline layout at that time
+        as this function is sometimes called really soon, when the WebKit parts are not fully created yet and updates were
+        calling back into some client callbacks that were not ready.
+
+        Also added an assert in RenderLayerCompositor::updateCompositingLayers to check for other cases that might
+        try to update the layers with a layout pending. That one led to finding an issue in the RenderMarquee, which
+        was updating on a timer callback. It might happen that a layout is pending while this timer fires and it 
+        tries to update the scroll position of the layers while a layout is still due.
+
+        There was already a protection to bail if a layout is pending in RenderMarquee::timerFired, so I've just broadened the scope
+        to the whole RenderView to catch all the layout requests.
+
+        Tests: compositing/geometry/assert-layout-not-done.html
+               compositing/geometry/assert-marquee-timer.html
+
+        * dom/Document.cpp:
+        (WebCore::Document::setVisualUpdatesAllowed):
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::updateCompositingLayers):
+        * rendering/RenderMarquee.cpp:
+        (WebCore::RenderMarquee::timerFired):
+
 2013-01-08  Justin Novosad  <junov@google.com>
 

trunk/Source/WebCore/dom/Document.cpp

@@ -1261 +1261 @@
         return;
 
+    FrameView* frameView = view();
+    bool needsLayout = frameView && renderer() && (frameView->layoutPending() || renderer()->needsLayout());
+    if (needsLayout) {
+        // There might be a layout pending, so make sure we don't update the screen with bogus data.
+        // The layout will actually update the compositing layers and repaint if needed.
+        return;
+    }
+
 #if USE(ACCELERATED_COMPOSITING)
     if (view())

trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp

@@ -409 +409 @@
         return;
 
+    ASSERT(!m_renderView->needsLayout());
+
     AnimationUpdateBlock animationUpdateBlock(m_renderView->frameView()->frame()->animation());
 

trunk/Source/WebCore/rendering/RenderMarquee.cpp

@@ -51 +51 @@
 #include "HTMLNames.h"
 #include "RenderLayer.h"
+#include "RenderView.h"
 
 using namespace std;
@@ -255 +256 @@
 void RenderMarquee::timerFired(Timer<RenderMarquee>*)
 {
-    if (m_layer->renderer()->needsLayout())
+    if (m_layer->renderer()->view()->needsLayout())
         return;