This happens on platforms that do not support independently composited iframes. What I think is happening is that a style change causes a layer in the hidden iframe to composite which bubbles up to the owner element. On the Mac platform pre-WebKit2, allowsIndependentlyCompositedIFrames returns true which stops propagation to the parent's compsitor. On a platform like Android (or if you return false from allowsIndependentlyCompositedIFrames), the parent compositor is turned on which schedules a style change. The parent compositor traverses all the layers and turns off compositing because the iframe layer is not part of any layer list. I have attached a basic test that I used to reproduce the infinite loop on Safari. I had to edit RenderLayerCompositor.cpp to not allow independently composited iframes.
Created attachment 79314 [details] Test case
Created attachment 83918 [details] Patch Add a check if an iframe is visible when deciding if it requires a compositing layer. Without the check we would create a compositing layer, but no backing and that caused the looping.
Comment on attachment 83918 [details] Patch This seems like a specific case of the more general issue that visibility:hidden and compositing don't play nicely together. Bug 29984 covers that. This patch is not correct because it needs to consider ancestor elements of the iframe that are composited and have visibility:hidden.
(In reply to comment #3) > (From update of attachment 83918 [details]) > This seems like a specific case of the more general issue that visibility:hidden and compositing don't play nicely together. Bug 29984 covers that. > > This patch is not correct because it needs to consider ancestor elements of the iframe that are composited and have visibility:hidden. Thank you for your review. I should work on adding a check for the iframe's ancestors' visibility. I tested bugs 29984 and 26770, and they seem to affect only the mac port for Safari. The Chrome port on Linux and on mac handles the test cases properly. Do you have an idea why is that? thanks,
(In reply to comment #4) > I tested bugs 29984 and 26770, and they seem to affect only the mac port for Safari. The Chrome port on Linux and on mac handles the test cases properly. Do you have an idea why is that? I was actually wrong. If I leave only one of the divs in the test from https://bug-29984-attachments.webkit.org/attachment.cgi?id=40479, I see the same infinite loop in QtTestBrower. And there are no iframes in that test.
*** Bug 68605 has been marked as a duplicate of this bug. ***
<rdar://problem/9485360>
There's a general issue with visibility and compositing that I hope to work on soon. iframe-specific patches are only bandaids at this point.
(In reply to comment #8) > There's a general issue with visibility and compositing that I hope to work on soon. iframe-specific patches are only bandaids at this point. I ran into this problem locally. I was wondering if you had filed a more general bug report against this issue? Have you begun work on this? Is this something I can help looking into? Thanks.
Looking at this now.
Here's the smoking gun. Inside of recalcStyle(), notifyIFramesOfCompositingChange() is calling scheduleSetNeedsStyleRecalc(), so we never get out of updateStyleForAllDocuments. #0 WebCore::ContainerNode::scheduleSetNeedsStyleRecalc (this=0x1001d4f00, changeType=WebCore::SyntheticStyleChange) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/ContainerNode.cpp:759 #1 0x00000001038ceeb0 in WebCore::RenderLayerCompositor::notifyIFramesOfCompositingChange (this=0x10017f850) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderLayerCompositor.cpp:1900 #2 0x00000001038ce58c in WebCore::RenderLayerCompositor::enableCompositingMode (this=0x10017f850, enable=true) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderLayerCompositor.cpp:125 #3 0x00000001038d14a4 in WebCore::RenderLayerCompositor::updateBacking (this=0x10017f850, layer=0x1001dcb28, shouldRepaint=WebCore::RenderLayerCompositor::CompositingChangeRepaintNow) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderLayerCompositor.cpp:340 #4 0x00000001038d1b93 in WebCore::RenderLayerCompositor::updateLayerCompositingState (this=0x10017f850, layer=0x1001dcb28, shouldRepaint=WebCore::RenderLayerCompositor::CompositingChangeRepaintNow) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderLayerCompositor.cpp:410 #5 0x00000001038bc222 in WebCore::RenderLayer::styleChanged (this=0x1001dcb28, diff=WebCore::StyleDifferenceEqual, oldStyle=0x123481310) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderLayer.cpp:4204 #6 0x000000010384c48b in WebCore::RenderBoxModelObject::styleDidChange (this=0x1001dc818, diff=WebCore::StyleDifferenceEqual, oldStyle=0x123481310) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderBoxModelObject.cpp:373 #7 0x0000000103834bc0 in WebCore::RenderBox::styleDidChange (this=0x1001dc818, diff=WebCore::StyleDifferenceEqual, oldStyle=0x123481310) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderBox.cpp:345 #8 0x00000001039121fd in WebCore::RenderReplaced::styleDidChange (this=0x1001dc818, diff=WebCore::StyleDifferenceEqual, oldStyle=0x123481310) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderReplaced.cpp:71 #9 0x00000001039f269d in WebCore::RenderWidget::styleDidChange (this=0x1001dc818, diff=WebCore::StyleDifferenceEqual, oldStyle=0x123481310) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderWidget.cpp:215 #10 0x0000000103902023 in WebCore::RenderObject::setStyle (this=0x1001dc818, style=@0x7fff5fbfd160) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderObject.cpp:1727 #11 0x000000010390177b in WebCore::RenderObject::setAnimatableStyle (this=0x1001dc818, style=@0x7fff5fbfd1a0) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderObject.cpp:1640 #12 0x000000010370a9e3 in WebCore::Node::setRenderStyle (this=0x1001d4f00, s=@0x7fff5fbfd2d0) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Node.cpp:1493 #13 0x0000000102d77b49 in WebCore::Element::recalcStyle (this=0x1001d4f00, change=WebCore::Node::NoChange) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Element.cpp:1124 #14 0x0000000102d77e3f in WebCore::Element::recalcStyle (this=0x1001d46f0, change=WebCore::Node::NoChange) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Element.cpp:1158 #15 0x0000000102d77e3f in WebCore::Element::recalcStyle (this=0x1001d2f90, change=WebCore::Node::NoChange) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Element.cpp:1158 #16 0x0000000102d77e3f in WebCore::Element::recalcStyle (this=0x1001d27b0, change=WebCore::Node::NoChange) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Element.cpp:1158 #17 0x0000000102d77e3f in WebCore::Element::recalcStyle (this=0x110e4aa40, change=WebCore::Node::NoChange) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Element.cpp:1158 #18 0x0000000102d77e3f in WebCore::Element::recalcStyle (this=0x100182c10, change=WebCore::Node::NoChange) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Element.cpp:1158 #19 0x0000000102ba7161 in WebCore::Document::recalcStyle (this=0x10082ce00, change=WebCore::Node::NoChange) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Document.cpp:1562 #20 0x0000000102ba81d9 in WebCore::Document::updateStyleIfNeeded (this=0x10082ce00) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Document.cpp:1616 #21 0x0000000102ba82e2 in WebCore::Document::updateStyleForAllDocuments () at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Document.cpp:1633 #22 0x0000000103a40a6e in WebCore::ScheduledAction::execute (this=0x1006b3ec0, document=0x10082ce00) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/bindings/js/ScheduledAction.cpp:131 #23 0x0000000103a40894 in WebCore::ScheduledAction::execute (this=0x1006b3ec0, context=0x10082cff8) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/bindings/js/ScheduledAction.cpp:80 #24 0x0000000102d10636 in WebCore::DOMTimer::fired (this=0x1006b3d80) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/page/DOMTimer.cpp:148 #25 0x0000000103c95bd7 in WebCore::ThreadTimers::sharedTimerFiredInternal (this=0x107a17910) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/platform/ThreadTimers.cpp:115 #26 0x0000000103c959a9 in WebCore::ThreadTimers::sharedTimerFired () at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/platform/ThreadTimers.cpp:93
What's the right solution for this? A flag that indicates we're in the process of recalculating style so that scheduleSetNeedsStyleRecalc() is not called? (In reply to comment #11) > Here's the smoking gun. Inside of recalcStyle(), notifyIFramesOfCompositingChange() is calling scheduleSetNeedsStyleRecalc(), so we never get out of updateStyleForAllDocuments. > > > #0 WebCore::ContainerNode::scheduleSetNeedsStyleRecalc (this=0x1001d4f00, changeType=WebCore::SyntheticStyleChange) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/ContainerNode.cpp:759 > #1 0x00000001038ceeb0 in WebCore::RenderLayerCompositor::notifyIFramesOfCompositingChange (this=0x10017f850) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderLayerCompositor.cpp:1900 > #2 0x00000001038ce58c in WebCore::RenderLayerCompositor::enableCompositingMode (this=0x10017f850, enable=true) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderLayerCompositor.cpp:125 > #3 0x00000001038d14a4 in WebCore::RenderLayerCompositor::updateBacking (this=0x10017f850, layer=0x1001dcb28, shouldRepaint=WebCore::RenderLayerCompositor::CompositingChangeRepaintNow) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderLayerCompositor.cpp:340 > #4 0x00000001038d1b93 in WebCore::RenderLayerCompositor::updateLayerCompositingState (this=0x10017f850, layer=0x1001dcb28, shouldRepaint=WebCore::RenderLayerCompositor::CompositingChangeRepaintNow) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderLayerCompositor.cpp:410 > #5 0x00000001038bc222 in WebCore::RenderLayer::styleChanged (this=0x1001dcb28, diff=WebCore::StyleDifferenceEqual, oldStyle=0x123481310) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderLayer.cpp:4204 > #6 0x000000010384c48b in WebCore::RenderBoxModelObject::styleDidChange (this=0x1001dc818, diff=WebCore::StyleDifferenceEqual, oldStyle=0x123481310) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderBoxModelObject.cpp:373 > #7 0x0000000103834bc0 in WebCore::RenderBox::styleDidChange (this=0x1001dc818, diff=WebCore::StyleDifferenceEqual, oldStyle=0x123481310) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderBox.cpp:345 > #8 0x00000001039121fd in WebCore::RenderReplaced::styleDidChange (this=0x1001dc818, diff=WebCore::StyleDifferenceEqual, oldStyle=0x123481310) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderReplaced.cpp:71 > #9 0x00000001039f269d in WebCore::RenderWidget::styleDidChange (this=0x1001dc818, diff=WebCore::StyleDifferenceEqual, oldStyle=0x123481310) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderWidget.cpp:215 > #10 0x0000000103902023 in WebCore::RenderObject::setStyle (this=0x1001dc818, style=@0x7fff5fbfd160) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderObject.cpp:1727 > #11 0x000000010390177b in WebCore::RenderObject::setAnimatableStyle (this=0x1001dc818, style=@0x7fff5fbfd1a0) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderObject.cpp:1640 > #12 0x000000010370a9e3 in WebCore::Node::setRenderStyle (this=0x1001d4f00, s=@0x7fff5fbfd2d0) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Node.cpp:1493 > #13 0x0000000102d77b49 in WebCore::Element::recalcStyle (this=0x1001d4f00, change=WebCore::Node::NoChange) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Element.cpp:1124 > #14 0x0000000102d77e3f in WebCore::Element::recalcStyle (this=0x1001d46f0, change=WebCore::Node::NoChange) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Element.cpp:1158 > #15 0x0000000102d77e3f in WebCore::Element::recalcStyle (this=0x1001d2f90, change=WebCore::Node::NoChange) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Element.cpp:1158 > #16 0x0000000102d77e3f in WebCore::Element::recalcStyle (this=0x1001d27b0, change=WebCore::Node::NoChange) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Element.cpp:1158 > #17 0x0000000102d77e3f in WebCore::Element::recalcStyle (this=0x110e4aa40, change=WebCore::Node::NoChange) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Element.cpp:1158 > #18 0x0000000102d77e3f in WebCore::Element::recalcStyle (this=0x100182c10, change=WebCore::Node::NoChange) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Element.cpp:1158 > #19 0x0000000102ba7161 in WebCore::Document::recalcStyle (this=0x10082ce00, change=WebCore::Node::NoChange) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Document.cpp:1562 > #20 0x0000000102ba81d9 in WebCore::Document::updateStyleIfNeeded (this=0x10082ce00) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Document.cpp:1616 > #21 0x0000000102ba82e2 in WebCore::Document::updateStyleForAllDocuments () at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Document.cpp:1633 > #22 0x0000000103a40a6e in WebCore::ScheduledAction::execute (this=0x1006b3ec0, document=0x10082ce00) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/bindings/js/ScheduledAction.cpp:131 > #23 0x0000000103a40894 in WebCore::ScheduledAction::execute (this=0x1006b3ec0, context=0x10082cff8) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/bindings/js/ScheduledAction.cpp:80 > #24 0x0000000102d10636 in WebCore::DOMTimer::fired (this=0x1006b3d80) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/page/DOMTimer.cpp:148 > #25 0x0000000103c95bd7 in WebCore::ThreadTimers::sharedTimerFiredInternal (this=0x107a17910) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/platform/ThreadTimers.cpp:115 > #26 0x0000000103c959a9 in WebCore::ThreadTimers::sharedTimerFired () at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/platform/ThreadTimers.cpp:93
I don't know yet. Still working on it.
The real issue here is that we enter compositing via RenderLayerCompositor::updateLayerCompositingState(), but then drop out of compositing again at the end of RenderLayerCompositor::computeCompositingRequirements(), because we didn't hit the composited layer because of visibility:hidden. The larger question is whether we should be in compositing mode if some hidden layer wants to be composited.
(In reply to comment #14) > The larger question is whether we should be in compositing mode if some hidden layer wants to be composited. Seems like a simple tradeoff: 1) Making the layer visible will be smoother if we keep the page in compositing mode. 2) If the layer stays hidden, then performance and memory use will be better if we keep the page out of compositing mode. Is there anything else I’m missing?
Created attachment 111878 [details] Patch
Comment on attachment 111878 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=111878&action=review > Source/WebCore/rendering/RenderLayerCompositor.h:155 > + void layerBecameComposited(const RenderLayer*) { ++m_compositedLayerCount; } > + void layerBecameNonComposited(const RenderLayer*) why are you passing a layer here if you just keep a count? Is the idea to expand this to keeping a Set<> sometimes?
(In reply to comment #17) > (From update of attachment 111878 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=111878&action=review > > > Source/WebCore/rendering/RenderLayerCompositor.h:155 > > + void layerBecameComposited(const RenderLayer*) { ++m_compositedLayerCount; } > > + void layerBecameNonComposited(const RenderLayer*) > > why are you passing a layer here if you just keep a count? Is the idea to expand this to keeping a Set<> sometimes? No, I just thought it made sense for a delegate-type method. It might be useful in future, but I had no specific plans for it.
The function name should start with "has" rather than "have" for consistency with the previously-declared function and with the subject of the phrase (a RenderLayerCompositor).
Comment on attachment 111878 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=111878&action=review > Source/WebCore/rendering/RenderLayerCompositor.cpp:251 > + int nonRootCompositedLayerCount = m_compositedLayerCount; > + if (rootLayer->isComposited()) > + --nonRootCompositedLayerCount; > + > + return nonRootCompositedLayerCount; I would write this as: // We want to return true if only the root layer is composited. return m_compositedLayerCount == rootLayer->isComposited(); Maybe that’s not as clear to you, but it seems pretty clear to me. >>> Source/WebCore/rendering/RenderLayerCompositor.h:155 >>> + void layerBecameNonComposited(const RenderLayer*) >> >> why are you passing a layer here if you just keep a count? Is the idea to expand this to keeping a Set<> sometimes? > > No, I just thought it made sense for a delegate-type method. It might be useful in future, but I had no specific plans for it. In debug builds you should keep a HashSet.
Comment on attachment 111878 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=111878&action=review > Source/WebCore/rendering/RenderLayerCompositor.cpp:322 > + if (childList.isEmpty() && !haveAnyCompositedLayers(updateRoot)) I think a comment should say something about why haveAnyCompositedLayers needs to be called. > Source/WebCore/rendering/RenderLayerCompositor.cpp:788 > // 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) { > + if (layer->isRootLayer() && !childState.m_subtreeIsCompositing && !requiresCompositingLayer(layer) && !m_forceCompositingMode && !haveAnyCompositedLayers(layer)) { I think a comment should say something about why haveAnyCompositedLayers needs to be called. > Source/WebCore/rendering/RenderLayerCompositor.h:259 > + bool haveAnyCompositedLayers(const RenderLayer* rootLayer) const; It’s not clear to me at least that the root layer is not considered when asking if there are any composited layers.
http://trac.webkit.org/changeset/98060
This broke the Windows build, Visual Studio doesn't like this: bool RenderLayerCompositor::hasAnyAdditionalCompositedLayers(const RenderLayer* rootLayer) const { return m_compositedLayerCount > rootLayer->isComposited(); } 5>c:\cygwin\home\jeffm\opensource\source\webcore\rendering\RenderLayerCompositor.cpp(247) : error C2220: warning treated as error - no 'object' file generated 5>c:\cygwin\home\jeffm\opensource\source\webcore\rendering\RenderLayerCompositor.cpp(247) : warning C4804: '>' : unsafe use of type 'bool' in operation
(In reply to comment #23) > This broke the Windows build, Visual Studio doesn't like this: > > > bool RenderLayerCompositor::hasAnyAdditionalCompositedLayers(const RenderLayer* rootLayer) const > { > return m_compositedLayerCount > rootLayer->isComposited(); > } > > > 5>c:\cygwin\home\jeffm\opensource\source\webcore\rendering\RenderLayerCompositor.cpp(247) : error C2220: warning treated as error - no 'object' file generated > 5>c:\cygwin\home\jeffm\opensource\source\webcore\rendering\RenderLayerCompositor.cpp(247) : warning C4804: '>' : unsafe use of type 'bool' in operation VS seems OK with this construct: return m_compositedLayerCount > (rootLayer->isComposited() ? 1 : 0);
(In reply to comment #24) > (In reply to comment #23) > > This broke the Windows build, Visual Studio doesn't like this: > > > > > > bool RenderLayerCompositor::hasAnyAdditionalCompositedLayers(const RenderLayer* rootLayer) const > > { > > return m_compositedLayerCount > rootLayer->isComposited(); > > } > > > > > > 5>c:\cygwin\home\jeffm\opensource\source\webcore\rendering\RenderLayerCompositor.cpp(247) : error C2220: warning treated as error - no 'object' file generated > > 5>c:\cygwin\home\jeffm\opensource\source\webcore\rendering\RenderLayerCompositor.cpp(247) : warning C4804: '>' : unsafe use of type 'bool' in operation > > VS seems OK with this construct: > > return m_compositedLayerCount > (rootLayer->isComposited() ? 1 : 0); Simon fixed this in <http://trac.webkit.org/changeset/98112>