[mac][wk2] REGRESSION (Tiled Drawing): fast/table/crash-section-logical-height-changed-needsCellRecalc.html asserts

• Component: Tables
• Status: RESOLVED INVALID

[Tim Horton]

While painting, we see:

ASSERTION FAILED: static_cast<unsigned>(m_start + length) <= string.length()
virtual void WebCore::InlineTextBox::paint(WebCore::PaintInfo &, const WebCore::LayoutPoint &, WebCore::LayoutUnit, WebCore::LayoutUnit)
1   0x109d423e0 WTFCrash
2   0x10b5cc938 WebCore::InlineTextBox::paint(WebCore::PaintInfo&, WebCore::LayoutPoint const&, WebCore::LayoutUnit, WebCore::LayoutUnit)
3   0x10b5bcef1 WebCore::InlineFlowBox::paint(WebCore::PaintInfo&, WebCore::LayoutPoint const&, WebCore::LayoutUnit, WebCore::LayoutUnit)
4   0x10c188e64 WebCore::RootInlineBox::paint(WebCore::PaintInfo&, WebCore::LayoutPoint const&, WebCore::LayoutUnit, WebCore::LayoutUnit)

It's 100% reproducible with a recent debug build, the patch in bug 121859, and WebKitTestRunner.

[Myles C. Maxfield]

Note that this crash does not occur if you create the nodes yourself in HTML that the JavaScript is creating, instead.

Here is what is happening:

1. RenderQuote computes its InlineTextBox layout, which includes offsets into RenderQuote's string (m_start and m_len). In this case, the string has a length of 1.
2. RenderBlock wants to update a separate first-letter renderer due to the :first-letter pseudo class
3. This calls RenderQuote::willBeRemovedFromTree -> RenderQuote::updateDepth
4. This calls setText(originalText()) which happens to reset the text to an empty string
5. Later, the InlineTextBox's m_start and m_len variables are unchanged, meaning that it is still is under the assumption that the renderer's text has a length of 1, even though the string has been swapped out to an empty string

Here is a reduction of the HTML page:

<style>
.c3 { position: fixed; }
.c12:first-letter { visibility: inherit; }
.c12 { -webkit-appearance: button; }
</style>
<script>
function boom() {
    var quote = document.createElement('q');
    document.documentElement.appendChild(quote);

    base2 = document.createElement('base');
    base2.setAttribute('class', 'c3');
    quote.appendChild(base2);

    var quote3 = document.createElement('q');
    quote3.setAttribute('class', 'c12');
    base2.appendChild(quote3);
}
window.onload = boom;
</script>


Here is the stack trace of when the string gets swapped out with an empty string:

#0	0x0000000114901737 in WebCore::RenderText::setTextInternal(WTF::String const&) at /WK/Source/WebCore/rendering/RenderText.cpp:1345
#1	0x0000000114901b57 in WebCore::RenderText::setText(WTF::String const&, bool) at /WK/Source/WebCore/rendering/RenderText.cpp:1406
#2	0x0000000114834a52 in WebCore::RenderQuote::updateDepth() at /WK/Source/WebCore/rendering/RenderQuote.cpp:462
#3	0x0000000114833ca9 in WebCore::RenderQuote::detachQuote() at /WK/Source/WebCore/rendering/RenderQuote.cpp:425
#4	0x0000000114833d08 in WebCore::RenderQuote::willBeRemovedFromTree() at /WK/Source/WebCore/rendering/RenderQuote.cpp:66
#5	0x00000001138eebcd in WebCore::RenderElement::removeChildInternal(WebCore::RenderObject*, WebCore::RenderElement::NotifyChildrenType) at /WK/Source/WebCore/rendering/RenderElement.cpp:601
#6	0x00000001138ee9a2 in WebCore::RenderElement::removeChild(WebCore::RenderObject*) at /WK/Source/WebCore/rendering/RenderElement.cpp:495
#7	0x000000011468ee88 in WebCore::RenderBlock::createFirstLetterRenderer(WebCore::RenderObject*, WebCore::RenderText*) at /WK/Source/WebCore/rendering/RenderBlock.cpp:5874
#8	0x000000011468f49a in WebCore::RenderBlock::updateFirstLetter() at /WK/Source/WebCore/rendering/RenderBlock.cpp:5959
#9	0x000000011468c588 in WebCore::RenderBlock::computePreferredLogicalWidths() at /WK/Source/WebCore/rendering/RenderBlock.cpp:4955
#10	0x00000001146dbca9 in WebCore::RenderBox::minPreferredLogicalWidth() const at /WK/Source/WebCore/rendering/RenderBox.cpp:918
#11	0x000000011468a681 in WebCore::RenderBlock::computeInlinePreferredLogicalWidths(WebCore::LayoutUnit&, WebCore::LayoutUnit&) at /WK/Source/WebCore/rendering/RenderBlock.cpp:5253
#12	0x0000000114689dc0 in WebCore::RenderBlock::computeIntrinsicLogicalWidths(WebCore::LayoutUnit&, WebCore::LayoutUnit&) const at /WK/Source/WebCore/rendering/RenderBlock.cpp:4926
#13	0x000000011468c718 in WebCore::RenderBlock::computePreferredLogicalWidths() at /WK/Source/WebCore/rendering/RenderBlock.cpp:4965
#14	0x00000001146dbd29 in WebCore::RenderBox::maxPreferredLogicalWidth() const at /WK/Source/WebCore/rendering/RenderBox.cpp:930
#15	0x00000001146ec445 in WebCore::RenderBox::computePositionedLogicalWidthUsing(WebCore::Length, WebCore::RenderBoxModelObject const*, WebCore::TextDirection, WebCore::LayoutUnit, WebCore::LayoutUnit, WebCore::Length, WebCore::Length, WebCore::Length, WebCore::Length, WebCore::RenderBox::LogicalExtentComputedValues&) const at /WK/Source/WebCore/rendering/RenderBox.cpp:3390
#16	0x00000001146e40b3 in WebCore::RenderBox::computePositionedLogicalWidth(WebCore::RenderBox::LogicalExtentComputedValues&, WebCore::RenderRegion*) const at /WK/Source/WebCore/rendering/RenderBox.cpp:3176
#17	0x00000001146e30e6 in WebCore::RenderBox::computeLogicalWidthInRegion(WebCore::RenderBox::LogicalExtentComputedValues&, WebCore::RenderRegion*) const at /WK/Source/WebCore/rendering/RenderBox.cpp:2160
#18	0x00000001146e2f47 in WebCore::RenderBox::updateLogicalWidth() at /WK/Source/WebCore/rendering/RenderBox.cpp:2142
#19	0x0000000114673faf in WebCore::RenderBlock::updateLogicalWidthAndColumnWidth() at /WK/Source/WebCore/rendering/RenderBlock.cpp:1552
#20	0x0000000114e4ecd7 in WebCore::RenderBlockFlow::layoutBlock(bool, WebCore::LayoutUnit) at /WK/Source/WebCore/rendering/RenderBlockFlow.cpp:241
#21	0x000000011467371d in WebCore::RenderBlock::layout() at /WK/Source/WebCore/rendering/RenderBlock.cpp:1385

[Myles C. Maxfield]

RenderQuote's m_depth gets set to -1 (because of the NO_CLOSE_QUOTE: depth--; case in RenderQuote::updateDepth())

[Simon Fraser (smfr)]

But why does this only happen with tiled drawing?

[Darin Adler]

From a distance this sounds like a classic “paint without doing a layout first” kind of issue.

I guess what we need here is a “how this is supposed to work” tutorial. I am not an expert, but here’s how I think it works. RenderQuote::updateDepth calls setText, and that is supposed to trigger the need for a layout. RenderText::setText does this by calling setNeedsLayoutAndPrefWidthsRecalc. Once that is called, a layout is needed before we can call paint. So either there is a paint code path that is not doing a layout first, or the layout doesn’t properly update the inline text box.

I’m also mixed up about some of the details. If the RenderQuote was removed from the tree, then why does it have inline text boxes any more? Was it put back into the tree?

[Alexey Proskuryakov]

This doesn't happen on regression test bots now. Did we have any steps to reproduce?

[Alexey Proskuryakov]

I'm going to unmark the test, but keep the bug open, as it had some analysis that may mean that there is a known root cause.