Bug 239823 – Assertion in RenderTreeBuilder::attachToRenderElementInternal

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Alex Christensen]

Assertion in RenderTreeBuilder::attachToRenderElementInternal

[Alex Christensen]

Created attachment 458463 [details]
Patch

[Alex Christensen]

<rdar://problem/92390285>

[zalan]

Comment on attachment 458463 [details]
Patch

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

> Source/WebCore/rendering/updating/RenderTreeBuilder.cpp:456
> +    if (beforeChild && beforeChild->parent() != &parent)
> +        beforeChild = nullptr;
> +

This indicates an invalid (and unexpected) tree structure. These fuzzer tests usually trigger many tree mutations and eventually find a bug in our code that corrupts the tree structure. We normally try to address the real issue.

[Alex Christensen]

Yes, beforeChild->parent() is null with this stack trace.  Something is messed up.

1   0x2c25b0e90 WebCore::RenderTreeBuilder::attachToRenderElementInternal(WebCore::RenderElement&, std::__1::unique_ptr<WebCore::RenderObject, WebCore::RenderObjectDeleter>, WebCore::RenderObject*, WebCore::RenderObject::IsInternalMove)
2   0x2c25ae3d4 WebCore::RenderTreeBuilder::attachToRenderElement(WebCore::RenderElement&, std::__1::unique_ptr<WebCore::RenderObject, WebCore::RenderObjectDeleter>, WebCore::RenderObject*)
3   0x2c25d29e8 WebCore::RenderTreeBuilder::Inline::attachIgnoringContinuation(WebCore::RenderInline&, std::__1::unique_ptr<WebCore::RenderObject, WebCore::RenderObjectDeleter>, WebCore::RenderObject*)
4   0x2c25d1e10 WebCore::RenderTreeBuilder::Inline::attach(WebCore::RenderInline&, std::__1::unique_ptr<WebCore::RenderObject, WebCore::RenderObjectDeleter>, WebCore::RenderObject*)
5   0x2c25acb04 WebCore::RenderTreeBuilder::attachInternal(WebCore::RenderElement&, std::__1::unique_ptr<WebCore::RenderObject, WebCore::RenderObjectDeleter>, WebCore::RenderObject*)
6   0x2c25ab330 WebCore::RenderTreeBuilder::attach(WebCore::RenderElement&, std::__1::unique_ptr<WebCore::RenderObject, WebCore::RenderObjectDeleter>, WebCore::RenderObject*)
7   0x2c260001c WebCore::RenderTreeUpdater::createRenderer(WebCore::Element&, WebCore::RenderStyle&&)
8   0x2c25fd6d0 WebCore::RenderTreeUpdater::updateElementRenderer(WebCore::Element&, WebCore::Style::ElementUpdate const&)
9   0x2c25fc938 WebCore::RenderTreeUpdater::updateRenderTree(WebCore::ContainerNode&)
10  0x2c25fbc48 WebCore::RenderTreeUpdater::commit(std::__1::unique_ptr<WebCore::Style::Update const, std::__1::default_delete<WebCore::Style::Update const> >)
11  0x2bdccf278 WebCore::Document::updateRenderTree(std::__1::unique_ptr<WebCore::Style::Update const, std::__1::default_delete<WebCore::Style::Update const> >)
12  0x2bdccfef0 WebCore::Document::resolveStyle(WebCore::Document::ResolveStyleType)
13  0x2bdcd1ad0 WebCore::Document::updateStyleIfNeeded()
14  0x2bfebb9a4 WebCore::FrameView::updateLayoutAndStyleIfNeededRecursive()
15  0x2c0071a18 WebCore::Page::layoutIfNeeded()
16  0x2c00736b4 WebCore::Page::updateRendering()
17  0x12792bf90 WebKit::WebPage::updateRendering()
18  0x125857bb4 WebKit::TiledCoreAnimationDrawingArea::updateRendering(WebKit::TiledCoreAnimationDrawingArea::UpdateRenderingType)
19  0x1258578e4 WebKit::TiledCoreAnimationDrawingArea::forceRepaint()
20  0x127928828 WebKit::WebPage::forceRepaintWithoutCallback()
21  0x126d56d90 WKBundlePageForceRepaint

[zalan]

it looks like when a RenderInline in continuation has pseudo renderer, we don't manage to remove the pseudo renderer as we clean up the continuation. 

This is an inline content in continuation (we are in the middle of transitioning the tree).

B---YGLS- -+  RenderView at (0,0) size 800x600 renderer->(0x2a1002620) layout->[normal child]
B-----LS- -+*   HTML RenderBlock at (0,0) size 800x600 renderer->(0x2a1003720) node->(0x2a1000fb0) layout->[self][normal child]
I----G--- -+      <pseudo> RenderInline renderer->(0x2a1003e20) node->(0x2a1003d80) layout->[self][normal child]
I---YG--- -+        RenderQuote renderer->(0x2a10040b0) layout->[self][normal child]
I---YG--- -+          RenderText renderer->(0x2a10041b0) layout->[self]
I-------- -+      SPAN RenderInline renderer->(0x2a1003f10) node->(0x2a1003a20) continuation->(0x2a10044e0) layout->[self]
I-------- -+      SPAN RenderInline renderer->(0x2a10044e0) node->(0x2a1003a20) layout->[self]
I----G--- -+      <pseudo> RenderInline renderer->(0x2a1004670) node->(0x2a10045d0) layout->[self]
I---YG--- --        RenderQuote renderer->(0x2a1004760)
I---YG--- --          RenderText renderer->(0x2a1004860)

and after destroying RenderInline(0x2a1003f10) -and cleaning up the continuation, we end up with leftover pseudo renderers.

B---YGLS- -+  RenderView at (0,0) size 800x600 renderer->(0x2a1002620) layout->[normal child]
B-----LS- -+    HTML RenderBlock at (0,0) size 800x600 renderer->(0x2a1003720) node->(0x2a1000fb0) layout->[self][normal child]
I----G--- -+      <pseudo> RenderInline renderer->(0x2a1003e20) node->(0x2a1003d80) layout->[self][normal child]
I---YG--- -+        RenderQuote renderer->(0x2a10040b0) layout->[self][normal child]
I---YG--- -+          RenderText renderer->(0x2a10041b0) layout->[self]
I----G--- -+      <pseudo> RenderInline renderer->(0x2a1004670) node->(0x2a10045d0) layout->[self]
I---YG--- --        RenderQuote renderer->(0x2a1004760)
I---YG--- --          RenderText renderer->(0x2a1004860)
I-------- -+*     Q RenderInline renderer->(0x2a1005aa0) node->(0x2a1003900) layout->[self]
and they confuse the tree builder which picks the wrong beforeChild as the insertion point when adding a child renderer under Q as a subsequent tree mutation step.


The fix is to clean up these pseudo renderers as we destroy their "associated" renderers (in continuation).

[zalan]

I managed to make this test flaky and the difference between the crashing and the non-crashing tree building is as follows:

both start with a fully built tree with continuation:

RenderView at (0,0) size 573x600 renderer->(0x114002620)
  HTML RenderBlock at (0,0) size 573x600 renderer->(0x114003720) node->(0x114000fb0)
    RenderBlock at (0,0) size 573x18 renderer->(0x114004260)
      <pseudo> RenderInline renderer->(0x114003e20) node->(0x114003d80)
        RenderQuote renderer->(0x1140040b0)
          RenderText renderer->(0x1140041b0)
      SPAN RenderInline renderer->(0x114003f10) node->(0x114003a20) continuation->(0x114001780)
    RenderBlock at (0,18) size 573x0 renderer->(0x114001780) continuation->(0x1140044e0)
      DIV RenderBlock at (0,0) size 573x0 renderer->(0x114001640) node->(0x114003bd0)
    RenderBlock at (0,18) size 573x18 renderer->(0x1140043a0)
      SPAN RenderInline renderer->(0x1140044e0) node->(0x114003a20)
      <pseudo> RenderInline renderer->(0x114004670) node->(0x1140045d0)
        RenderQuote renderer->(0x114004760)
          RenderText renderer->(0x114004860)

this essentially maps to <q style="display: contents"><span><div></div</span></q>

at this point we turn <q> to "display: block". It triggers a tree re-building.

The non-crashing case first initiates a full tear-down on the tree (step #1) ->

(this is the result of the tear-down process)
RenderView at (0,0) size 573x600 renderer->(0x114002620) layout->[normal child]
  HTML RenderBlock at (0,0) size 573x600 renderer->(0x114003720) node->(0x114000fb0) layout->[self][normal child]

and start building it again by adding <q> (step #2):
RenderView at (0,0) size 573x600 renderer->(0x114002620) layout->[normal child]
  HTML RenderBlock at (0,0) size 573x600 renderer->(0x114003720) node->(0x114000fb0) layout->[self][normal child]
    Q RenderInline renderer->(0x114003e20) node->(0x114003900) layout->[self]

and complete the tree by inserting the pseudo renderers etc.


However in the crashing case, before we start tearing down the tree, we first insert the <q> instead (this is step #2 in the working case)

RenderView at (0,0) size 573x600 renderer->(0x166002620) layout->[normal child]
  HTML RenderBlock at (0,0) size 573x600 renderer->(0x166003720) node->(0x166000fb0) layout->[normal child]
    RenderBlock at (0,0) size 573x18 renderer->(0x166004260)
      <pseudo> RenderInline renderer->(0x166003e20) node->(0x166003d80)
        RenderQuote renderer->(0x1660040b0)
          RenderText renderer->(0x1660041b0)
      SPAN RenderInline renderer->(0x166003f10) node->(0x166003a20) continuation->(0x166001780)
    RenderBlock at (0,18) size 573x0 renderer->(0x166001780) continuation->(0x1660044e0)
      DIV RenderBlock at (0,0) size 573x0 renderer->(0x166001640) node->(0x166003bd0)
    RenderBlock at (0,18) size 573x18 renderer->(0x1660043a0) layout->[normal child]
      SPAN RenderInline renderer->(0x1660044e0) node->(0x166003a20)
      <pseudo> RenderInline renderer->(0x166004670) node->(0x1660045d0)
        RenderQuote renderer->(0x166004760)
          RenderText renderer->(0x166004860)
      Q RenderInline renderer->(0x166005aa0) node->(0x166003900) layout->[self]

and start the tear-down process after (step #1 in the working case)

(this is the result of the full tear-down process)
RenderView at (0,0) size 573x600 renderer->(0x166002620) layout->[normal child]
  HTML RenderBlock at (0,0) size 573x600 renderer->(0x166003720) node->(0x166000fb0) layout->[self][normal child]
    <pseudo> RenderInline renderer->(0x166003e20) node->(0x166003d80) layout->[self][normal child]
      RenderQuote renderer->(0x1660040b0) layout->[self][normal child]
        RenderText renderer->(0x1660041b0) layout->[self]
    <pseudo> RenderInline renderer->(0x166004670) node->(0x1660045d0) layout->[self]
      RenderQuote renderer->(0x166004760)
        RenderText renderer->(0x166004860)
    Q RenderInline renderer->(0x166005aa0) node->(0x166003900) layout->[self]

At this point the pseudo renderers are at the wrong positions (they are supposed to be under Q) and additional insertions (as we start building up the tree) may assert.


tl;dr;
working case: we completely tear down the tree (with continuation) before turning <q> from "display: contents" to "display: block"
crashing case: we create and insert the renderer for <q> before tearing down the tree and the tear-down process produces an invalid tree.

[Antti Koivisto]

Created attachment 459891 [details]
Patch

[Antti Koivisto]

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

[EWS]

Found 3 new test failures: webgl/2.0.0/conformance/textures/image_bitmap_from_image_bitmap/tex-2d-rgba-rgba-unsigned_short_5_5_5_1.html, webgl/2.0.0/conformance2/textures/image_bitmap_from_image_bitmap/tex-2d-rg8-rg-unsigned_byte.html, webgl/2.0.0/conformance2/textures/image_bitmap_from_image_bitmap/tex-2d-rg8ui-rg_integer-unsigned_byte.html

[EWS]

Committed r295083 (251178@main): <https://commits.webkit.org/251178@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 459891 [details].

[Rob Buis]

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