Closed Bug 523468 Opened 10 years ago Closed 10 years ago

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

Categories

(Core :: Layout, defect)

x86
All
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: jruderman, Assigned: roc)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, regression, testcase)

Attachments

(3 files, 1 obsolete file)

Attached file 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
Could this have been caused by bug 504524?
Looks like it.
Blocks: 504524
Attached file simplified testcase
Currently renders as

c
b
Assignee: nobody → roc
Attached patch fix (obsolete) — Splinter Review
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.
Attachment #407939 - Flags: review?(dbaron)
Attachment #407939 - Flags: review?(dbaron) → review+
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
It means we're in a MathML container.
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)
Attached patch fix v2Splinter Review
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.
Attachment #407939 - Attachment is obsolete: true
Attachment #408755 - Flags: review?(dbaron)
Whiteboard: [needs review]
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
Interesting. I guess that should be "aLine = overflowLines->begin()".
yup, that fixes the build issue.  With that, "fix v2" does indeed fix bug 525582.
Comment on attachment 408755 [details] [diff] [review]
fix v2

r=dbaron (with the fix described)
Attachment #408755 - Flags: review?(dbaron) → review+
Whiteboard: [needs review] → [needs landing]
OS: Mac OS X → All
http://hg.mozilla.org/mozilla-central/rev/13141391870a
Status: NEW → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Keywords: regression
Resolution: --- → FIXED
Whiteboard: [needs landing]
You need to log in before you can comment on or make changes to this bug.