Title: [149388] trunk
Revision
149388
Author
[email protected]
Date
2013-04-30 12:04:34 -0700 (Tue, 30 Apr 2013)

Log Message

Cap max CALayer tree depth to avoid crashes
https://bugs.webkit.org/show_bug.cgi?id=115431
<rdar://problem/13401861>

Source/WebCore:

Reviewed by Tim Horton.

Core Animation can crash if fed deeply nested layer trees.
Avoid this by capping CALayer tree depth at some empirically-determined
level.

Test: compositing/layer-creation/deep-tree.html

* platform/graphics/ca/GraphicsLayerCA.h:
(WebCore::GraphicsLayerCA::CommitState::CommitState): Add treeDepth to the CommitState.
* platform/graphics/ca/GraphicsLayerCA.cpp: Set cMaxLayerTreeDepth to 250
(WebCore::GraphicsLayerCA::flushCompositingStateForThisLayerOnly): We need a dummy CommitState
here. It is not expected that flushCompositingStateForThisLayerOnly() will be called for
a layer in the middle of the hierarchy, only for custom-managed leaf layers, so we don't try
to compute the correct tree depth.
(WebCore::GraphicsLayerCA::recursiveCommitChanges): Pass in the commitState. Since this is
copied for each frame, no need to decrement commitState.treeDepth.
(WebCore::GraphicsLayerCA::commitLayerChangesBeforeSublayers): Increment treeDepth once or
twice. If we've reached max, be sure to set the ChildrenChanged flag. We delay tree truncation
until commitLayerChangesAfterSublayers() since ChildrenChanged can be set again when children
are being processed.
(WebCore::GraphicsLayerCA::commitLayerChangesAfterSublayers):
(WebCore::GraphicsLayerCA::updateSublayerList): If we've hit max depth, just set
empty sublayers.

LayoutTests:

Reviewed by Tim Horton.

Test that makes a very deep tree. Should note crash, and should match
the reference.

* compositing/layer-creation/deep-tree-expected.html: Added.
* compositing/layer-creation/deep-tree.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (149387 => 149388)


--- trunk/LayoutTests/ChangeLog	2013-04-30 18:50:39 UTC (rev 149387)
+++ trunk/LayoutTests/ChangeLog	2013-04-30 19:04:34 UTC (rev 149388)
@@ -1,3 +1,17 @@
+2013-04-30  Simon Fraser  <[email protected]>
+
+        Cap max CALayer tree depth to avoid crashes
+        https://bugs.webkit.org/show_bug.cgi?id=115431
+        <rdar://problem/13401861>
+
+        Reviewed by Tim Horton.
+        
+        Test that makes a very deep tree. Should note crash, and should match
+        the reference.
+
+        * compositing/layer-creation/deep-tree-expected.html: Added.
+        * compositing/layer-creation/deep-tree.html: Added.
+
 2013-04-30  Andreas Kling  <[email protected]>
 
         REGRESSION(r149287): Assertion failure in fast/frames/flattening/iframe-flattening-crash.html

Added: trunk/LayoutTests/compositing/layer-creation/deep-tree-expected.html (0 => 149388)


--- trunk/LayoutTests/compositing/layer-creation/deep-tree-expected.html	                        (rev 0)
+++ trunk/LayoutTests/compositing/layer-creation/deep-tree-expected.html	2013-04-30 19:04:34 UTC (rev 149388)
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+    <style>
+        .box {
+            height: 100px;
+            width: 100px;
+            margin: 10px 0;
+            background-color: blue;
+            -webkit-transform: translateZ(0);
+        }
+        
+        .leaf {
+            background-color: green;
+        }
+    </style>
+</head>
+<body>
+    <div id="before" class="leaf box"></div>
+    <div id="deep" class="box"></div>
+    <div id="after" class="leaf box"></div>
+</body>
+</html>
Property changes on: trunk/LayoutTests/compositing/layer-creation/deep-tree-expected.html
___________________________________________________________________

Added: svn:mime-type

Added: svn:keywords

Added: svn:eol-style

Added: trunk/LayoutTests/compositing/layer-creation/deep-tree.html (0 => 149388)


--- trunk/LayoutTests/compositing/layer-creation/deep-tree.html	                        (rev 0)
+++ trunk/LayoutTests/compositing/layer-creation/deep-tree.html	2013-04-30 19:04:34 UTC (rev 149388)
@@ -0,0 +1,47 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+    <style>
+        .box {
+            height: 100px;
+            width: 100px;
+            margin: 10px 0;
+            background-color: blue;
+            -webkit-transform: translateZ(0);
+        }
+        
+        .leaf {
+            background-color: green;
+        }
+    </style>
+    <script type="text/_javascript_" charset="utf-8">
+        
+        function makeDeepTree(depth, host)
+        {
+            var parent = host;
+            var innerElement;
+            for (var i = 0; i < depth; i++) {
+                var currElement = document.createElement("div");
+                currElement.className = 'box';
+                parent.appendChild(currElement);
+                parent = currElement;
+                innerElement = currElement;
+            }
+            innerElement.classList.add('leaf');
+        }
+        
+        function doTest()
+        {
+            makeDeepTree(500, document.getElementById('deep'));
+        }
+        
+        window.addEventListener('load', doTest, false);
+    </script>
+</head>
+<body>
+    <div id="before" class="leaf box"></div>
+    <div id="deep" class="box"></div>
+    <div id="after" class="leaf box"></div>
+</body>
+</html>
Property changes on: trunk/LayoutTests/compositing/layer-creation/deep-tree.html
___________________________________________________________________

Added: svn:mime-type

Added: svn:keywords

Added: svn:eol-style

Modified: trunk/Source/WebCore/ChangeLog (149387 => 149388)


--- trunk/Source/WebCore/ChangeLog	2013-04-30 18:50:39 UTC (rev 149387)
+++ trunk/Source/WebCore/ChangeLog	2013-04-30 19:04:34 UTC (rev 149388)
@@ -1,3 +1,34 @@
+2013-04-30  Simon Fraser  <[email protected]>
+
+        Cap max CALayer tree depth to avoid crashes
+        https://bugs.webkit.org/show_bug.cgi?id=115431
+        <rdar://problem/13401861>
+
+        Reviewed by Tim Horton.
+        
+        Core Animation can crash if fed deeply nested layer trees.
+        Avoid this by capping CALayer tree depth at some empirically-determined
+        level.
+
+        Test: compositing/layer-creation/deep-tree.html
+
+        * platform/graphics/ca/GraphicsLayerCA.h:
+        (WebCore::GraphicsLayerCA::CommitState::CommitState): Add treeDepth to the CommitState.
+        * platform/graphics/ca/GraphicsLayerCA.cpp: Set cMaxLayerTreeDepth to 250
+        (WebCore::GraphicsLayerCA::flushCompositingStateForThisLayerOnly): We need a dummy CommitState
+        here. It is not expected that flushCompositingStateForThisLayerOnly() will be called for
+        a layer in the middle of the hierarchy, only for custom-managed leaf layers, so we don't try
+        to compute the correct tree depth.
+        (WebCore::GraphicsLayerCA::recursiveCommitChanges): Pass in the commitState. Since this is
+        copied for each frame, no need to decrement commitState.treeDepth.
+        (WebCore::GraphicsLayerCA::commitLayerChangesBeforeSublayers): Increment treeDepth once or
+        twice. If we've reached max, be sure to set the ChildrenChanged flag. We delay tree truncation
+        until commitLayerChangesAfterSublayers() since ChildrenChanged can be set again when children
+        are being processed.
+        (WebCore::GraphicsLayerCA::commitLayerChangesAfterSublayers):
+        (WebCore::GraphicsLayerCA::updateSublayerList): If we've hit max depth, just set
+        empty sublayers.
+
 2013-04-30  Darin Adler  <[email protected]>
 
         Formatting tweaks

Modified: trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp (149387 => 149388)


--- trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp	2013-04-30 18:50:39 UTC (rev 149387)
+++ trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp	2013-04-30 19:04:34 UTC (rev 149388)
@@ -60,6 +60,9 @@
 // texture size limit on all supported hardware.
 static const int cMaxPixelDimension = 2000;
 
+// Derived empirically: <rdar://problem/13401861>
+static const int cMaxLayerTreeDepth = 250;
+
 // If we send a duration of 0 to CA, then it will use the default duration
 // of 250ms. So send a very small value instead.
 static const float cAnimationAlmostZeroDuration = 1e-3f;
@@ -898,10 +901,12 @@
 {
     float pageScaleFactor;
     bool hadChanges = m_uncommittedChanges;
+    
+    CommitState commitState;
 
     FloatPoint offset = computePositionRelativeToBase(pageScaleFactor);
-    commitLayerChangesBeforeSublayers(pageScaleFactor, offset, m_visibleRect);
-    commitLayerChangesAfterSublayers();
+    commitLayerChangesBeforeSublayers(commitState, pageScaleFactor, offset, m_visibleRect);
+    commitLayerChangesAfterSublayers(commitState);
 
     if (hadChanges && client())
         client()->didCommitChangesForLayer(this);
@@ -1059,7 +1064,7 @@
     if (affectedByPageScale)
         baseRelativePosition += m_position;
     
-    commitLayerChangesBeforeSublayers(pageScaleFactor, baseRelativePosition, oldVisibleRect);
+    commitLayerChangesBeforeSublayers(childCommitState, pageScaleFactor, baseRelativePosition, oldVisibleRect);
 
     if (isRunningTransformAnimation()) {
         childCommitState.ancestorHasTransformAnimation = true;
@@ -1068,7 +1073,7 @@
 
     if (m_maskLayer) {
         GraphicsLayerCA* maskLayerCA = static_cast<GraphicsLayerCA*>(m_maskLayer);
-        maskLayerCA->commitLayerChangesBeforeSublayers(pageScaleFactor, baseRelativePosition, maskLayerCA->visibleRect());
+        maskLayerCA->commitLayerChangesBeforeSublayers(childCommitState, pageScaleFactor, baseRelativePosition, maskLayerCA->visibleRect());
     }
 
     const Vector<GraphicsLayer*>& childLayers = children();
@@ -1083,9 +1088,9 @@
         static_cast<GraphicsLayerCA*>(m_replicaLayer)->recursiveCommitChanges(childCommitState, localState, pageScaleFactor, baseRelativePosition, affectedByPageScale);
 
     if (m_maskLayer)
-        static_cast<GraphicsLayerCA*>(m_maskLayer)->commitLayerChangesAfterSublayers();
+        static_cast<GraphicsLayerCA*>(m_maskLayer)->commitLayerChangesAfterSublayers(childCommitState);
 
-    commitLayerChangesAfterSublayers();
+    commitLayerChangesAfterSublayers(childCommitState);
 
     if (affectedByTransformAnimation && client() && m_layer->layerType() == PlatformCALayer::LayerTypeTiledBackingLayer)
         client()->notifyFlushBeforeDisplayRefresh(this);
@@ -1124,10 +1129,18 @@
     return deviceScaleFactor();
 }
 
-void GraphicsLayerCA::commitLayerChangesBeforeSublayers(float pageScaleFactor, const FloatPoint& positionRelativeToBase, const FloatRect& oldVisibleRect)
+void GraphicsLayerCA::commitLayerChangesBeforeSublayers(CommitState& commitState, float pageScaleFactor, const FloatPoint& positionRelativeToBase, const FloatRect& oldVisibleRect)
 {
-    if (!m_uncommittedChanges)
+    ++commitState.treeDepth;
+    if (m_structuralLayer)
+        ++commitState.treeDepth;
+
+    if (!m_uncommittedChanges) {
+        // Ensure that we cap layer depth in commitLayerChangesAfterSublayers().
+        if (commitState.treeDepth > cMaxLayerTreeDepth)
+            m_uncommittedChanges |= ChildrenChanged;
         return;
+    }
 
     bool needTiledLayer = requiresTiledLayer(pageScaleFactor);
     if (needTiledLayer != m_usingTiledBacking)
@@ -1221,15 +1234,19 @@
         // Sublayers may set this flag again, so clear it to avoid always updating sublayers in commitLayerChangesAfterSublayers().
         m_uncommittedChanges &= ~ChildrenChanged;
     }
+
+    // Ensure that we cap layer depth in commitLayerChangesAfterSublayers().
+    if (commitState.treeDepth > cMaxLayerTreeDepth)
+        m_uncommittedChanges |= ChildrenChanged;
 }
 
-void GraphicsLayerCA::commitLayerChangesAfterSublayers()
+void GraphicsLayerCA::commitLayerChangesAfterSublayers(CommitState& commitState)
 {
     if (!m_uncommittedChanges)
         return;
 
     if (m_uncommittedChanges & ChildrenChanged)
-        updateSublayerList();
+        updateSublayerList(commitState.treeDepth > cMaxLayerTreeDepth);
 
     if (m_uncommittedChanges & ReplicatedLayerChanged)
         updateReplicatedLayers();
@@ -1252,8 +1269,13 @@
     m_layer->setName(name());
 }
 
-void GraphicsLayerCA::updateSublayerList()
+void GraphicsLayerCA::updateSublayerList(bool maxLayerDepthReached)
 {
+    if (maxLayerDepthReached) {
+        m_layer->setSublayers(PlatformCALayerList());
+        return;
+    }
+    
     const PlatformCALayerList* customSublayers = m_layer->customSublayers();
 
     PlatformCALayerList structuralLayerChildren;

Modified: trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h (149387 => 149388)


--- trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h	2013-04-30 18:50:39 UTC (rev 149387)
+++ trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h	2013-04-30 19:04:34 UTC (rev 149388)
@@ -133,8 +133,10 @@
 
     struct CommitState {
         bool ancestorHasTransformAnimation;
+        int treeDepth;
         CommitState()
             : ancestorHasTransformAnimation(false)
+            , treeDepth(0)
         { }
     };
     void recursiveCommitChanges(const CommitState&, const TransformState&, float pageScaleFactor = 1, const FloatPoint& positionRelativeToBase = FloatPoint(), bool affectedByPageScale = false);
@@ -223,8 +225,8 @@
         return m_runningAnimations.find(animationName) != m_runningAnimations.end();
     }
 
-    void commitLayerChangesBeforeSublayers(float pageScaleFactor, const FloatPoint& positionRelativeToBase, const FloatRect& oldVisibleRect);
-    void commitLayerChangesAfterSublayers();
+    void commitLayerChangesBeforeSublayers(CommitState&, float pageScaleFactor, const FloatPoint& positionRelativeToBase, const FloatRect& oldVisibleRect);
+    void commitLayerChangesAfterSublayers(CommitState&);
 
     FloatPoint computePositionRelativeToBase(float& pageScale) const;
 
@@ -320,7 +322,7 @@
     
     // All these "update" methods will be called inside a BEGIN_BLOCK_OBJC_EXCEPTIONS/END_BLOCK_OBJC_EXCEPTIONS block.
     void updateLayerNames();
-    void updateSublayerList();
+    void updateSublayerList(bool maxLayerDepthReached = false);
     void updateGeometry(float pixelAlignmentScale, const FloatPoint& positionRelativeToBase);
     void updateTransform();
     void updateChildrenTransform();
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to