Last Comment Bug 413048 - Crash [@ nsFrameList::SortByContentOrder] with -moz-column, float
: Crash [@ nsFrameList::SortByContentOrder] with -moz-column, float
Status: VERIFIED FIXED
: [sg:critical?]
: assertion, crash, fixed1.9.0.2, testcase
Product: Core
Classification: Components
Component: Layout
: Trunk
: x86 Mac OS X
: P3 critical (vote)
: ---
Assigned To: Robert O'Callahan (:roc) (Mozilla Corporation)
: layout
:
:
: 306939 425981 426040
  Show dependency treegraph
 
Reported: 2008-01-18 17:50 PST by Jesse Ruderman
Modified: 2008-09-23 21:31 PDT (History)
12 users (show)
roc: blocking1.9-
roc: wanted1.9.0.x+
asac: wanted1.8.0.x-
roc: in‑testsuite+
See Also:
Crash Signature:
[@ nsFrameList::SortByContentOrder]


Attachments
testcase (crashes Firefox when loaded) (207 bytes, text/html)
2008-01-18 17:50 PST, Jesse Ruderman
no flags Details
stack trace (6.10 KB, text/plain)
2008-01-18 17:51 PST, Jesse Ruderman
no flags Details
fix (6.89 KB, patch)
2008-07-23 03:17 PDT, Robert O'Callahan (:roc) (Mozilla Corporation)
no flags Details | Diff | Splinter Review
fix v2 (6.89 KB, patch)
2008-07-23 11:59 PDT, Robert O'Callahan (:roc) (Mozilla Corporation)
dbaron: review+
dbaron: superreview+
samuel.sidler+old: approval1.9.0.2+
Details | Diff | Splinter Review

Summon comment box

Description Jesse Ruderman 2008-01-18 17:50:31 PST
Created attachment 297903 [details]
testcase (crashes Firefox when loaded)

Loading this testcase in a Mac trunk debug build triggers:

###!!! ASSERTION: unexpected flow: 'mFrames.ContainsFrame(nextInFlow)', file /Users/jruderman/trunk/mozilla/layout/generic/nsInlineFrame.cpp, line 470

###!!! ASSERTION: StealFrame failure: 'NS_SUCCEEDED(rv)', file /Users/jruderman/trunk/mozilla/layout/generic/nsContainerFrame.cpp, line 1092

Crash in nsIFrame::GetNextSibling, called by nsFrameList::SortByContentOrder, dereferencing 0xddddddfd.
Comment 1 Jesse Ruderman 2008-01-18 17:51:32 PST
Created attachment 297904 [details]
stack trace
Comment 2 Robert O'Callahan (:roc) (Mozilla Corporation) 2008-07-22 21:24:28 PDT
This is very nasty. Basically, inline frames have no protection against pulling a placeholder continuation into the same container as its prev-in-flow. Blocks have that protection in nsBlockFrame::HandleOverflowPlaceholdersForPulledFrame ... but I just now realized that that's broken for the case where aFrame is a span; we descend into the span and traverse its children, but later we assume that the frames we're looking at have a block frame as parent...

I wonder if it would be easier to just remove float continuations as we're planning to do.
Comment 3 Robert O'Callahan (:roc) (Mozilla Corporation) 2008-07-22 22:33:58 PDT
I think we should just disable float breaking in columns. It's too fragile to fix up with any reasonable amount of effort. We should reform layout (at least by making placeholders not have continuation frames) and then revisit the issue.
Comment 4 Robert O'Callahan (:roc) (Mozilla Corporation) 2008-07-23 03:17:01 PDT
Created attachment 330909 [details] [review]
fix

This disables float breaking in columns by reflowing them with unrestricted height.

The change to nsBlockReflowState::AddFloat is the scary bit. We need this change because we don't want to mark the placeholder for a below-current-line float as incomplete, that triggers this crash. As far as I can tell, this code is only needed to stop the teardown of next-in-flows, so we only need the GetNextInFlow check --- which, conveniently, will now never be true for floats in columns.

The test 386147-1.html depends on floats breaking across columns so I just disabled it.
Comment 5 Jesse Ruderman 2008-07-23 11:39:59 PDT
>-== 386147-1.html 386147-1-ref.html
>+fails == 386147-1.html 386147-1-ref.html # bug 413048

Please reference a new bug instead of this one.  I like being able to grep reftest.list for bug references and make sure they are all still open bugs.
Comment 6 Robert O'Callahan (:roc) (Mozilla Corporation) 2008-07-23 11:56:22 PDT
Sounds reasonable. I filed bug 447660 for that.
Comment 7 Robert O'Callahan (:roc) (Mozilla Corporation) 2008-07-23 11:59:28 PDT
Created attachment 330973 [details] [review]
fix v2

Updated with that
Comment 8 David Baron [:dbaron] 2008-08-07 16:23:52 PDT
Comment on attachment 330973 [details] [review]
fix v2

r+sr=dbaron
Comment 9 Robert O'Callahan (:roc) (Mozilla Corporation) 2008-08-12 02:33:24 PDT
Pushed 5d09fb3015d1.
Comment 10 Gary Kwong [:nth10sd] 2008-08-15 21:36:00 PDT
Verified fixed with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1a2pre) Gecko/20080816113032 Minefield/3.1a2pre
Comment 11 Robert O'Callahan (:roc) (Mozilla Corporation) 2008-08-17 19:28:08 PDT
Comment on attachment 330973 [details] [review]
fix v2

This should fix a number of bugs on branch.
Comment 12 Samuel Sidler (old account; do not CC) 2008-08-19 16:43:28 PDT
Comment on attachment 330973 [details] [review]
fix v2

Approved for 1.9.0.2. Please land in CVS. a=ss
Comment 13 Robert O'Callahan (:roc) (Mozilla Corporation) 2008-08-25 02:47:35 PDT
Checked into 1.9.0.
Comment 14 Alexander Sack 2008-08-31 17:09:46 PDT
doesnt look like 1.8.0 is affected. if i am mistaken, please flip wanted or blocking flag.

Note You need to log in before you can comment on or make changes to this bug.