Closed Bug 404721 Opened 17 years ago Closed 16 years ago

"ASSERTION: Null out-of-flow for placeholder?" and crash with -moz-column, float, {ib}

Categories

(Core :: Layout, defect, P3)

defect

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: jruderman, Assigned: MatsPalmgren_bugz)

References

Details

(Keywords: assertion, crash, testcase)

Attachments

(4 files, 2 obsolete files)

Loading the testcase triggers three assertions and a crash.

###!!! ASSERTION: Null out-of-flow for placeholder?: 'outOfFlow', file /Users/jruderman/trunk/mozilla/layout/base/../generic/nsPlaceholderFrame.h, line 175

###!!! ASSERTION: null ptr: 'nsnull != aFrame', file /Users/jruderman/trunk/mozilla/layout/generic/nsFrameList.cpp, line 121

###!!! ASSERTION: Must have the out of flow in some child list: 'found', file /Users/jruderman/trunk/mozilla/layout/generic/nsBlockFrame.cpp, line 4277

Null deref crash at one of the following:
* [@ ReparentFrame] 
       calling [@ nsIFrame::GetParent]
* [@ nsLayoutUtils::GetFloatFromPlaceholder] 
       calling [@ nsIFrame::GetStyleDisplay]
Flags: blocking1.9?
Attached patch wip (obsolete) — Splinter Review
This fixes the crash, but...
Attached file Frame dumps
We're removing the green Inline(span)@0x1ad6920 frame, which has a child
placeholder frame with continuations, however the span itself and its
parent (yellow Block(div)) does not have continuations.
Which means we're leaving a bunch of placeholder continuations dangling...

This frame tree looks weird to me. I thought it was a frame tree invariant
that continuations of in-flow descendants were descendants of the frame
itself or its continuation.  That is, there should be continuations for
the yellow DIV and green SPAN between the cyan DIV and red placeholder,
so that when the green SPAN is removed, its continuations are removed and
thereby also all (in-flow) descendant continuations.

If this is not the case (anymore?) then we need to make the
frame destruction code aware of this.
Float placeholder continuations have always been children of the next-in-flow block. I thought the frame destruction code was aware of that.
Blocks: 401734
Attached patch wip (obsolete) — Splinter Review
(This isn't quite right, we shouldn't traverse out-of-flow continuations
 and the "!aRemovedFrame->GetNextContinuation()" optimization is probably
 not correct either. (eg. if the green Inline(span) had a continuation))
Attachment #289775 - Attachment is obsolete: true
Attached patch Patch rev. 1Splinter Review
This should do it, but only if the following assumption is correct:
All in-flow descendants (including placeholders) of an
*out-of-flow continuation* must be a continuation frame and the
start of that continuation chain must be a descendant of the first
frame in the out-of-flow continuation chain.
(so we can still avoid traversing continuations in DoDeletingFrameSubtree)
Assignee: nobody → mats.palmgren
Attachment #289864 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #289865 - Flags: review?(bzbarsky)
Flags: in-testsuite?
OS: Mac OS X → All
Hardware: PC → All
You should get roc to review this; he understands our continuation model invariants better than I do.
Attachment #289865 - Flags: superreview?(roc)
Attachment #289865 - Flags: review?(roc)
Attachment #289865 - Flags: review?(bzbarsky)
Flags: blocking1.9? → blocking1.9+
Priority: -- → P3
I fixed the wiki docs. Sorry about that. We really need to get rid of float placeholder continuations :-(.

> All in-flow descendants (including placeholders) of an
> *out-of-flow continuation* must be a continuation frame

I'm not sure what you mean by this. It's certainly possible for a float continuation to have a child that is a first-in-flow. For example there might be a child element that just starts on the second page.
Flags: wanted-next+
Flags: blocking1.9-
Flags: tracking1.9+
Is the attached patch good or bad?
Testcase in comment #0 WFM on Mac trunk Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1a2pre) Gecko/20080818150509 Minefield/3.1a2pre with mallocscribble.
Mats / roc, is this patch still relevant after the patch for bug 413048 (which disabled float breaking in columns)?
probably not. We'd like to reenable float breaking in columns eventually, but probably only after some major architectural changes that would obsolete this patch. By the way, it would be good if the WFM float/column bugs could have crashtests checked in so we don't lose them when we come to reenable float breaking in columns.
I'm planning to check in crashtests as soon as I get a chance to check in :)  I'm going to skip the security-sensitive bugs for now, though.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → WORKSFORME
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: