412243 - Crash [@ nsContinuingTextFrame::GetFirstContinuation] with -moz-column, float

Status: RESOLVED WORKSFORME

(Core :: Layout, defect, P3)

Description

[Jesse Ruderman]

Attachment: testcase (crashes Firefox when loaded)

This fairly simple testcase makes Firefox crash 
[@ nsContinuingTextFrame::GetFirstContinuation].

The testcase is based on the crashtest for bug 391894.

Comment 1

[Mats Palmgren (inactive)]

Attachment: Patch rev. 1

Comment 2

[Mats Palmgren (inactive)]

Comment on attachment 296924 [details] [diff] [review]
Patch rev. 1

Hmm, but why is a nsContinuingTextFrame the first continuation?

Comment 3

[Mats Palmgren (inactive)]

Attachment: Frame dumps, stacks

This looks pretty bad actually...

So, we're removing the DIV element and thus the SPAN.
The SPAN has been split into 3 floats (in separate columns) and each
float has a text frame.

At the top is the content tree before removing the DIV.  Then the frame
tree at the start of nsCSSFrameConstructor::ContentRemoved(), the child
frame to be destroyed is marked yellow (correct).

I added an assertion in nsContinuingTextFrame::SetPrevInFlow() that
asserts if 'aPrevInFlow' is null, that caught the stack for when
the nsContinuingTextFrame became the first continuation.

Then follows the frame tree when nsCSSFrameConstructor::ContentRemoved()
returns, as you can see the error has already occurred here - we still
have frames in the tree for the SPAN (magenta,lime) and for the text.

The error occurs when we reflow the "b " text frame (brown) which is a
nsContinuingTextFrame.
The crash occurs because I used GetFirstContinuation() in the diagnostic
code for bug 406380 (which apparently has never been used before?)
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsTextFrameThebes.cpp&rev=3.160&root=/cvsroot&mark=5196#5169

I'm pretty sure the bug is in DeletingFrameSubtree(), like I was
speculating in bug 411835, I think we must process next-in-flows for
all descendant out-of-flows.  I need to think about how to handle the
placeholder continuations properly... which involves bug 404721 too.
I'll try to come up with a combined patch on bug 411835.

Comment 4

[Martijn Wargers (dead)]

Attachment: testcase2

Comment 5

[Jesse Ruderman]

With the first testcase, I see

###!!! ASSERTION: How can an nsContinuingTextFrame be the first continuation?: 'previous', file /Users/jruderman/trunk/mozilla/layout/generic/nsTextFrameThebes.cpp, line 3274

before the crash.

Martijn's testcase2 doesn't crash for me.

Comment 6

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Both testcases in comment #0 and comment #4 WFM on Mac trunk Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1a2pre) Gecko/20080818150509 Minefield/3.1a2pre with mallocscribble.  No assertions / crashes at all.

Comment 7

[Samuel Sidler (old account; do not CC)]

Jesse, can you confirm that this is WFM? Is it also true using Firefox 3.0 and, if so, do we have a fix range?

Comment 8

[Jesse Ruderman]

WFM on trunk (tested on Tiger).  Yay!

Comment 9

[Daniel Veditz [:dveditz]]

WFM on 1.9.0.5 too.