Title: [98060] trunk
Revision
98060
Author
simon.fra...@apple.com
Date
2011-10-20 20:02:05 -0700 (Thu, 20 Oct 2011)

Log Message

Hidden composited iframes cause infinite loop
https://bugs.webkit.org/show_bug.cgi?id=52655

Source/WebCore:

Reviewed by Darin Adler.

visibility:hidden is problematic for compositing, because it causes
RenderLayers to be removed from the z-order layer tree. This confuses
RenderLayerCompositor in several ways; it never sees these layers
when traversing the tree as it computes compositing requirements, or
rebuilds the layer tree.

This is a particular problem with composited iframes. When an iframe
becomes composited, scheduleSetNeedsStyleRecalc() is called on that
iframe's ownerElement in the parent document. If this happens inside
Document::updateStyleForAllDocuments(), we get into an infinite loop
because notifyIFramesOfCompositingChange() queues up style update as we
bounce in and out of compositing mode, so documentsThatNeedStyleRecalc
never empties out.

This is an initial, conservative fix that doesn't attempt to fix all
the issues with visibility. It changes RenderLayerCompositor to count
the number of compositing RenderLayers, and to not leave compositing
mode if there are any (even if they are hidden, so not hit while
traversing the z-order tree). This avoids the infinite loop.

Test: compositing/visibility/hidden-iframe.html

* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::ensureBacking):
(WebCore::RenderLayer::clearBacking):
* rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::RenderLayerCompositor):
(WebCore::RenderLayerCompositor::hasAnyAdditionalCompositedLayers):
(WebCore::RenderLayerCompositor::updateCompositingLayers):
(WebCore::RenderLayerCompositor::computeCompositingRequirements):
* rendering/RenderLayerCompositor.h:
(WebCore::RenderLayerCompositor::layerBecameComposited):
(WebCore::RenderLayerCompositor::layerBecameNonComposited):

LayoutTests:

Reviewed by Darin Adler.

Test with a visibility:hidden iframe, whose subframe becomes composited.

* compositing/visibility/hidden-iframe-expected.txt: Added.
* compositing/visibility/hidden-iframe.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (98059 => 98060)


--- trunk/LayoutTests/ChangeLog	2011-10-21 02:37:01 UTC (rev 98059)
+++ trunk/LayoutTests/ChangeLog	2011-10-21 03:02:05 UTC (rev 98060)
@@ -1,3 +1,15 @@
+2011-10-20  Simon Fraser  <simon.fra...@apple.com>
+
+        Hidden composited iframes cause infinite loop
+        https://bugs.webkit.org/show_bug.cgi?id=52655
+
+        Reviewed by Darin Adler.
+        
+        Test with a visibility:hidden iframe, whose subframe becomes composited.
+
+        * compositing/visibility/hidden-iframe-expected.txt: Added.
+        * compositing/visibility/hidden-iframe.html: Added.
+
 2011-10-20  Dirk Pranke  <dpra...@chromium.org>
 
         Suppress inspector failures for the moment.

Added: trunk/LayoutTests/compositing/visibility/hidden-iframe-expected.txt (0 => 98060)


--- trunk/LayoutTests/compositing/visibility/hidden-iframe-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/compositing/visibility/hidden-iframe-expected.txt	2011-10-21 03:02:05 UTC (rev 98060)
@@ -0,0 +1,3 @@
+PASS: test did not hang.
+
+

Added: trunk/LayoutTests/compositing/visibility/hidden-iframe.html (0 => 98060)


--- trunk/LayoutTests/compositing/visibility/hidden-iframe.html	                        (rev 0)
+++ trunk/LayoutTests/compositing/visibility/hidden-iframe.html	2011-10-21 03:02:05 UTC (rev 98060)
@@ -0,0 +1,31 @@
+<html>
+  <head>
+    <style>
+      iframe {
+        visibility: hidden;
+        position: absolute;
+      }
+    </style>
+    <script>
+      if (window.layoutTestController) {
+          layoutTestController.dumpAsText();
+          layoutTestController.waitUntilDone();
+      }
+
+      // Called from subframe.
+      function testDone()
+      {
+        // This timeout is necessary to detect the hang.
+        window.setTimeout(function() {
+          document.getElementById('results').innerText = 'PASS: test did not hang.';
+          if (window.layoutTestController)
+            layoutTestController.notifyDone();
+        }, 0);
+      }
+    </script>
+  </head>
+  <body>
+    <p id="results">This test should not hang.</p>
+    <iframe src=""
+  </body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (98059 => 98060)


--- trunk/Source/WebCore/ChangeLog	2011-10-21 02:37:01 UTC (rev 98059)
+++ trunk/Source/WebCore/ChangeLog	2011-10-21 03:02:05 UTC (rev 98060)
@@ -1,3 +1,44 @@
+2011-10-20  Simon Fraser  <simon.fra...@apple.com>
+
+        Hidden composited iframes cause infinite loop
+        https://bugs.webkit.org/show_bug.cgi?id=52655
+
+        Reviewed by Darin Adler.
+        
+        visibility:hidden is problematic for compositing, because it causes
+        RenderLayers to be removed from the z-order layer tree. This confuses
+        RenderLayerCompositor in several ways; it never sees these layers
+        when traversing the tree as it computes compositing requirements, or
+        rebuilds the layer tree.
+        
+        This is a particular problem with composited iframes. When an iframe
+        becomes composited, scheduleSetNeedsStyleRecalc() is called on that
+        iframe's ownerElement in the parent document. If this happens inside
+        Document::updateStyleForAllDocuments(), we get into an infinite loop
+        because notifyIFramesOfCompositingChange() queues up style update as we
+        bounce in and out of compositing mode, so documentsThatNeedStyleRecalc
+        never empties out.
+        
+        This is an initial, conservative fix that doesn't attempt to fix all
+        the issues with visibility. It changes RenderLayerCompositor to count
+        the number of compositing RenderLayers, and to not leave compositing
+        mode if there are any (even if they are hidden, so not hit while
+        traversing the z-order tree). This avoids the infinite loop.
+
+        Test: compositing/visibility/hidden-iframe.html
+
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::ensureBacking):
+        (WebCore::RenderLayer::clearBacking):
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::RenderLayerCompositor):
+        (WebCore::RenderLayerCompositor::hasAnyAdditionalCompositedLayers):
+        (WebCore::RenderLayerCompositor::updateCompositingLayers):
+        (WebCore::RenderLayerCompositor::computeCompositingRequirements):
+        * rendering/RenderLayerCompositor.h:
+        (WebCore::RenderLayerCompositor::layerBecameComposited):
+        (WebCore::RenderLayerCompositor::layerBecameNonComposited):
+
 2011-10-20  Antoine Labour  <pi...@chromium.org>
 
         Make WebCore depend on translator_glsl instead of translator_common

Modified: trunk/Source/WebCore/rendering/RenderLayer.cpp (98059 => 98060)


--- trunk/Source/WebCore/rendering/RenderLayer.cpp	2011-10-21 02:37:01 UTC (rev 98059)
+++ trunk/Source/WebCore/rendering/RenderLayer.cpp	2011-10-21 03:02:05 UTC (rev 98060)
@@ -3788,13 +3788,17 @@
 #if USE(ACCELERATED_COMPOSITING)
 RenderLayerBacking* RenderLayer::ensureBacking()
 {
-    if (!m_backing)
+    if (!m_backing) {
         m_backing = adoptPtr(new RenderLayerBacking(this));
+        compositor()->layerBecameComposited(this);
+    }
     return m_backing.get();
 }
 
 void RenderLayer::clearBacking()
 {
+    if (m_backing && !renderer()->documentBeingDestroyed())
+        compositor()->layerBecameNonComposited(this);
     m_backing.clear();
 }
 

Modified: trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp (98059 => 98060)


--- trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp	2011-10-21 02:37:01 UTC (rev 98059)
+++ trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp	2011-10-21 03:02:05 UTC (rev 98060)
@@ -95,6 +95,7 @@
     , m_updateCompositingLayersTimer(this, &RenderLayerCompositor::updateCompositingLayersTimerFired)
     , m_hasAcceleratedCompositing(true)
     , m_compositingTriggers(static_cast<ChromeClient::CompositingTriggerFlags>(ChromeClient::AllTriggers))
+    , m_compositedLayerCount(0)
     , m_showDebugBorders(false)
     , m_showRepaintCounter(false)
     , m_compositingConsultsOverlap(true)
@@ -241,6 +242,11 @@
     updateCompositingLayers();
 }
 
+bool RenderLayerCompositor::hasAnyAdditionalCompositedLayers(const RenderLayer* rootLayer) const
+{
+    return m_compositedLayerCount > rootLayer->isComposited();
+}
+
 void RenderLayerCompositor::updateCompositingLayers(CompositingUpdateType updateType, RenderLayer* updateRoot)
 {
     m_updateCompositingLayersTimer.stop();
@@ -309,7 +315,9 @@
 
         // Host the document layer in the RenderView's root layer.
         if (updateRoot == rootRenderLayer()) {
-            if (childList.isEmpty())
+            // Even when childList is empty, don't drop out of compositing mode if there are
+            // composited layers that we didn't hit in our traversal (e.g. because of visibility:hidden).
+            if (childList.isEmpty() && !hasAnyAdditionalCompositedLayers(updateRoot))
                 destroyRootLayer();
             else
                 m_rootContentLayer->setChildren(childList);
@@ -774,8 +782,9 @@
     }
 
     // If we're back at the root, and no other layers need to be composited, and the root layer itself doesn't need
-    // to be composited, then we can drop out of compositing mode altogether.
-    if (layer->isRootLayer() && !childState.m_subtreeIsCompositing && !requiresCompositingLayer(layer) && !m_forceCompositingMode) {
+    // to be composited, then we can drop out of compositing mode altogether. However, don't drop out of compositing mode
+    // if there are composited layers that we didn't hit in our traversal (e.g. because of visibility:hidden).
+    if (layer->isRootLayer() && !childState.m_subtreeIsCompositing && !requiresCompositingLayer(layer) && !m_forceCompositingMode && !hasAnyAdditionalCompositedLayers(layer)) {
         enableCompositingMode(false);
         willBeComposited = false;
     }

Modified: trunk/Source/WebCore/rendering/RenderLayerCompositor.h (98059 => 98060)


--- trunk/Source/WebCore/rendering/RenderLayerCompositor.h	2011-10-21 02:37:01 UTC (rev 98059)
+++ trunk/Source/WebCore/rendering/RenderLayerCompositor.h	2011-10-21 03:02:05 UTC (rev 98060)
@@ -151,6 +151,13 @@
 
     void clearBackingForAllLayers();
     
+    void layerBecameComposited(const RenderLayer*) { ++m_compositedLayerCount; }
+    void layerBecameNonComposited(const RenderLayer*)
+    {
+        ASSERT(m_compositedLayerCount > 0);
+        --m_compositedLayerCount;
+    }
+    
     void didStartAcceleratedAnimation(CSSPropertyID);
     
 #if ENABLE(VIDEO)
@@ -248,6 +255,8 @@
 
     bool layerHas3DContent(const RenderLayer*) const;
     bool hasNonIdentity3DTransform(RenderObject*) const;
+    
+    bool hasAnyAdditionalCompositedLayers(const RenderLayer* rootLayer) const;
 
     void ensureRootLayer();
     void destroyRootLayer();
@@ -290,6 +299,7 @@
     bool m_hasAcceleratedCompositing;
     ChromeClient::CompositingTriggerFlags m_compositingTriggers;
 
+    int m_compositedLayerCount;
     bool m_showDebugBorders;
     bool m_showRepaintCounter;
     bool m_compositingConsultsOverlap;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to