Bug 99796 - ASSERTION FAILED: m_next in LayoutState.cpp
Summary: ASSERTION FAILED: m_next in LayoutState.cpp
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: MathML (show other bugs)
Version: 420+
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dave Barton
URL: https://svgwg.org/svg2-draft/single-p...
Keywords:
: 100464 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-10-18 19:04 PDT by Philip Rogers
Modified: 2012-12-13 11:17 PST (History)
8 users (show)

See Also:


Attachments
bug 100465 test case 1 crash log on my Mac, shows this bug 99796 (12.26 KB, text/plain)
2012-10-26 17:09 PDT, Dave Barton
no flags Details
Patch (3.87 KB, patch)
2012-10-26 17:48 PDT, Dave Barton
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philip Rogers 2012-10-18 19:04:26 PDT
We assert on the SVG2 draft page: https://svgwg.org/svg2-draft/single-page.html.

I think it's likely this is related to MathML.

Debug assert:
ASSERTION FAILED: m_next
../../third_party/WebKit/Source/WebCore/rendering/LayoutState.cpp(49) : WebCore::LayoutState::LayoutState(WebCore::LayoutState *, WebCore::RenderBox *, const LayoutSize &, LayoutUnit, bool, WebCore::ColumnInfo *)
1   0x2cffb33 WebCore::LayoutState::LayoutState(WebCore::LayoutState*, WebCore::RenderBox*, WebCore::FractionalLayoutSize const&, WebCore::FractionalLayoutUnit, bool, WebCore::ColumnInfo*)
2   0x2cff984 WebCore::LayoutState::LayoutState(WebCore::LayoutState*, WebCore::RenderBox*, WebCore::FractionalLayoutSize const&, WebCore::FractionalLayoutUnit, bool, WebCore::ColumnInfo*)
3   0x2dd5cb5 WebCore::RenderView::pushLayoutState(WebCore::RenderBox*, WebCore::FractionalLayoutSize const&, WebCore::FractionalLayoutUnit, bool, WebCore::ColumnInfo*)
4   0x2dd5a5f WebCore::LayoutStateMaintainer::push(WebCore::RenderBox*, WebCore::FractionalLayoutSize, WebCore::FractionalLayoutUnit, bool, WebCore::ColumnInfo*)
5   0x2dd592d WebCore::LayoutStateMaintainer::LayoutStateMaintainer(WebCore::RenderView*, WebCore::RenderBox*, WebCore::FractionalLayoutSize, bool, WebCore::FractionalLayoutUnit, bool, WebCore::ColumnInfo*)
6   0x2dc65a4 WebCore::LayoutStateMaintainer::LayoutStateMaintainer(WebCore::RenderView*, WebCore::RenderBox*, WebCore::FractionalLayoutSize, bool, WebCore::FractionalLayoutUnit, bool, WebCore::ColumnInfo*)
7   0x2e29e07 WebCore::RenderFlexibleBox::layoutBlock(bool, WebCore::FractionalLayoutUnit)
8   0x2d09565 WebCore::RenderBlock::layout()
9   0x2f31599 WebCore::RenderObject::layoutIfNeeded()
10  0x3020c1f WebCore::RenderMathMLBlock::computeChildrenPreferredLogicalHeights()
11  0x3026fd3 WebCore::RenderMathMLRow::computePreferredLogicalWidths()
12  0x2dabdd2 WebCore::RenderBox::maxPreferredLogicalWidth() const
13  0x3020b61 WebCore::RenderMathMLBlock::computeChildrenPreferredLogicalHeights()
14  0x3026fd3 WebCore::RenderMathMLRow::computePreferredLogicalWidths()
15  0x2dabd42 WebCore::RenderBox::minPreferredLogicalWidth() const
16  0x2d2baaa WebCore::RenderBlock::computeInlinePreferredLogicalWidths()
17  0x2d2ac8e WebCore::RenderBlock::computePreferredLogicalWidths()
18  0x2f8f4c1 WebCore::RenderTableCell::computePreferredLogicalWidths()
19  0x2c9a636 WebCore::AutoTableLayout::recalcColumn(unsigned int)
20  0x2c9b3fe WebCore::AutoTableLayout::fullRecalc()
21  0x2c9b485 WebCore::AutoTableLayout::computePreferredLogicalWidths(WebCore::FractionalLayoutUnit&, WebCore::FractionalLayoutUnit&)
22  0x2f867ca WebCore::RenderTable::computePreferredLogicalWidths()
23  0x2dabd42 WebCore::RenderBox::minPreferredLogicalWidth() const
24  0x2d2c9b1 WebCore::RenderBlock::computeBlockPreferredLogicalWidths()
25  0x2d2aca1 WebCore::RenderBlock::computePreferredLogicalWidths()
26  0x2dabd42 WebCore::RenderBox::minPreferredLogicalWidth() const
27  0x2d2c9b1 WebCore::RenderBlock::computeBlockPreferredLogicalWidths()
28  0x2d2aca1 WebCore::RenderBlock::computePreferredLogicalWidths()
29  0x2dabd42 WebCore::RenderBox::minPreferredLogicalWidth() const
30  0x2d2c9b1 WebCore::RenderBlock::computeBlockPreferredLogicalWidths()
31  0x2d2aca1 WebCore::RenderBlock::computePreferredLogicalWidths()
Comment 1 Dirk Schulze 2012-10-18 19:11:25 PDT
Niiiiice! Assertion on opening a Web standard specification :D.
Comment 2 Dave Barton 2012-10-25 14:27:52 PDT
I have 2 problems:

1. I can't reproduce the assert failing. That spec page hasn't changed, but I've tried in debug builds of both Safari (base webkit) and Chrome. There's only one <math> element inside a <table> in that page, so that must be causing the problem. It'd be great to have that inside some page that would reliably trigger the assert failure.

2. I guess I don't understand LayoutState. MathML is perhaps unique in that because of operator stretching (think large parentheses or brackets), a preferred logical width often can't be calculated until one's children's "preferred logical height" is known. See RenderMathMLBlock.h/cpp. I now see that RenderMedia::layout has this comment: "When calling layout() on a child node, a parent must either push a LayoutStateMaintainter, or instantiate LayoutStateDisabler." I guess I need to at least do one of those in RenderMathMLBlock::computeChildrenPreferredLogicalHeights before doing child->layoutIfNeeded(). I gather that even LayoutStateDisabler is maybe really to disable only repainting, and further LayoutStates will still be pushed for offset calculations for some reason, which might still trigger this assert? (RenderView.h says: "Note that even when disabled, LayoutState is still used to store layoutDelta." Why?) Sorry I am so confused. Is there a document or set of tests or person you can point me to for learning more about these issues? I had thought the two lines in RenderMathMLBlock::computeChildrenPreferredLogicalHeights:

    // Ensure a full repaint will happen after layout finishes.
    setNeedsLayout(true, MarkOnlyThis);

would deal with all this, and it's worked in existing tests for months up to now, but apparently that is not enough. Thanks for any help or pointers!
Comment 3 Dave Barton 2012-10-26 17:09:43 PDT
Created attachment 171053 [details]
bug 100465 test case 1 crash log on my Mac, shows this bug 99796
Comment 4 Dave Barton 2012-10-26 17:11:53 PDT
Thanks very much to smfr and eseidel for helping me with LayoutState in irc chat.

I now have a patch for this but don't know how to test it. This bug occurs when something asks for a typical MathML object's preferred logical width(s) when *not* doing an actual layout, so there's no LayoutState. How can I make this happen, especially during LayoutTestRunner? I can't reproduce this exact bug (99796), and I don't see the bottom of the stack trace in comment 0 here. A Chrome Canary user is crashing with a stack trace ending (at least the version I've seen) with:

0x020a5e3e	[Google Chrome Framework]	- RenderBox.cpp:665]	WebCore::RenderBox::minPreferredLogicalWidth
0x014afb6f	[Google Chrome Framework]	- WebFrameImpl.cpp:646]	WebKit::WebFrameImpl::contentsPreferredWidth

This is inside Source/WebKit/chromium/src/WebFrameImpl.cpp, which is chromium-specific, right? There's also a big stack trace in bug 100464, which I think is a duplicate of this bug. Finally, on my machine the first test case in bug 100465 apparently causes this bug also, with a stack trace I just attached here.

I see repainting tests in LayoutTests, but I need something different here. Any short explanation, example or hint would be greatly appreciated!
Comment 5 Dave Barton 2012-10-26 17:48:31 PDT
Created attachment 171064 [details]
Patch
Comment 6 Eric Seidel (no email) 2012-10-26 17:56:42 PDT
Comment on attachment 171064 [details]
Patch

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

> Source/WebCore/rendering/mathml/RenderMathMLBlock.cpp:172
> +    if (!hadLayoutState)
> +        renderView->pushLayoutState(this);

I'm curious as to why we don't just always push?
Comment 7 Dave Barton 2012-10-26 18:15:47 PDT
Comment on attachment 171064 [details]
Patch

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

>> Source/WebCore/rendering/mathml/RenderMathMLBlock.cpp:172
>> +        renderView->pushLayoutState(this);
> 
> I'm curious as to why we don't just always push?

RenderView.cpp:858 in void RenderView::pushLayoutState(RenderObject* root):
    ASSERT(m_layoutState == 0);
Comment 8 Eric Seidel (no email) 2012-10-26 18:20:17 PDT
Comment on attachment 171064 [details]
Patch

OK.  This seems like a reasonable mitigation then.  You might wait until tomorrow to mark cq+, just in case others who know LayoutState better than I choose to comment.
Comment 9 WebKit Review Bot 2012-10-27 11:28:14 PDT
Comment on attachment 171064 [details]
Patch

Clearing flags on attachment: 171064

Committed r132737: <http://trac.webkit.org/changeset/132737>
Comment 10 WebKit Review Bot 2012-10-27 11:28:19 PDT
All reviewed patches have been landed.  Closing bug.
Comment 11 Dave Barton 2012-12-13 11:17:41 PST
*** Bug 100464 has been marked as a duplicate of this bug. ***