- Revision
- 131964
- Author
- simon.fra...@apple.com
- Date
- 2012-10-19 17:15:23 -0700 (Fri, 19 Oct 2012)
Log Message
Source/WebCore: Fix a hang when combining tile cache layers with preserve-3d or reflections
https://bugs.webkit.org/show_bug.cgi?id=99890
<rdar://problem/12539560>
Reviewed by Dean Jackson.
The new tile cache code added an updateSublayers() call when switching to/from
tiled layers. This confused later sublayer rebuilding, causing us to attempt to
add a layer as a child of itself, causing a hang in CA.
Fix by removing all the explicit calls to updateFoo when updating the structural
layer and switching to/from tiled layers. Instead, we set dirty flags, and rely
on the fact that these flag-dirtying functions get called before the later functions
that process those dirty flags. This is assured by some reordering of the update
function calls.
A final wrinkle is that ensureStructuralLayer() can change the layer that our
parent GraphicsLayer put in its sublayer list. Rather than diddle with that sublayer
list directly like we used to, just call noteSublayersChanged() on the parent, and have
commitLayerChangesAfterSublayers() check the ChildrenChanged and do a second update
of sublayers if necessary (we clear the flag in commitLayerChangesBeforeSublayers(), so
only do this work if a sublayer requested it).
Tests: compositing/tiling/preserve3d-tiled.html
compositing/tiling/reflected-tiled.html
* platform/graphics/ca/GraphicsLayerCA.cpp:
(WebCore::GraphicsLayerCA::commitLayerChangesBeforeSublayers):
(WebCore::GraphicsLayerCA::commitLayerChangesAfterSublayers):
(WebCore::GraphicsLayerCA::ensureStructuralLayer):
(WebCore::GraphicsLayerCA::swapFromOrToTiledLayer):
* platform/graphics/ca/mac/PlatformCALayerMac.mm:
(PlatformCALayer::appendSublayer): Add assertion about adding a layer to itself.
(PlatformCALayer::insertSublayer): Ditto.
(PlatformCALayer::replaceSublayer): Ditto.
LayoutTests: Fix a hang when combining tile cache layers with preserve-3d or reflections
https://bugs.webkit.org/show_bug.cgi?id=99890
Reviewed by Dean Jackson.
Tests that combine tiled layers with preserve-3d and reflections.
* compositing/tiling/preserve3d-tiled-expected.txt: Added.
* compositing/tiling/preserve3d-tiled.html: Added.
* compositing/tiling/reflected-tiled-expected.txt: Added.
* compositing/tiling/reflected-tiled.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (131963 => 131964)
--- trunk/LayoutTests/ChangeLog 2012-10-20 00:00:19 UTC (rev 131963)
+++ trunk/LayoutTests/ChangeLog 2012-10-20 00:15:23 UTC (rev 131964)
@@ -1,3 +1,17 @@
+2012-10-19 Simon Fraser <simon.fra...@apple.com>
+
+ Fix a hang when combining tile cache layers with preserve-3d or reflections
+ https://bugs.webkit.org/show_bug.cgi?id=99890
+
+ Reviewed by Dean Jackson.
+
+ Tests that combine tiled layers with preserve-3d and reflections.
+
+ * compositing/tiling/preserve3d-tiled-expected.txt: Added.
+ * compositing/tiling/preserve3d-tiled.html: Added.
+ * compositing/tiling/reflected-tiled-expected.txt: Added.
+ * compositing/tiling/reflected-tiled.html: Added.
+
2012-10-19 Emil A Eklund <e...@chromium.org>
Unreviewed gardening.
Added: trunk/LayoutTests/compositing/tiling/preserve3d-tiled-expected.txt (0 => 131964)
--- trunk/LayoutTests/compositing/tiling/preserve3d-tiled-expected.txt (rev 0)
+++ trunk/LayoutTests/compositing/tiling/preserve3d-tiled-expected.txt 2012-10-20 00:15:23 UTC (rev 131964)
@@ -0,0 +1 @@
+This test should not crash.
Property changes on: trunk/LayoutTests/compositing/tiling/preserve3d-tiled-expected.txt
___________________________________________________________________
Added: svn:mime-type
Added: svn:keywords
Added: svn:eol-style
Added: trunk/LayoutTests/compositing/tiling/preserve3d-tiled.html (0 => 131964)
--- trunk/LayoutTests/compositing/tiling/preserve3d-tiled.html (rev 0)
+++ trunk/LayoutTests/compositing/tiling/preserve3d-tiled.html 2012-10-20 00:15:23 UTC (rev 131964)
@@ -0,0 +1,30 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+ <style>
+ .tiled {
+ height: 5500px;
+ width: 300px;
+ background-color: silver;
+ border: 1px solid black;
+ }
+
+ .composited {
+ -webkit-transform: translateZ(1px);
+ -webkit-transform-style: preserve-3d;
+ }
+ </style>
+ <script>
+ if (window.testRunner) {
+ testRunner.dumpAsText();
+ }
+ </script>
+</head>
+<body>
+
+<div class="composited tiled">
+ This test should not crash.
+</div>
+</body>
+</html>
Property changes on: trunk/LayoutTests/compositing/tiling/preserve3d-tiled.html
___________________________________________________________________
Added: svn:mime-type
Added: svn:keywords
Added: svn:eol-style
Added: trunk/LayoutTests/compositing/tiling/reflected-tiled-expected.txt (0 => 131964)
--- trunk/LayoutTests/compositing/tiling/reflected-tiled-expected.txt (rev 0)
+++ trunk/LayoutTests/compositing/tiling/reflected-tiled-expected.txt 2012-10-20 00:15:23 UTC (rev 131964)
@@ -0,0 +1,3 @@
+This test should not crash.
+
+
Property changes on: trunk/LayoutTests/compositing/tiling/reflected-tiled-expected.txt
___________________________________________________________________
Added: svn:mime-type
Added: svn:keywords
Added: svn:eol-style
Added: trunk/LayoutTests/compositing/tiling/reflected-tiled.html (0 => 131964)
--- trunk/LayoutTests/compositing/tiling/reflected-tiled.html (rev 0)
+++ trunk/LayoutTests/compositing/tiling/reflected-tiled.html 2012-10-20 00:15:23 UTC (rev 131964)
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+ <style>
+ .tiled {
+ height: 500px;
+ width: 5000px;
+ background-color: silver;
+ border: 5px solid black;
+ background-image: -webkit-linear-gradient(left, green, yellow, orange, blue);
+ -webkit-box-reflect: below 10px;
+ }
+ </style>
+ <script>
+ if (window.testRunner) {
+ testRunner.dumpAsText();
+ }
+ </script>
+</head>
+<body>
+
+<p>This test should not crash.</p>
+
+<div class="tiled"></div>
+
+</body>
+</html>
Property changes on: trunk/LayoutTests/compositing/tiling/reflected-tiled.html
___________________________________________________________________
Added: svn:mime-type
Added: svn:keywords
Added: svn:eol-style
Modified: trunk/Source/WebCore/ChangeLog (131963 => 131964)
--- trunk/Source/WebCore/ChangeLog 2012-10-20 00:00:19 UTC (rev 131963)
+++ trunk/Source/WebCore/ChangeLog 2012-10-20 00:15:23 UTC (rev 131964)
@@ -1,3 +1,41 @@
+2012-10-19 Simon Fraser <simon.fra...@apple.com>
+
+ Fix a hang when combining tile cache layers with preserve-3d or reflections
+ https://bugs.webkit.org/show_bug.cgi?id=99890
+ <rdar://problem/12539560>
+
+ Reviewed by Dean Jackson.
+
+ The new tile cache code added an updateSublayers() call when switching to/from
+ tiled layers. This confused later sublayer rebuilding, causing us to attempt to
+ add a layer as a child of itself, causing a hang in CA.
+
+ Fix by removing all the explicit calls to updateFoo when updating the structural
+ layer and switching to/from tiled layers. Instead, we set dirty flags, and rely
+ on the fact that these flag-dirtying functions get called before the later functions
+ that process those dirty flags. This is assured by some reordering of the update
+ function calls.
+
+ A final wrinkle is that ensureStructuralLayer() can change the layer that our
+ parent GraphicsLayer put in its sublayer list. Rather than diddle with that sublayer
+ list directly like we used to, just call noteSublayersChanged() on the parent, and have
+ commitLayerChangesAfterSublayers() check the ChildrenChanged and do a second update
+ of sublayers if necessary (we clear the flag in commitLayerChangesBeforeSublayers(), so
+ only do this work if a sublayer requested it).
+
+ Tests: compositing/tiling/preserve3d-tiled.html
+ compositing/tiling/reflected-tiled.html
+
+ * platform/graphics/ca/GraphicsLayerCA.cpp:
+ (WebCore::GraphicsLayerCA::commitLayerChangesBeforeSublayers):
+ (WebCore::GraphicsLayerCA::commitLayerChangesAfterSublayers):
+ (WebCore::GraphicsLayerCA::ensureStructuralLayer):
+ (WebCore::GraphicsLayerCA::swapFromOrToTiledLayer):
+ * platform/graphics/ca/mac/PlatformCALayerMac.mm:
+ (PlatformCALayer::appendSublayer): Add assertion about adding a layer to itself.
+ (PlatformCALayer::insertSublayer): Ditto.
+ (PlatformCALayer::replaceSublayer): Ditto.
+
2012-10-19 Chris Fleizach <cfleiz...@apple.com>
AX: aria-hidden=false does not work as expected
Modified: trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp (131963 => 131964)
--- trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp 2012-10-20 00:00:19 UTC (rev 131963)
+++ trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp 2012-10-20 00:15:23 UTC (rev 131964)
@@ -1073,6 +1073,12 @@
if (m_uncommittedChanges & (Preserves3DChanged | ReplicatedLayerChanged))
updateStructuralLayer(pageScaleFactor, positionRelativeToBase);
+ if (m_uncommittedChanges & GeometryChanged)
+ updateGeometry(pageScaleFactor, positionRelativeToBase);
+
+ if (m_uncommittedChanges & DrawsContentChanged)
+ updateLayerDrawsContent(pageScaleFactor, positionRelativeToBase);
+
if (m_uncommittedChanges & NameChanged)
updateLayerNames();
@@ -1088,12 +1094,6 @@
if (m_uncommittedChanges & BackgroundColorChanged) // Needs to happen before ChildrenChanged, and after updating image or video
updateLayerBackgroundColor();
- if (m_uncommittedChanges & ChildrenChanged)
- updateSublayerList();
-
- if (m_uncommittedChanges & GeometryChanged)
- updateGeometry(pageScaleFactor, positionRelativeToBase);
-
if (m_uncommittedChanges & TransformChanged)
updateTransform();
@@ -1103,9 +1103,6 @@
if (m_uncommittedChanges & MasksToBoundsChanged)
updateMasksToBounds();
- if (m_uncommittedChanges & DrawsContentChanged)
- updateLayerDrawsContent(pageScaleFactor, positionRelativeToBase);
-
if (m_uncommittedChanges & ContentsVisibilityChanged)
updateContentsVisibility();
@@ -1148,6 +1145,12 @@
if (m_uncommittedChanges & AcceleratesDrawingChanged)
updateAcceleratesDrawing();
+
+ if (m_uncommittedChanges & ChildrenChanged) {
+ updateSublayerList();
+ // Sublayers may set this flag again, so clear it to avoid always updating sublayers in commitLayerChangesAfterSublayers().
+ m_uncommittedChanges &= ~ChildrenChanged;
+ }
}
void GraphicsLayerCA::commitLayerChangesAfterSublayers()
@@ -1155,6 +1158,9 @@
if (!m_uncommittedChanges)
return;
+ if (m_uncommittedChanges & ChildrenChanged)
+ updateSublayerList();
+
if (m_uncommittedChanges & ReplicatedLayerChanged)
updateReplicatedLayers();
@@ -1414,8 +1420,19 @@
ensureStructuralLayer(structuralLayerPurpose(), pageScaleFactor, positionRelativeToBase);
}
-void GraphicsLayerCA::ensureStructuralLayer(StructuralLayerPurpose purpose, float pageScaleFactor, const FloatPoint& positionRelativeToBase)
+void GraphicsLayerCA::ensureStructuralLayer(StructuralLayerPurpose purpose, float /*pageScaleFactor*/, const FloatPoint& /*positionRelativeToBase*/)
{
+ const LayerChangeFlags structuralLayerChangeFlags = NameChanged
+ | GeometryChanged
+ | TransformChanged
+ | ChildrenTransformChanged
+ | ChildrenChanged
+ | BackfaceVisibilityChanged
+#if ENABLE(CSS_FILTERS)
+ | FiltersChanged
+#endif
+ | OpacityChanged;
+
if (purpose == NoStructuralLayer) {
if (m_structuralLayer) {
// Replace the transformLayer in the parent with this layer.
@@ -1431,17 +1448,7 @@
// Release the structural layer.
m_structuralLayer = 0;
- // Update the properties of m_layer now that we no longer have a structural layer.
- updateGeometry(pageScaleFactor, positionRelativeToBase);
- updateTransform();
- updateChildrenTransform();
-
-#if ENABLE(CSS_FILTERS)
- updateFilters();
-#endif
-
- updateSublayerList();
- updateOpacityOnLayer();
+ m_uncommittedChanges |= structuralLayerChangeFlags;
}
return;
}
@@ -1469,19 +1476,12 @@
if (!structuralLayerChanged)
return;
- updateLayerNames();
+ m_uncommittedChanges |= structuralLayerChangeFlags;
- // Update the properties of the structural layer.
- updateGeometry(pageScaleFactor, positionRelativeToBase);
- updateTransform();
- updateChildrenTransform();
- updateBackfaceVisibility();
-#if ENABLE(CSS_FILTERS)
- // Filters cause flattening, so we should never have a layer for preserve-3d.
- if (purpose != StructuralLayerForPreserves3D)
- updateFilters();
-#endif
-
+ // We've changed the layer that our parent added to its sublayer list, so tell it to update
+ // sublayers again in its commitLayerChangesAfterSublayers().
+ static_cast<GraphicsLayerCA*>(parent())->noteSublayersChanged();
+
// Set properties of m_layer to their default values, since these are expressed on on the structural layer.
FloatPoint point(m_size.width() / 2.0f, m_size.height() / 2.0f);
FloatPoint3D anchorPoint(0.5f, 0.5f, 0);
@@ -1500,17 +1500,7 @@
}
}
- // Move this layer to be a child of the transform layer.
- // If m_layer doesn't have a parent, it means it's the root layer and
- // is likely hosted by something that is not expecting to be changed
- ASSERT(m_layer->superlayer());
- m_layer->superlayer()->replaceSublayer(m_layer.get(), m_structuralLayer.get());
- m_structuralLayer->appendSublayer(m_layer.get());
-
moveOrCopyAnimations(Move, m_layer.get(), m_structuralLayer.get());
-
- updateSublayerList();
- updateOpacityOnLayer();
}
GraphicsLayerCA::StructuralLayerPurpose GraphicsLayerCA::structuralLayerPurpose() const
@@ -2578,7 +2568,7 @@
return m_size.width() * pageScaleFactor > cMaxPixelDimension || m_size.height() * pageScaleFactor > cMaxPixelDimension;
}
-void GraphicsLayerCA::swapFromOrToTiledLayer(bool useTiledLayer, float pageScaleFactor, const FloatPoint& positionRelativeToBase)
+void GraphicsLayerCA::swapFromOrToTiledLayer(bool useTiledLayer, float /*pageScaleFactor*/, const FloatPoint& /*positionRelativeToBase*/)
{
ASSERT(m_layer->layerType() != PlatformCALayer::LayerTypePageTileCacheLayer);
ASSERT(useTiledLayer != m_usingTiledLayer);
@@ -2600,20 +2590,20 @@
if (oldLayer->superlayer())
oldLayer->superlayer()->replaceSublayer(oldLayer.get(), m_layer.get());
- updateGeometry(pageScaleFactor, positionRelativeToBase);
- updateTransform();
- updateChildrenTransform();
- updateSublayerList();
- updateMasksToBounds();
+ m_uncommittedChanges |= ChildrenChanged
+ | GeometryChanged
+ | TransformChanged
+ | ChildrenTransformChanged
+ | MasksToBoundsChanged
+ | ContentsOpaqueChanged
+ | BackfaceVisibilityChanged
+ | BackgroundColorChanged
+ | ContentsScaleChanged
+ | AcceleratesDrawingChanged
#if ENABLE(CSS_FILTERS)
- updateFilters();
+ | FiltersChanged
#endif
- updateContentsOpaque();
- updateBackfaceVisibility();
- updateLayerBackgroundColor();
- updateContentsScale(pageScaleFactor, positionRelativeToBase);
- updateAcceleratesDrawing();
- updateOpacityOnLayer();
+ | OpacityChanged;
#ifndef NDEBUG
String name = String::format("%sCALayer(%p) GraphicsLayer(%p) ", (m_layer->layerType() == PlatformCALayer::LayerTypeWebTiledLayer) ? "Tiled " : "", m_layer->platformLayer(), this) + m_name;
Modified: trunk/Source/WebCore/platform/graphics/ca/mac/PlatformCALayerMac.mm (131963 => 131964)
--- trunk/Source/WebCore/platform/graphics/ca/mac/PlatformCALayerMac.mm 2012-10-20 00:00:19 UTC (rev 131963)
+++ trunk/Source/WebCore/platform/graphics/ca/mac/PlatformCALayerMac.mm 2012-10-20 00:15:23 UTC (rev 131964)
@@ -330,6 +330,7 @@
void PlatformCALayer::appendSublayer(PlatformCALayer* layer)
{
BEGIN_BLOCK_OBJC_EXCEPTIONS
+ ASSERT(m_layer.get() != layer->m_layer.get());
[m_layer.get() addSublayer:layer->m_layer.get()];
END_BLOCK_OBJC_EXCEPTIONS
}
@@ -337,6 +338,7 @@
void PlatformCALayer::insertSublayer(PlatformCALayer* layer, size_t index)
{
BEGIN_BLOCK_OBJC_EXCEPTIONS
+ ASSERT(m_layer.get() != layer->m_layer.get());
[m_layer.get() insertSublayer:layer->m_layer.get() atIndex:index];
END_BLOCK_OBJC_EXCEPTIONS
}
@@ -344,6 +346,7 @@
void PlatformCALayer::replaceSublayer(PlatformCALayer* reference, PlatformCALayer* layer)
{
BEGIN_BLOCK_OBJC_EXCEPTIONS
+ ASSERT(m_layer.get() != layer->m_layer.get());
[m_layer.get() replaceSublayer:reference->m_layer.get() with:layer->m_layer.get()];
END_BLOCK_OBJC_EXCEPTIONS
}
