Bug 59026 – RenderLayers mutate the GraphicsLayer tree during GraphicsLayerClient::paintContents callback

• Status: NEW
• Component: New Bugs

[James Robinson]

The composited layer paint callback can call RenderLayer::paintLayer() which calls RenderLayer::updateCompositingLayers() (http://trac.webkit.org/browser/trunk/Source/WebCore/rendering/RenderLayer.cpp#L2577) which can cause arbitrary mutations to the GraphicsLayer tree or turn compositing off.  This likely produces incorrect compositing results everywhere and causes crashes in the Chromium compositor implementation as it is currently not safe to arbitrary mutations during painting.  Here's one example callstack showing the compositor becoming disabled during paint (chromium-specific bits of the stack omitted):

 #9 0x00000000006a8fb7 in WebCore::RenderLayerCompositor::enableCompositingMode
    (this=0x7fffebcb8640, enable=false)
    at third_party/WebKit/Source/WebCore/rendering/RenderLayerCompositor.cpp:138
#10 0x00000000006ab160 in WebCore::RenderLayerCompositor::computeCompositingRequirements (this=0x7fffebcb8640, layer=0x7fffebc89c78, 
    overlapMap=0x7fffffffa960, compositingState=..., 
    layersChanged=@0x7fffffffa95c)
    at third_party/WebKit/Source/WebCore/rendering/RenderLayerCompositor.cpp:726
#11 0x00000000006a9580 in WebCore::RenderLayerCompositor::updateCompositingLayers (this=0x7fffebcb8640, 
    updateType=WebCore::CompositingUpdateOnPaitingOrHitTest, 
    updateRoot=0x7fffebc89c78)
    at third_party/WebKit/Source/WebCore/rendering/RenderLayerCompositor.cpp:287
#12 0x0000000000694f7c in WebCore::RenderLayer::updateCompositingAndLayerListsIfNeeded (this=0x7fffebc89c78)
    at third_party/WebKit/Source/WebCore/rendering/RenderLayer.cpp:3880
#13 0x000000000068ecf0 in WebCore::RenderLayer::paintLayer (
    this=0x7fffebc89c78, rootLayer=0x7fffebc89c78, p=0x7fffeb40a6c0, 
    paintDirtyRect=..., paintBehavior=0, paintingRoot=0x0, 
    overlapTestRequests=0x7fffffffad80, paintFlags=0)
    at third_party/WebKit/Source/WebCore/rendering/RenderLayer.cpp:2578
#14 0x000000000068e281 in WebCore::RenderLayer::paint (this=0x7fffebc89c78, 
    p=0x7fffeb40a6c0, damageRect=..., paintBehavior=0, paintingRoot=0x0)
    at third_party/WebKit/Source/WebCore/rendering/RenderLayer.cpp:2425
#15 0x0000000000cc7b59 in WebCore::FrameView::paintContents (
    this=0x7fffeb710600, p=0x7fffeb40a6c0, rect=...)
    at third_party/WebKit/Source/WebCore/page/FrameView.cpp:2355
#16 0x000000000289322f in WebCore::ScrollView::paint (this=0x7fffeb710600, 
    context=0x7fffeb40a6c0, rect=...)
    at third_party/WebKit/Source/WebCore/platform/ScrollView.cpp:1005
#17 0x0000000000710ef7 in WebCore::RenderWidget::paint (this=0x7ffff7ecee18, 
    paintInfo=..., tx=0, ty=0)
    at third_party/WebKit/Source/WebCore/rendering/RenderWidget.cpp:301
#18 0x00000000006a4831 in WebCore::RenderLayerBacking::paintIntoLayer (
    this=0x7fffeb6f8070, rootLayer=0x7fffebc9cdd8, context=0x7fffeb40a6c0, 
    paintDirtyRect=..., paintBehavior=0, 
    paintingPhase=WebCore::GraphicsLayerPaintAll, paintingRoot=0x7ffff7ecee18)
    at third_party/WebKit/Source/WebCore/rendering/RenderLayerBacking.cpp:1153
#19 0x00000000006a4ea9 in WebCore::RenderLayerBacking::paintContents (
    this=0x7fffeb6f8070, graphicsLayer=0x7fffeb6e1800, context=..., 
    paintingPhase=WebCore::GraphicsLayerPaintAll, clip=...)
    at third_party/WebKit/Source/WebCore/rendering/RenderLayerBacking.cpp:1224
#20 0x0000000002920104 in WebCore::GraphicsLayer::paintGraphicsLayerContents (
    this=0x7fffeb6e1800, context=..., clip=...)
    at third_party/WebKit/Source/WebCore/platform/graphics/GraphicsLayer.cpp:248

[Simon Fraser (smfr)]

What's more interesting are stacks where the z-order lists are dirtied during painting.

[James Robinson]

(In reply to comment #1)
> What's more interesting are stacks where the z-order lists are dirtied during painting.

I don't think the z-order lists are being dirtied during painting, but they can definitely be dirty when calling paintLayer() on some composited layer and updating the z-order lists can cause the compositing requirements to change.  In the software rendering path, updating the z-order list for a RenderLayer is fine so long as it completes before iterating into any children of the RenderLayer, but in the compositing case it can cause the compositing requirements to change in weird ways.

If I understand correctly we want to build the z-order lists lazily (since we don't need them for normal layout, just rendering), but we definitely need them before painting.  I think the best solution here is to add a call to rebuild the z-order lists for all RenderLayers in the tree that the compositor can invoke before it calls paintContents() for any GraphicsLayers in the tree.

[James Robinson]

Posted a patch to https://bugs.webkit.org/show_bug.cgi?id=59159 to resolve the issue in Chromium.  I haven't been able to reproduce bad behavior on the mac, but I'm not sure if this is due to bad timing or a lack of understanding of the CoreAnimation runloop update stuff.