399687 - "ASSERTION: aPrevFrame must be the last continuation in its chain!" with -moz-column, overflowing height

Status: RESOLVED FIXED

(Core :: Layout, defect, P5)

Description

[Jesse Ruderman]

Attachment: testcase

###!!! ASSERTION: aPrevFrame must be the last continuation in its chain!: '!aPrevFrame || !aPrevFrame->GetNextContinuation()', file /Users/jruderman/trunk/mozilla/layout/base/nsFrameManager.cpp, line 672

I think this can lead to crashes where Firefox dereferences 0xdddddddd.

Comment 1

[fantasai]

Well then, we'll need to fix the crashes. This right here is expected behavior: the new frame should inserted right after ocontainer where it visually ends, not where it logically ends. Do you have any testcases that actually crash with this assert?

Comment 2

[Jesse Ruderman]

This bug is about an assertion failure, not the visual rendering of the testcase, so the "expected behavior" is not triggering an assertion failure :)

I could try to make another reduced testcase for a crash.  Should I wait until this assertion is fixed and bug 399843 is fixed, though?  It's tricky to make reduced testcases for crashes when there are other crashes around.

Comment 3

[fantasai]

Attachment: patch to update assertion

Comment 4

[fantasai]

What I'm saying is, I don't expect this assertion to be indicating a potential crash. If it is, that means other code has problems.

Comment 5

[Jesse Ruderman]

Ok, I'll try to make a reduced testcase for one of the crashes I saw along with this assertion.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 285134 [details] [diff] [review]
patch to update assertion

Request approval when we get out of tree-red state

Comment 7

[Jesse Ruderman]

(In reply to comment #5)
> Ok, I'll try to make a reduced testcase for one of the crashes I saw along 
> with this assertion.

Filed bug 400223.

Comment 8

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 285134 [details] [diff] [review]
patch to update assertion

a=drivers for after M9 freeze

Comment 9

[Reed Loden [:reed]]

fantasai said she can check-in herself.

Comment 10

[fantasai]

Fix checked in: nsFrameManager.cpp rev 1.257

Comment 11

[Jesse Ruderman]

Crashtest checked in.

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 285134 [details] [diff] [review]
patch to update assertion

layout/base/nsFrameManager.cpp:682: warning: suggest parentheses around && within ||

Was this really the order of operations that you wanted?