Closed
Bug 538267
Opened 15 years ago
Closed 15 years ago
"ASSERTION: Creating a circular frame list, this is very bad" with -moz-column, floating first-letter & crash [@ nsFrameList::RemoveFrame(nsIFrame*) ]
Categories
(Core :: Layout, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.9.3a1
Tracking | Status | |
---|---|---|
status1.9.2 | --- | unaffected |
status1.9.1 | --- | unaffected |
People
(Reporter: jruderman, Assigned: tnikkel)
References
Details
(5 keywords, Whiteboard: [sg:critical?])
Crash Data
Attachments
(2 files, 1 obsolete file)
(deleted),
text/html
|
Details | |
(deleted),
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
###!!! ASSERTION: Out of flow frame doesn't have the expected parent: 'outOfFlowFrame->GetParent() == this', file layout/generic/nsBlockFrame.cpp, line 6682
###!!! ASSERTION: Broken frame linkage: 'prevSibling && prevSibling->GetNextSibling() == aFrame', file layout/generic/nsFrameList.cpp, line 132
###!!! ASSERTION: Creating a circular frame list, this is very bad.: 'this != aNextSibling', file nsIFrame.h, line 901
Crash [@ nsFrameList::RemoveFrame]
This testcase is similar to the one in bug 417902.
Comment 1•15 years ago
|
||
Worksforme, using:
Mozilla/5.0 (Windows; U; Windows NT 6.1; nl; rv:1.9.1.6) Gecko/20091201 Firefox/3.5.6
Comment 2•15 years ago
|
||
Crashes for me, using:
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a1pre) Gecko/20100105 Minefield/3.7a1pre
(and I get the same 3 assertions as Jesse when I try a linux debug build)
But, confirmed WFM using 3.5.6:
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.6) Gecko/20091201 Firefox/3.5.6
So, regression since then 3.5.6.
Keywords: regression,
regressionwindow-wanted
OS: Mac OS X → All
Comment 3•15 years ago
|
||
WFM in 3.6, too:
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2pre) Gecko/20100106 Namoroka/3.6pre
Here's a crash report from my mozilla-central nightly:
http://crash-stats.mozilla.com/report/index/62e6f024-74b0-4fa3-a9ab-abc402100106
Keywords: crashreportid
Summary: "ASSERTION: Creating a circular frame list, this is very bad" with -moz-column, floating first-letter → "ASSERTION: Creating a circular frame list, this is very bad" with -moz-column, floating first-letter & crash [@ nsFrameList::RemoveFrame(nsIFrame*) ]
Comment 4•15 years ago
|
||
That first assert comes when outOfFlowFrame is an nsFirstLetterFrame (naturally) and its parent is our prev-in-flow. This is happening with this stack:
#3 0x00007ffff609c775 in nsBlockFrame::CollectFloats (this=0x7fffe17a9878, aFrame=0x7fffe17a9168, aList=..., aFromOverflow=0, aCollectSiblings=1)
at ../../../mozilla/layout/generic/nsBlockFrame.cpp:6681
#4 0x00007ffff609c80e in nsBlockFrame::CollectFloats (this=0x7fffe17a9878, aFrame=0x7fffe17cddd8, aList=..., aFromOverflow=0, aCollectSiblings=1)
at ../../../mozilla/layout/generic/nsBlockFrame.cpp:6693
#5 0x00007ffff608b7be in nsBlockFrame::ReparentFloats (this=0x7fffe17cd640, aFirstFrame=0x7fffe17cddd8, aOldParent=0x7fffe17a9878, aFromOverflow=0,
aReparentSiblings=1) at ../../../mozilla/layout/generic/nsBlockFrame.cpp:1677
where that aOldParent passed to ReparentFloats is the next-in-flow and the |this| in frame 5 is the thing that actually has the placeholder and the first-letter frame on its child lists (at least at the point when that assert actually fires).
Then we call RemoveFrame on the mFloats of the next-in-flow block with the letter frame which gives us that second assert. Then we hit that "circular frame list" assert due to calling SetNextSibling(nsnull) on a null frame pointer (which asserts this != aNextSibling).
Then we crash due to trying to read mNextSibling on said null frame pointer.
So fundamentally, the issue is calling RemoveFrame on a frame list with a frame that is not in that list and also has no previous sibling...
Comment 5•15 years ago
|
||
And the key is that after the RecoverLetterFrames call for the frame removal we end up with this frametree (just the part under the columnset):
ColumnSet(div)(1)@0x7fffddb06fa8 {0,0,0,1140} [state=00001020] [content=0x7fffe0c9ebf0] [overflow=0,-360,2161,1500] [sc=0x7fffddb06bc0]<
Block(div)(1)@0x7fffddb06e60 next=0x7fffe57a9058 next-in-flow=0x7fffe57a9058 [content=0x7fffe0c9ebf0] {0,0,1,1140} [state=20101000] [overflow=0,-360,600,1500] sc=0x7fffddb170b0(i=0,b=0) pst=:-moz-column-content<
Float-list<
Letter(span)(0)@0x7fffddb17868 {0,0,0,0} [state=00000502] [content=0x7fffe59c22e0] [sc=0x7fffddceba90] pst=:first-letter<
Text(0)@0x7fffddb17598[0,1,T] {0,0,0,0} [state=00000402] [content=0x7fffe9fe2c80] sc=0x7fffddb172b8 pst=:-moz-non-element<
"c"
>
>
>
>
Block(div)(1)@0x7fffe57a9058 prev-in-flow=0x7fffddb06e60 [content=0x7fffe0c9ebf0] {961,0,1,1140} [state=20101004] [overflow=0,0,1200,1140] sc=0x7fffddb170b0(i=1,b=0) pst=:-moz-column-content<
line 0x7fffe57a9020: count=1 state=inline,dirty,prevmarginclean,not impacted,not wrapped,before:nobr,after:nobr[0x8141] {0,0,1200,1140} <
Inline(span)(0)@0x7fffddb17600 {660,0,540,1140} [state=00001000] [content=0x7fffe59c22e0] [sc=0x7fffddb17468]<
Placeholder(span)(0)@0x7fffddb17948 {0,0,0,0} [state=00100402] [content=0x7fffe59c22e0] outOfFlowFrame=Letter(span)(0)@0x7fffddb17868
>
>
>
>
In particular, the float is parented to the wrong block.
Comment 6•15 years ago
|
||
I'll bet this is a regression from bug 499841. :(
Timothy, can you take a look?
Blocks: 499841
Assignee | ||
Comment 7•15 years ago
|
||
I will look into it.
Assignee | ||
Comment 8•15 years ago
|
||
We need to pass down the block continuation as a separate parameter in addition to the parent frame as when the first-letter text is inside a span the parent frame is the span and we don't have a reference to the continuation that should be the parent anymore.
Assignee: nobody → tnikkel
Attachment #420499 -
Flags: review?(bzbarsky)
Assignee | ||
Updated•15 years ago
|
Keywords: regressionwindow-wanted
Comment 9•15 years ago
|
||
Hmm. Why the change from using GetGeometricParent? What does GetGeometricParent return in this case?
Note that the return value of GetGeometricParent is what will get the float list in the end, so if it's wrong then _that_ is what we need to address, no? It looks like you do address that, which makes me wonder why you need to then pass around aBlockContinuation to CreateFloatingLetterFrame; it seems like you could just grab it from the frame constructor state.
I think the arguments for WrapFramesInFirstLetterFrame could use some documenting; I'm starting to get lost in them.
Assignee | ||
Comment 10•15 years ago
|
||
(In reply to comment #9)
> Hmm. Why the change from using GetGeometricParent? What does
> GetGeometricParent return in this case?
>
> Note that the return value of GetGeometricParent is what will get the float
> list in the end, so if it's wrong then _that_ is what we need to address, no?
> It looks like you do address that, which makes me wonder why you need to then
> pass around aBlockContinuation to CreateFloatingLetterFrame; it seems like you
> could just grab it from the frame constructor state.
GetGeometricParent will just return whatever float containing block we passed to the constructor of the frame constructor state we create in CreateLetterFrame just before calling CreateFloatingLetterFrame. So you are correct that I don't need to change the GetGeometricParent part, and that I don't need to pass aBlockContinuation to CreateFloatingLetterFrame. It is the float containing block of the frame constructor state that needs to be changed.
I'll make these changes.
> I think the arguments for WrapFramesInFirstLetterFrame could use some
> documenting; I'm starting to get lost in them.
Good point, I will add some documentation.
Assignee | ||
Comment 11•15 years ago
|
||
Made the changes mentioned in comment 10 and also changed the assertion in CreateLetterFrame.
Attachment #420499 -
Attachment is obsolete: true
Attachment #420846 -
Flags: review?(bzbarsky)
Attachment #420499 -
Flags: review?(bzbarsky)
Comment 12•15 years ago
|
||
Comment on attachment 420846 [details] [diff] [review]
patch v2
Excellent. Thanks!
Attachment #420846 -
Flags: review?(bzbarsky) → review+
Reporter | ||
Updated•15 years ago
|
Whiteboard: [sg:critical?]
Assignee | ||
Comment 13•15 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/853fc7683a46
The bug that caused this (bug 499841) only landed on trunk, so we don't need this anywhere else.
Status: NEW → RESOLVED
Closed: 15 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
Updated•14 years ago
|
Assignee | ||
Comment 14•14 years ago
|
||
Added crashtest
http://hg.mozilla.org/mozilla-central/rev/ad14722d17f0
Flags: in-testsuite? → in-testsuite+
Updated•14 years ago
|
Crash Signature: [@ nsFrameList::RemoveFrame(nsIFrame*) ]
You need to log in
before you can comment on or make changes to this bug.
Description
•