- Revision
- 139146
- Author
- [email protected]
- Date
- 2013-01-08 18:57:50 -0800 (Tue, 08 Jan 2013)
Log 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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (139145 => 139146)
--- trunk/LayoutTests/ChangeLog 2013-01-09 02:37:29 UTC (rev 139145)
+++ trunk/LayoutTests/ChangeLog 2013-01-09 02:57:50 UTC (rev 139146)
@@ -1,3 +1,15 @@
+2013-01-08 Alexandru Chiculita <[email protected]>
+
+ 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 <[email protected]>
Make WebGLRenderingContext inherit from ActiveDOMObject
Added: trunk/LayoutTests/compositing/geometry/assert-layout-not-done-expected.txt (0 => 139146)
--- trunk/LayoutTests/compositing/geometry/assert-layout-not-done-expected.txt (rev 0)
+++ trunk/LayoutTests/compositing/geometry/assert-layout-not-done-expected.txt 2013-01-09 02:57:50 UTC (rev 139146)
@@ -0,0 +1 @@
+
Added: trunk/LayoutTests/compositing/geometry/assert-layout-not-done.html (0 => 139146)
--- trunk/LayoutTests/compositing/geometry/assert-layout-not-done.html (rev 0)
+++ trunk/LayoutTests/compositing/geometry/assert-layout-not-done.html 2013-01-09 02:57:50 UTC (rev 139146)
@@ -0,0 +1,27 @@
+<!doctype html>
+<html>
+ <style>
+ .container {
+ position: relative;
+ z-index: 0;
+ top: 123px; /* easy to detect values */
+ left: 123px; /* easy to detect values */
+ }
+ #child {
+ width: 100px;
+ height: 100px;
+ background-color: gray;
+ -webkit-transform: translateZ(0px);
+ }
+ </style>
+ <script>
+ if (window.testRunner)
+ testRunner.dumpAsText(false);
+ </script>
+ <body>
+ <!-- This test passes if there's no ASSERT in debug mode. There should be a gray rectangle on screen. -->
+ <div class="container">
+ <div id="child"></div>
+ </div>
+ </body>
+</html>
\ No newline at end of file
Added: trunk/LayoutTests/compositing/geometry/assert-marquee-timer-expected.txt (0 => 139146)
--- trunk/LayoutTests/compositing/geometry/assert-marquee-timer-expected.txt (rev 0)
+++ trunk/LayoutTests/compositing/geometry/assert-marquee-timer-expected.txt 2013-01-09 02:57:50 UTC (rev 139146)
@@ -0,0 +1 @@
+Counter:
Added: trunk/LayoutTests/compositing/geometry/assert-marquee-timer.html (0 => 139146)
--- trunk/LayoutTests/compositing/geometry/assert-marquee-timer.html (rev 0)
+++ trunk/LayoutTests/compositing/geometry/assert-marquee-timer.html 2013-01-09 02:57:50 UTC (rev 139146)
@@ -0,0 +1,54 @@
+<!doctype html>
+<html>
+ <head>
+ <style>
+ .content {
+ -webkit-transform: translateZ(1px);
+ display: block;
+ width: 100px;
+ height: 30px;
+ background-color: green;
+ }
+ </style>
+ </head>
+ <body>
+ <!-- This test passes if it doesn't trigger an ASSERT in debug mode. It makes sure that the timer
+ used to update the marquee will not run before the layout is done. -->
+
+ <div id="container">
+ <marquee behavior="scroll" direction="left" scrolldelay="1" truespeed="true" scrollamount="1"><span class="content"></span></marquee>
+ <marquee behavior="scroll" direction="left" scrolldelay="2" truespeed="true" scrollamount="1"><span class="content"></span></marquee>
+ <marquee behavior="scroll" direction="left" scrolldelay="3" truespeed="true" scrollamount="1"><span class="content"></span></marquee>
+ <marquee behavior="scroll" direction="left" scrolldelay="4" truespeed="true" scrollamount="1"><span class="content"></span></marquee>
+ <marquee behavior="scroll" direction="left" scrolldelay="5" truespeed="true" scrollamount="1"><span class="content"></span></marquee>
+ <marquee behavior="scroll" direction="left" scrolldelay="6" truespeed="true" scrollamount="1"><span class="content"></span></marquee>
+ <marquee behavior="scroll" direction="left" scrolldelay="7" truespeed="true" scrollamount="1"><span class="content"></span></marquee>
+ <marquee behavior="scroll" direction="left" scrolldelay="8" truespeed="true" scrollamount="1"><span class="content"></span></marquee>
+ </div>
+
+ <p>Counter: <span id="counter"></span></p>
+
+ <script>
+ if (window.testRunner) {
+ testRunner.waitUntilDone();
+ testRunner.dumpAsText(false);
+ }
+
+ var times = 0;
+ // Trigger "pending layouts", that might run after the marquee timer is fired.
+ var counter = document.getElementById("counter");
+ var textNode = document.createTextNode();
+ counter.appendChild(textNode);
+ var interval = setInterval(function() {
+ textNode.nodeValue = Math.random();
+ if ((++times) >= 10) {
+ document.getElementById("container").remove();
+ textNode.nodeValue = "";
+ clearInterval(interval);
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }
+ }, 10);
+ </script>
+ </body>
+</html>
\ No newline at end of file
Modified: trunk/Source/WebCore/ChangeLog (139145 => 139146)
--- trunk/Source/WebCore/ChangeLog 2013-01-09 02:37:29 UTC (rev 139145)
+++ trunk/Source/WebCore/ChangeLog 2013-01-09 02:57:50 UTC (rev 139146)
@@ -1,3 +1,37 @@
+2013-01-08 Alexandru Chiculita <[email protected]>
+
+ 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 <[email protected]>
CanvasRenderingContext2D::setFont argument may reference destroyed object
Modified: trunk/Source/WebCore/dom/Document.cpp (139145 => 139146)
--- trunk/Source/WebCore/dom/Document.cpp 2013-01-09 02:37:29 UTC (rev 139145)
+++ trunk/Source/WebCore/dom/Document.cpp 2013-01-09 02:57:50 UTC (rev 139146)
@@ -1260,6 +1260,14 @@
if (!visualUpdatesAllowed)
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())
view()->updateCompositingLayersAfterLayout();
Modified: trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp (139145 => 139146)
--- trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp 2013-01-09 02:37:29 UTC (rev 139145)
+++ trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp 2013-01-09 02:57:50 UTC (rev 139146)
@@ -408,6 +408,8 @@
if (!m_reevaluateCompositingAfterLayout && !m_compositing)
return;
+ ASSERT(!m_renderView->needsLayout());
+
AnimationUpdateBlock animationUpdateBlock(m_renderView->frameView()->frame()->animation());
bool checkForHierarchyUpdate = m_reevaluateCompositingAfterLayout;
Modified: trunk/Source/WebCore/rendering/RenderMarquee.cpp (139145 => 139146)
--- trunk/Source/WebCore/rendering/RenderMarquee.cpp 2013-01-09 02:37:29 UTC (rev 139145)
+++ trunk/Source/WebCore/rendering/RenderMarquee.cpp 2013-01-09 02:57:50 UTC (rev 139146)
@@ -50,6 +50,7 @@
#include "HTMLMarqueeElement.h"
#include "HTMLNames.h"
#include "RenderLayer.h"
+#include "RenderView.h"
using namespace std;
@@ -254,7 +255,7 @@
void RenderMarquee::timerFired(Timer<RenderMarquee>*)
{
- if (m_layer->renderer()->needsLayout())
+ if (m_layer->renderer()->view()->needsLayout())
return;
if (m_reset) {
