Crash [@ nsSubDocumentFrame::Reflow] with testcase, using first-letter, first-line, inline-block and iframes

RESOLVED WORKSFORME

Status

()

defect
P3
critical
RESOLVED WORKSFORME
13 years ago
6 years ago

People

(Reporter: martijn.martijn, Assigned: mats)

Tracking

({crash, regression, testcase})

Trunk
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9 -
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [dbaron-1.9:RsCo], crash signature)

Attachments

(4 attachments)

Reporter

Description

13 years ago
Posted file testcase
See testcase, which crashes my current trunk Mozilla builds within 2 seconds.
It doesn't crash in a 2007-01-26 build, but it does crash with 2007-01-28 build.
This is when the fix for bug 9458 went in. I guess that makes sense, since the testcase also makes use of inline-block.
The testcase isn't completely minimised, I can try and minimise it further if wanted.

Talkback ID: TB29084255Y
nsSubDocumentFrame::Reflow  [mozilla\layout\generic\nsframeframe.cpp, line 426]
nsLineLayout::ReflowFrame  [mozilla\layout\generic\nslinelayout.cpp, line 890]
So I see all sorts of asserts, then:

###!!! ASSERTION: What's going on?: 'mInnerView', file ../../../mozilla/layout/generic/nsFrameFrame.cpp, line 716

Then in Reflow() we try to use mInnerView and crash...

If this _could_ be minimized, that would be nice.  Then we could try to figure out why the docshell has a presshell when it shouldn't.
Flags: blocking1.9?
Assignee

Comment 2

13 years ago
Posted file frame dump + stack
I get the following assertion, which is the root cause of the crash:
###!!! ASSERTION: inserting after sibling frame with different parent

Here's the stack and frame dump when that occurs.
In ContentInserted() we have aContainer=<fieldset> and aChild=<legend>.
First we calculate prevSibling=0x1110f28 (blue) at line 8924:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/base/nsCSSFrameConstructor.cpp&rev=1.1308&root=/cvsroot&mark=8924-8926#8917
and set 'parentFrame' to its parent, a first-line frame, 0x1089a00 (green).

The reason we get this rather odd result is that <legend>'s are not
allowed to be siblings to anything except other legend frames:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/base/nsCSSFrameConstructor.cpp&rev=1.1308&root=/cvsroot&mark=8066-8074#8031

This is correct AFAICT. Next we find that we 'haveFirstLetterStyle' and
remove those frames and recalculate 'prevSibling':
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/base/nsCSSFrameConstructor.cpp&rev=1.1308&root=/cvsroot&mark=9049-9053#9041
Since 'parentFrame' is now the first-line frame IsValidSibling() doesn't
object to inserting it at 'aIndexInContainer' and we set 'prevSibling'
to be the frame at that point, a text frame 0x11e7970 (yellow).

(Note that we don't do "parentFrame = prevSibling->GetParent()" like we did
 on line 8946 when we first calculated 'prevSibling'. Why?)

Then we construct the new frame as a child of 'parentFrame'
(the first-line frame) and insert it using 'prevSibling' which has a
different parent frame.
Oops.


I guess we could add "parentFrame = prevSibling->GetParent()" which would
make us create the new frame with the correct parent but then the special
nsFieldSetFrame logic that picks up legend frames would be bypassed...
(actually, I don't think we have a choice, we need to do this)

Another solution might be to remove the fieldset stuff in IsValidSibling()
and instead test for the special fieldset/legend situation just before
InsertFrames()... (we probably need to do both)
Assignee

Comment 3

13 years ago
Posted patch wipSplinter Review
Fixes the crash, but makes nsFieldSetFrame miss the legend frame...
Assignee

Updated

13 years ago
OS: Windows XP → All
Hardware: PC → All
> Note that we don't do "parentFrame = prevSibling->GetParent()"

Uh...  Didn't we fix that once?  I _swear_ I recall writing that.

In any case, we should absolutely do it.

As for IsValidSibling(), I'd really like to get rid of that method, so whatever we can do to do that...
Flags: blocking1.9? → blocking1.9+

Comment 5

12 years ago
Mats did this fall off your radar?
Assignee

Comment 6

12 years ago
It did.  Putting it on my list, but feel free to take it if you have a patch.
Assignee: nobody → mats.palmgren
The underlying problem is still there.  I have no idea why this particular testcase stopped hitting that problem, but it really should't be hard to custom-write one that hits it.
Reporter

Comment 9

12 years ago
Ok, a windows resize after the testcase has done its thing still crashes in current trunk builds.
Reporter

Comment 10

12 years ago
The testcase doesn't crash at all for me anymore with current trunk build.
I don't get the assertion in a debug build that Mats saw in comment 2.
I get this assertion instead:
###!!! ASSERTION: NS_BLOCK_HAS_FIRST_LETTER_STYLE state out of sync: 'haveFirstL
etterStyle == ((mState & NS_BLOCK_HAS_FIRST_LETTER_STYLE) != 0)', file c:/mozill
a/mozilla/layout/generic/nsBlockFrame.cpp, line 6143
However, that might just be a normal assertion.
That's not a "normal" assertion; in this case it's catching that we don't support first-letter on <legend>.  See also bug 369758.
Whiteboard: [dbaron-1.9:RsCo]
Reporter

Comment 12

12 years ago
Posted file testcase2
Ok, this is a testcase that actually crashes again in curren trunk build.
Flags: wanted1.9.0.x+
Flags: blocking1.9-
Flags: tracking1.9+

Comment 13

11 years ago
nsSubDocumentFrame::Reflow is the #14 topcrash for Firefox 3 Beta 5.  Is the topcrash this bug?

Comment 14

11 years ago
I filed bug 426987 with a testcase reduced from a URL mentioned in a Breakpad report.

Comment 15

11 years ago
WFM.  No crashes, and the only assertion I see is the one in bug 399262.

Martijn, do your testcases still crash for you now that the topcrashes have been fixed?
Reporter

Comment 16

11 years ago
Indeed, this is worksforme in current trunk build.
I don't know if the wip patch is still useful, probably not.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → WORKSFORME
Please check in the tests as regression tests!
Flags: in-testsuite+
Flags: in-testsuite+ → in-testsuite?
Flags: wanted1.9.0.x+
Added testcase2 as a crashtest.  Can't quite make sense of how to do that with testcase1, so leaving the "in-testsuite?" flag.
Crash Signature: [@ nsSubDocumentFrame::Reflow]
You need to log in before you can comment on or make changes to this bug.