Bug 38954 – ASSERTION FAILED: rootLayer == m_clipRectsRoot with this testcase

• Component: Layout and Rendering
• Status: NEW

[Simon Fraser (smfr)]

Created attachment 55781 [details]
Testcase

The attached testcase asserts:

ASSERTION FAILED: rootLayer == m_clipRectsRoot
(/Volumes/InternalData/Development/webkit/OpenSource/WebCore/rendering/RenderLayer.cpp:2795 void WebCore::RenderLayer::updateClipRects(const WebCore::RenderLayer*))

If I remove the position: relative on the body, it no longer asserts.

[Simon Fraser (smfr)]

The assertion happens because RenderLayer::backgroundClipRect() asks the parent() for clip rects, but this can skip over compositing layer boundaries.

For example, in this tree:

layer 0x1275a8e58 at (0,0) size 599x332 (composited, bounds at (0,0) size 599x332)
  RenderView 0x10fb2cf98 at (0,0) size 599x332
 positive z-order list(1)
  layer 0x127b80d58 at (0,0) size 599x118 layerType: background only (composited, bounds at (0,0) size 599x332)
   negative z-order list(1)
    layer 0x12711b538 at (8,8) size 20x20
      RenderBlock (positioned) 0x1275f8228 zI: -1 {DIV} at (0,0) size 20x20 [bgcolor=#0000FF]
     positive z-order list(1)
      layer 0x127b49c68 at (8,8) size 20x0 (composited, bounds at (0,0) size 20x0)
        RenderBlock 0x1275f6b98 {DIV} at (0,0) size 20x0
  layer 0x127b80d58 at (0,0) size 599x118 layerType: foreground only (composited, bounds at (0,0) size 599x332)
    RenderBlock 0x127b30408 {HTML} at (0,0) size 599x118
   positive z-order list(1)
    layer 0x10fb2a098 at (8,8) size 583x102 (composited, bounds at (0,0) size 583x102)
      RenderBody 0x127b0d9b8 {BODY} at (8,8) size 583x102 [bgcolor=#C0C0C0]
     normal flow list(1)
      layer 0x1271e91c8 at (8,8) size 202x102 clip at (9,9) size 200x100
        RenderBlock 0x124aed948 {DIV} at (0,0) size 202x102 [border: (1px solid #000000)]

we cache clip rects on 0x10fb2a098 relative to itself (because it's composited). But then when painting 0x12711b538 relative to root 0x127b80d58, we ask for clip rects on parent(), which is 0x1271e91c8.

I think we should fix bug 38959 before this one, since currently the rendering of the testcase is not correct.

[Simon Fraser (smfr)]

Now happens on google images.

[Simon Fraser (smfr)]

*** Bug 70400 has been marked as a duplicate of this bug. ***

[Simon Fraser (smfr)]

*** Bug 71174 has been marked as a duplicate of this bug. ***

[Rob Buis]

Created attachment 118120 [details]
Potential fix

Posting this on behalf of George Staikos. Is this the right approach for a fix?

[Early Warning System Bot]

Comment on attachment 118120 [details]
Potential fix

Attachment 118120 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/10742118

[Gyuyoung Kim]

Comment on attachment 118120 [details]
Potential fix

Attachment 118120 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/10736878

[WebKit Review Bot]

Comment on attachment 118120 [details]
Potential fix

Attachment 118120 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10742120

[Simon Fraser (smfr)]

Comment on attachment 118120 [details]
Potential fix

View in context: https://bugs.webkit.org/attachment.cgi?id=118120&action=review

> Source/WebCore/ChangeLog:8
> +        The cache is not necessarily valid at this point.

Why?

[Rob Buis]

Created attachment 118400 [details]
Patch

[Dirk Schulze]

Comment on attachment 118400 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=118400&action=review

r- because of missing Test.

> Source/WebCore/ChangeLog:4
> +        ASSERTION FAILED: rootLayer == m_clipRectsRoot with this testcase
> +        https://bugs.webkit.org/show_bug.cgi?id=38954

Even if that is an assert, it needs a crash test.

> Source/WebCore/ChangeLog:8
> +        The cache is not necessarily valid at his point.

You haven't answered to Simons question.

[Leo Yang]

Created attachment 118961 [details]
Another potential fix

[Simon Fraser (smfr)]

Comment on attachment 118961 [details]
Another potential fix

View in context: https://bugs.webkit.org/attachment.cgi?id=118961&action=review

> Source/WebCore/ChangeLog:8
> +        When calling RenderLayer::backgroundClipRect() the parent might be composited.
> +        In this case we might be calling it with *rootLayer* which is not the previous
> +        cached root clipping layer. So use temporary clip rects instread.

But who calls backgroundClipRect() like this? What's the stack for the backgroundClipRect() call that makes things go bad?

[Leo Yang]

(In reply to comment #13)
> (From update of attachment 118961 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=118961&action=review
> 
> > Source/WebCore/ChangeLog:8
> > +        When calling RenderLayer::backgroundClipRect() the parent might be composited.
> > +        In this case we might be calling it with *rootLayer* which is not the previous
> > +        cached root clipping layer. So use temporary clip rects instread.
> 
> But who calls backgroundClipRect() like this? What's the stack for the backgroundClipRect() call that makes things go bad?

-Layer tree

layer 7ffee4d0 at (0,0) size 1024x569 (composited, bounds at (0,0) size 1024x569)
  RenderView 777ced08 at (0,0) size 1024x569
 positive z-order list(1)
  layer 77a25160 at (0,0) size 1024x118 layerType: background only (composited, bounds at (0,0) size 1024x569)
   negative z-order list(1)
    layer 777c35b0 at (8,8) size 10x10
      RenderBlock (positioned) 777fd698 zI: -10 {DIV} at (0,0) size 10x10 [bgcolor=#0000FF] class="container"
     positive z-order list(1)
      layer 2fd23938 at (8,8) size 10x0 (composited, bounds at (0,0) size 10x0)
        RenderBlock 7bdee498 {DIV} at (0,0) size 10x0 class="compositing"
  layer 77a25160 at (0,0) size 1024x118 layerType: foreground only (composited, bounds at (0,0) size 1024x569)
    RenderBlock 777cb150 {HTML} at (0,0) size 1024x118
   positive z-order list(1)
    layer 2fd2acf8 at (8,8) size 1008x102 (composited, bounds at (0,0) size 1008x102)
      RenderBody 777cbd48 {BODY} at (8,8) size 1008x102 [bgcolor=#C0C0C0]
     normal flow list(1)
      layer 7fed9f98 at (8,8) size 202x102 clip at (9,9) size 200x100
        RenderBlock 777ff3a0 {DIV} at (0,0) size 202x102 [border: (1px solid #000000)] class="box"

- Backtrace #1
Breakpoint 1, WebCore::RenderLayer::updateClipRects (this=0x2fd2acf8, rootLayer=0x77a25160, relevancy=WebCore::IgnoreOverlayScrollbarSize)
    at /home/leo/workspace/playbook/webkit/Source/WebCore/rendering/RenderLayer.cpp:3344
3344        if (m_clipRects) {
(gdb) bt
#0  WebCore::RenderLayer::updateClipRects (this=0x2fd2acf8, rootLayer=0x77a25160, relevancy=WebCore::IgnoreOverlayScrollbarSize)
    at /home/leo/workspace/playbook/webkit/Source/WebCore/rendering/RenderLayer.cpp:3344
#1  0x7d22e7d8 in WebCore::RenderLayer::updateClipRects (this=0x7fed9f98, rootLayer=0x77a25160, relevancy=WebCore::IgnoreOverlayScrollbarSize)
    at /home/leo/workspace/playbook/webkit/Source/WebCore/rendering/RenderLayer.cpp:3353
#2  0x7d22efbc in WebCore::RenderLayer::parentClipRects (this=0x777c35b0, rootLayer=0x77a25160, clipRects=..., temporaryClipRects=false, 
    relevancy=WebCore::IgnoreOverlayScrollbarSize) at /home/leo/workspace/playbook/webkit/Source/WebCore/rendering/RenderLayer.cpp:3436
#3  0x7d22f070 in WebCore::RenderLayer::backgroundClipRect (this=0x777c35b0, rootLayer=0x77a25160, temporaryClipRects=false, 
    relevancy=WebCore::IgnoreOverlayScrollbarSize) at /home/leo/workspace/playbook/webkit/Source/WebCore/rendering/RenderLayer.cpp:3445
#4  0x7d22f294 in WebCore::RenderLayer::calculateRects (this=0x777c35b0, rootLayer=0x77a25160, paintDirtyRect=..., layerBounds=..., backgroundRect=..., 
    foregroundRect=..., outlineRect=..., temporaryClipRects=false, relevancy=WebCore::IgnoreOverlayScrollbarSize)
    at /home/leo/workspace/playbook/webkit/Source/WebCore/rendering/RenderLayer.cpp:3462
#5  0x7d22b110 in WebCore::RenderLayer::paintLayer (this=0x777c35b0, rootLayer=0x77a25160, p=0x77bfe764, paintDirtyRect=..., paintBehavior=0, 
    paintingRoot=0x777cb150, overlapTestRequests=0x0, paintFlags=0) at /home/leo/workspace/playbook/webkit/Source/WebCore/rendering/RenderLayer.cpp:2641
#6  0x7d22b9c0 in WebCore::RenderLayer::paintList (this=0x77a25160, list=0x33ac15d8, rootLayer=0x77a25160, p=0x77bfe764, paintDirtyRect=..., 
    paintBehavior=0, paintingRoot=0x777cb150, overlapTestRequests=0x0, paintFlags=0)
    at /home/leo/workspace/playbook/webkit/Source/WebCore/rendering/RenderLayer.cpp:2761
#7  0x7d240de0 in WebCore::RenderLayerBacking::paintIntoLayer (this=0x33ac3950, rootLayer=0x77a25160, context=0x77bfe764, paintDirtyRect=..., 
    paintBehavior=0, paintingPhase=5, paintingRoot=0x777cb150) at /home/leo/workspace/playbook/webkit/Source/WebCore/rendering/RenderLayerBacking.cpp:1148
#8  0x7d2414e8 in WebCore::RenderLayerBacking::paintContents (this=0x33ac3950, graphicsLayer=0x7f7f9680, context=..., paintingPhase=5, clip=...)
    at /home/leo/workspace/playbook/webkit/Source/WebCore/rendering/RenderLayerBacking.cpp:1238
#9  0x7dacd564 in WebCore::GraphicsLayer::paintGraphicsLayerContents (this=0x7f7f9680, context=..., clip=...)
    at /home/leo/workspace/playbook/webkit/Source/WebCore/platform/graphics/GraphicsLayer.cpp:271
#10 0x7d86ab58 in WebCore::LayerWebKitThread::paintContents (this=0x7ffcfcd0, contentsRect=..., scale=1, isSolidColor=0x77bfede9, color=0x77bfed38)
    at /home/leo/workspace/playbook/webkit/Source/WebCore/platform/graphics/blackberry/LayerWebKitThread.cpp:145
#11 0x7de72a0c in WebCore::LayerTiler::updateTextureContentsIfNeeded (this=0x7f7f9578, scale=1)
    at /home/leo/workspace/playbook/webkit/Source/WebCore/platform/graphics/blackberry/LayerTiler.cpp:287
#12 0x7d86ae7c in WebCore::LayerWebKitThread::updateTextureContentsIfNeeded (this=0x7ffcfcd0)
    at /home/leo/workspace/playbook/webkit/Source/WebCore/platform/graphics/blackberry/LayerWebKitThread.cpp:183
.......

- Back trace #2 (ASSERT hits)
#0  0x7d22e784 in WebCore::RenderLayer::updateClipRects (this=0x2fd2acf8, rootLayer=0x2fd2acf8, relevancy=WebCore::IgnoreOverlayScrollbarSize)
    at /home/leo/workspace/playbook/webkit/Source/WebCore/rendering/RenderLayer.cpp:3345
#1  0x7d22efbc in WebCore::RenderLayer::parentClipRects (this=0x7fed9f98, rootLayer=0x2fd2acf8, clipRects=..., temporaryClipRects=false, 
    relevancy=WebCore::IgnoreOverlayScrollbarSize) at /home/leo/workspace/playbook/webkit/Source/WebCore/rendering/RenderLayer.cpp:3436
#2  0x7d22f070 in WebCore::RenderLayer::backgroundClipRect (this=0x7fed9f98, rootLayer=0x2fd2acf8, temporaryClipRects=false, 
    relevancy=WebCore::IgnoreOverlayScrollbarSize) at /home/leo/workspace/playbook/webkit/Source/WebCore/rendering/RenderLayer.cpp:3445
#3  0x7d22f294 in WebCore::RenderLayer::calculateRects (this=0x7fed9f98, rootLayer=0x2fd2acf8, paintDirtyRect=..., layerBounds=..., backgroundRect=..., 
    foregroundRect=..., outlineRect=..., temporaryClipRects=false, relevancy=WebCore::IgnoreOverlayScrollbarSize)
    at /home/leo/workspace/playbook/webkit/Source/WebCore/rendering/RenderLayer.cpp:3462
#4  0x7d22b110 in WebCore::RenderLayer::paintLayer (this=0x7fed9f98, rootLayer=0x2fd2acf8, p=0x77bfe73c, paintDirtyRect=..., paintBehavior=0, 
    paintingRoot=0x777cbd48, overlapTestRequests=0x0, paintFlags=0) at /home/leo/workspace/playbook/webkit/Source/WebCore/rendering/RenderLayer.cpp:2641
#5  0x7d22b9c0 in WebCore::RenderLayer::paintList (this=0x2fd2acf8, list=0x33ac14a0, rootLayer=0x2fd2acf8, p=0x77bfe73c, paintDirtyRect=..., 
    paintBehavior=0, paintingRoot=0x777cbd48, overlapTestRequests=0x0, paintFlags=0)
    at /home/leo/workspace/playbook/webkit/Source/WebCore/rendering/RenderLayer.cpp:2761
#6  0x7d241064 in WebCore::RenderLayerBacking::paintIntoLayer (this=0x33ac3128, rootLayer=0x2fd2acf8, context=0x77bfe73c, paintDirtyRect=..., 
    paintBehavior=0, paintingPhase=WebCore::GraphicsLayerPaintAll, paintingRoot=0x777cbd48)
    at /home/leo/workspace/playbook/webkit/Source/WebCore/rendering/RenderLayerBacking.cpp:1185
#7  0x7d2414e8 in WebCore::RenderLayerBacking::paintContents (this=0x33ac3128, graphicsLayer=0x77c64ac0, context=..., 
    paintingPhase=WebCore::GraphicsLayerPaintAll, clip=...) at /home/leo/workspace/playbook/webkit/Source/WebCore/rendering/RenderLayerBacking.cpp:1238
#8  0x7dacd564 in WebCore::GraphicsLayer::paintGraphicsLayerContents (this=0x77c64ac0, context=..., clip=...)
    at /home/leo/workspace/playbook/webkit/Source/WebCore/platform/graphics/GraphicsLayer.cpp:271
#9  0x7d86ab58 in WebCore::LayerWebKitThread::paintContents (this=0x34dcca50, contentsRect=..., scale=1, isSolidColor=0x77bfedc1, color=0x77bfed10)
    at /home/leo/workspace/playbook/webkit/Source/WebCore/platform/graphics/blackberry/LayerWebKitThread.cpp:145
#10 0x7de72a0c in WebCore::LayerTiler::updateTextureContentsIfNeeded (this=0x34dcc968, scale=1)
    at /home/leo/workspace/playbook/webkit/Source/WebCore/platform/graphics/blackberry/LayerTiler.cpp:287
#11 0x7d86ae7c in WebCore::LayerWebKitThread::updateTextureContentsIfNeeded (this=0x34dcca50)
    at /home/leo/workspace/playbook/webkit/Source/WebCore/platform/graphics/blackberry/LayerWebKitThread.cpp:183
.......

In the first back trace, layer 0x2fd2acf8(for composited body) cached clip rects with rootLayer 0x77a25160(root layer of render view). But in the second back trace (ASSERT hits), layer 0x2fd2acf8 was trying to update his clip rects with rootLayer 0x2fd2acf8 (itself which is for composited body because it's the container block of the absolute positioned div "container"). It seems when painting 0x2fd2acf8 updating clip rects will go out of the bound of composited layer.

PS: Some code line number in the back traces might be incorrect compared with the Tip of Tree because I'm using a little bit older code base. But I don't think it will affect the analysis.

[Simon Fraser (smfr)]

Ah, I think this is happening because of the  z-index: -10; in the test case, which causes us to make two compositing layers (foreground layer and background layer).

[Simon Fraser (smfr)]

Comment on attachment 118961 [details]
Another potential fix

The given test case no longer asserts on top of tree, so this patch can't be valid.

[Simon Fraser (smfr)]

Created attachment 139950 [details]
Another testcase (reduction of panic.com blog)

[Simon Fraser (smfr)]

Comment on attachment 139950 [details]
Another testcase (reduction of panic.com blog)

File bug 85455 on the issue revealed by this testcase.

[Doug Kelly]

<rdar://58097337>