523468 - "ASSERTION: aFrame not the result of GetPrimaryFrameFor()?"

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Jesse Ruderman]

Attachment: testcase

###!!! ASSERTION: aFrame not the result of GetPrimaryFrameFor()?: 'aFrame == aFrame->GetFirstContinuation()', file layout/base/nsCSSFrameConstructor.cpp, line 8920

###!!! ASSERTION: SetMayHaveFrame failed?: 'mContent->MayHaveFrame()', file layout/generic/nsFrame.cpp, line 343

Comment 1

[Jesse Ruderman]

Could this have been caused by bug 504524?

Comment 2

[Timothy Nikkel (:tnikkel)]

Looks like it.

Comment 3

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

Attachment: simplified testcase

Currently renders as

c
b

Comment 4

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

Attachment: fix

We weren't dirtying the next line when an inline frame pulls or pushes frames to its next-in-flow.

Code to dirty the next line already existed in various places, and was outright wrong in some of them, so I'm adding a state bit to nsLineLayout to indicate that the next line should be dirtied, and having nsBlockFrame do it right in DoReflowInlineFrames. This also reduces overhead in case we need to dirty the next line for multiple reasons.

Comment 5

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

The patch also fixes bug 523460.

Comment 6

[David Baron :dbaron:]

Comment on attachment 407939 [details] [diff] [review]
fix

> nsInlineFrame::PushFrames(nsPresContext* aPresContext,

>+  if (aState.mLineLayout) {
>+    aState.mLineLayout->SetDirtyNextLine();
>+  }

What does it mean for aState.mLineLayout to be null?


r=dbaron

Comment 7

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

It means we're in a MathML container.

Comment 8

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

Checked in:
http://hg.mozilla.org/mozilla-central/rev/9f5177a978ac

Caused crashes, backed out:
http://hg.mozilla.org/mozilla-central/rev/2e01e97f9ded

Valgrind seems to indicate the crash is triggered by layout/generic/crashtests/391894-1.html:

==49900== Invalid read of size 4
==49900==    at 0x8E16AFD: nsLineBox::IsInline() const (nsLineBox.h:247)
==49900==    by 0x8E119F8: nsBlockFrame::DoReflowInlineFrames(nsBlockReflowState&, n
sLineLayout&, nsLineList_iterator, nsFlowAreaRect&, int&, nsFloatManager::SavedState
*, int*, LineReflowStatus*, int) (nsBlockFrame.cpp:3676)

==49900== Invalid read of size 4
==49900==    at 0x8DFCF43: nsLineBox::MarkDirty() (nsLineBox.h:252)
==49900==    by 0x8E11A3E: nsBlockFrame::DoReflowInlineFrames(nsBlockReflowState&, n
sLineLayout&, nsLineList_iterator, nsFlowAreaRect&, int&, nsFloatManager::SavedState
*, int*, LineReflowStatus*, int) (nsBlockFrame.cpp:3677)

Comment 9

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

Attachment: fix v2

This fixes the crash. In some cases aLine gets pushed to the overflow lines, it is then incorrect to pass PR_FALSE for aInOverflowLines when creating the iterator.

The changes are all in the first hunk, just passing the right value for the aInOverflowLines parameter when creating the iterator. If aLine was pushed, it must be the first line in the overflow lines now. We also need to update aLine so it's associated with the right line list.

Comment 10

[Daniel Holbert [:dholbert]]

I just applied "fix v2" to see if it fixes bug 525582, but it breaks my build, with this error:

> /mozilla/layout/generic/nsBlockFrame.cpp:3684: error: no match for ‘operator=’ in ‘aLine = overflowLines->nsLineList::front()’
> /mozilla/layout/generic/nsLineBox.h:1446: note: candidates are: nsLineList_iterator& nsLineList_iterator::operator=(const nsLineList_iterator&)
> /mozilla/layout/generic/nsLineBox.h:1453: note:                 nsLineList_iterator& nsLineList_iterator::operator=(const nsLineList_reverse_iterator&)
> make[1]: *** [nsBlockFrame.o] Error 1

Comment 11

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

Interesting. I guess that should be "aLine = overflowLines->begin()".

Comment 12

[Daniel Holbert [:dholbert]]

yup, that fixes the build issue.  With that, "fix v2" does indeed fix bug 525582.

Comment 13

[David Baron :dbaron:]

Comment on attachment 408755 [details] [diff] [review]
fix v2

r=dbaron (with the fix described)

Comment 14

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

http://hg.mozilla.org/mozilla-central/rev/13141391870a