194952 - M17x FF104 crash on changing float value for bound element [@ GetNearestContainingBlock] [@ nsIFrame::GetParent]

Status: RESOLVED FIXED

(Core :: Layout: Floats, defect, P2)

Description

[Boris Zbarsky [:bzbarsky]]

This is filed based on the post at
http://www.mozillazine.org/forums/viewtopic.php?t=6538

Testcase coming up (this is crashing my tip CVS build, btw).

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: The XBL (pretty minimal)

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: HTML testcase (this will crash!)

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: same HTML as plaintext, for ease of reading.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Notes:

1)  The style has to be changed from the <constructor> -- changing it from the
    onload event of the HTML page does nothing.
2)  The position:absolute on the anonymous <div> is necessary.
3)  <children/> can be inside or outside the <div>; if it's not present at all,
    the binding seems to not get applied at all.

We manage to fire about every possible assertion that can fire with this setup:

###!!! ASSERTION: not in child list: 'frameList.ContainsFrame(aChildFrame)',
file
/home/bzbarsky/mozilla/profile/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp,
line 1261
###!!! ASSERTION: didn't find frame to delete: 'result', file
/home/bzbarsky/mozilla/profile/mozilla/layout/html/base/src/nsAbsoluteContainingBlock.cpp,
line 138
###!!! ASSERTION: can't find deleted frame in lines: 'line != line_end', file
/home/bzbarsky/mozilla/profile/mozilla/layout/html/base/src/nsBlockFrame.cpp,
line 5050
###!!! ASSERTION: bad prevSibling: 'tmp == aDeletedFrame', file
/home/bzbarsky/mozilla/profile/mozilla/layout/html/base/src/nsBlockFrame.cpp,
line 5054
###!!! ASSERTION: no placeholder frame: 'nsnull != placeholderFrame', file
/home/bzbarsky/mozilla/profile/mozilla/layout/html/base/src/nsHTMLReflowState.cpp,
line 976

The crash follows directly from this last assertion:

(gdb) frame 0
#0  nsIFrame::GetParent (this=0x0, aParent=0xbfffd09c)
    at ../../../../dist/include/layout/nsIFrame.h:616
616       nsresult GetParent(nsIFrame** aParent) const { *aParent = mParent;
return NS_OK; }
(gdb) p this
$1 = (nsIFrame *) 0x0

(gdb) frame 2
#2  0x40f06759 in nsHTMLReflowState::InitAbsoluteConstraints (this=0xbfffd2d0, 
    aPresContext=0x8721968, cbrs=0xbfffd9f0, containingBlockWidth=13650, 
    containingBlockHeight=8204)
    at
/home/bzbarsky/mozilla/profile/mozilla/layout/html/base/src/nsHTMLReflowState.cpp:981
981       nsIFrame* blockFrame = GetNearestContainingBlock(placeholderFrame,
(gdb) p placeholderFrame
$2 = (nsIFrame *) 0x0

My initial guess, not knowing the XBL stuff very well, is that the binding is
attached from inside frame construction, the <constructor> executes, causes an
inline style change and the ensuing synchronous reframe, then the call to
binding attachment unwinds and we continue with the frame construction we were
in the middle of... except the frame we were binding got destroyed and things
are all confused.

Stack:

#0  nsIFrame::GetParent (this=0x0, aParent=0xbfffd09c)
    at ../../../../dist/include/layout/nsIFrame.h:616
#1  0x40f05baa in GetNearestContainingBlock (aFrame=0x0, aContentArea=@0xbfffd120)
    at
/home/bzbarsky/mozilla/profile/mozilla/layout/html/base/src/nsHTMLReflowState.cpp:595
#2  0x40f06759 in nsHTMLReflowState::InitAbsoluteConstraints (this=0xbfffd2d0, 
    aPresContext=0x8721968, cbrs=0xbfffd9f0, containingBlockWidth=13650, 
    containingBlockHeight=8204)
    at
/home/bzbarsky/mozilla/profile/mozilla/layout/html/base/src/nsHTMLReflowState.cpp:981
#3  0x40f086de in nsHTMLReflowState::InitConstraints (this=0xbfffd2d0, 
    aPresContext=0x8721968, aContainingBlockWidth=13650,
aContainingBlockHeight=8204, 
    aBorder=0x0, aPadding=0x0)
    at
/home/bzbarsky/mozilla/profile/mozilla/layout/html/base/src/nsHTMLReflowState.cpp:1985
#4  0x40f05480 in nsHTMLReflowState::Init (this=0xbfffd2d0, aPresContext=0x8721968, 
    aContainingBlockWidth=-1, aContainingBlockHeight=-1, aBorder=0x0, aPadding=0x0)
    at
/home/bzbarsky/mozilla/profile/mozilla/layout/html/base/src/nsHTMLReflowState.cpp:337
#5  0x40f0533e in nsHTMLReflowState::nsHTMLReflowState (this=0xbfffd2d0, 
    aPresContext=0x8721968, aParentReflowState=@0xbfffd9f0, aFrame=0x87c7d58, 
    aAvailableSpace=@0xbfffd39c, aContainingBlockWidth=-1,
aContainingBlockHeight=-1, 
    aReason=eReflowReason_Initial)
    at
/home/bzbarsky/mozilla/profile/mozilla/layout/html/base/src/nsHTMLReflowState.cpp:306
#6  0x40ec6478 in nsAbsoluteContainingBlock::ReflowAbsoluteFrame (this=0x87c6198, 
    aDelegatingFrame=0x87c614c, aPresContext=0x8721968, aReflowState=@0xbfffd9f0, 
    aContainingBlockWidth=-1, aContainingBlockHeight=-1, aKidFrame=0x87c7d58, 
    aReason=eReflowReason_Initial, aStatus=@0xbfffd4bc)
    at
/home/bzbarsky/mozilla/profile/mozilla/layout/html/base/src/nsAbsoluteContainingBlock.cpp:477
#7  0x40ec605b in nsAbsoluteContainingBlock::IncrementalReflow (this=0x87c6198, 
    aDelegatingFrame=0x87c614c, aPresContext=0x8721968, aReflowState=@0xbfffd9f0, 
    aContainingBlockWidth=-1, aContainingBlockHeight=-1, aWasHandled=@0xbfffd8d4)
    at
/home/bzbarsky/mozilla/profile/mozilla/layout/html/base/src/nsAbsoluteContainingBlock.cpp:355
#8  0x40ec9635 in nsBlockFrame::Reflow (this=0x87c614c, aPresContext=0x8721968, 
    aMetrics=@0xbfffdaa8, aReflowState=@0xbfffd9f0, aStatus=@0xbfffdcfc)
    at
/home/bzbarsky/mozilla/profile/mozilla/layout/html/base/src/nsBlockFrame.cpp:757
#9  0x40ee1fae in nsContainerFrame::ReflowChild (this=0x87ba46c,
aKidFrame=0x87c614c, 
    aPresContext=0x8721968, aDesiredSize=@0xbfffdaa8, aReflowState=@0xbfffd9f0,
aX=0, 
    aY=0, aFlags=0, aStatus=@0xbfffdcfc)
    at
/home/bzbarsky/mozilla/profile/mozilla/layout/html/base/src/nsContainerFrame.cpp:929
#10 0x40f03d59 in CanvasFrame::Reflow (this=0x87ba46c, aPresContext=0x8721968, 
    aDesiredSize=@0xbfffdcb0, aReflowState=@0xbfffdb78, aStatus=@0xbfffdcfc)
    at
/home/bzbarsky/mozilla/profile/mozilla/layout/html/base/src/nsHTMLFrame.cpp:587

Comment 5

[Martijn Wargers (dead)]

http://bugzilla.mozilla.org/show_bug.cgi?id=194952#c4
Note 2: It also applies for position:fixed for the anonymous div.

Comment 6

[Jay Patel [:jay]]

Adding M17rc1 to summary adn topcrash keyword to keep this on our radar.  Not
sure it is a true topcrash, but since we have a reproducible testcase it might
be worth looking into for Mozilla 1.7.  

I just crashed with rc1.  Will update bug with my Talkback incident as soon as
it gets processed.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Jay, see comment 4.... no need for more stacks or comments -- we already know
exactly where this is crashing and even the immediate reason for the crash,
though not quite how we get into that state.  If your stack looks different,
it's a different bug.

Comment 8

[Jay Patel [:jay]]

My stack signature was different, but the stack trace looked almost the same.  I
think bug 241509 might be dup of this one (my crash was in 
GetNearestContainingBlock).

Comment 9

[chris hofmann]

any chance for a patch for this in time for 1.7?

Comment 10

[Boris Zbarsky [:bzbarsky]]

Maybe the real issue to look into is the synchronous reframe?

Comment 11

[Boris Zbarsky [:bzbarsky]]

OK, so I'm guessing that the reframe happens in ContentAppended() when we call
ProcessAttachedQueue and the frames are _not_ removed from the various
framelists properly because they've not been hooked up yet.  Then we stick bogus
frames in the frame tree.

Maybe we should just move the queue processing to the end of ContentAppended (or
off onto an event altogether?  But that wouldn't attach bindings to nodes
immediately on insertion into the document...)

Comment 12

[Jay Patel [:jay]]

Boris:  Is this a dup of bug 133219?  Any updates on a patch?  

Comment 13

[Boris Zbarsky [:bzbarsky]]

(In reply to comment #12)
> Is this a dup of bug 133219?

Unlikely, unless I've mis-diagnosed the problem here (since this bug is
specifically an XBL issue).

> Any updates on a patch?  

The problem is that I don't know enough about how XBL is "supposed" to work (or
more precisely what exactly our chrome assumes about it) to know what would be
safe things to try.  If I had a few days to spend on chasing regressions, maybe,
but I won't have that till July.

I _think_ moving off the binding attachment to the end of ContentAppended should
be safe... but I'm not sure.

Comment 14

[Jay Patel [:jay]]

Updating summary with M17rc2 and [@  GetNearestContainingBlock] since the HTML
testcase is still crashing for me with Mozilla 1.7 rc2 under that stack signature. 

Since we have a reproducible testcase for this, it would be nice to get a fix
for Mozilla 1.7.  Anyone else have time to work on this?

Comment 15

[Jay Patel [:jay]]

This continues to crash for the latest releases of Mozilla 1.7 and Firefox 0.9.
Marking this topcrash+ since we have a reproducible testcase and nominating
for  	blocking-aviary1.0.

Comment 16

[Boris Zbarsky [:bzbarsky]]

Note that the fix for bug 230170 would also fix this (by making the reframe async).

Comment 17

[Jay Patel [:jay]]

Looks like bug 230170 was fixed, but can anyone confirm that this made it onto
the Mozilla 1.7 and Aviary-1.0 branches?  Boris?

Comment 18

[Boris Zbarsky [:bzbarsky]]

No, bug 230170 did not make it onto any branches (and will not).

Quite frankly, I doubt there is a "safe" fix to this bug (one that I,
personally, would be willing to check into a branch).

Comment 19

[chris hofmann]

sounds like minus for the aviary branch.  renominate if I've read this wrong or
a safe fix appears in the next few weeks.

Comment 20

[Jay Patel [:jay]]

Since a fix for this has been checked into the Trunk...and I don't see a large
number of these crashes anymore in recent Talkback data for Firefox 1.0PR1 or
Mozilla 1.7.3, marking this worksforme to get it off the radar.

Comment 21

[Boris Zbarsky [:bzbarsky]]

Reopening.  All that's needed is to add |var foo = document.body.offsetWidth;|
after the style change in the testcases here and we'll still crash.

Basically, we have to prevent the situation in comment 4 from occuring -- we
need to fire constructors later in frame construction, when the frame tree is
all set up.

Shaver has a bug that has some comments to that effect, so ccing him -- we
should mark this dependent.

Comment 22

[Jay Patel [:jay]]

Still crashing with HTML testcase, so updating summary with FF10RC2.  Still on
the topcrash list, although a little ways down in the rankings.

This was -'d for 1.0, but since we seem to know what's going on here, we should
revisit this after 1.0 is out the door.

Comment 23

[timeless]

*** Bug 269985 has been marked as a duplicate of this bug. ***

Comment 24

[Jay Patel [:jay]]

Anyone cc'd on this bug have an update on this?  Any chance we can get it in for
Mozilla 1.7.5?

Comment 25

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

Is there a good reason to think that the case discussed in this bug is the same
one that's causing the topcrash?  If not, the topcrash should be discussed on a
separate bug.

Comment 26

[Jay Patel [:jay]]

This isn't really a topcrash anymore, but since we know what the problem is and
have a possible fix, I figured we should get it in.

Comment 27

[Boris Zbarsky [:bzbarsky]]

> and have a possible fix

We do?  Did I miss something?  Also, note comment 18.

Comment 28

[Jay Patel [:jay]]

My mistake, I misread comment #21.  I guess we'll have to wait for a "safe fix"
for the branch and/or see how the bug you mention Shaver commented on is resolved.

Comment 29

[Asa Dotzler [:asa]]

1.7.5 has shipped. Moving request to 1.7.6.

Comment 30

[Daniel Veditz [:dveditz]]

without a trunk-baked patch, a- for 1.7.6 from drivers

Comment 31

[timeless]

*** Bug 286677 has been marked as a duplicate of this bug. ***

Comment 32

[timeless]

*** Bug 287981 has been marked as a duplicate of this bug. ***

Comment 33

[Martijn Wargers (dead)]

Attachment: Testcase

The xbl file of the previous testcase had a namespace error (which Mozilla now
supports because the xml parser was updated some time ago).
This testcase is still basically the same as the previous one.
It doesn't crash anymore in current trunk builds.

Comment 34

[Martijn Wargers (dead)]

Ok, that testcase crashes 2004-08-09 build, but not the 2004-08-10 build
anymore. Maybe the fix from bug 230170 fixed this?

Comment 35

[Boris Zbarsky [:bzbarsky]]

Martijn, see comment 16 and comment 21...

Comment 36

[Jay Patel [:jay]]

For the record, this continues to be a topcrasher for Firefox 1.0.4 and will
remain on the Aviary branch until we fix it there.  It is the #5 topcrasher, so
if someone does come up with a fix, we might consider it for a future 1.0.x
release...so please submit a patch if you have one.

I don't see any incidents in Trunk builds or the Deer Park Alpha 1, so we should
be ok going forward with Firefox 1.1.

Comment 37

[Jay Patel [:jay]]

Here is a link to the latest Aviary branch crashes for GetNearestContainingBlock:
http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=1&searchby=stacksig&match=contains&searchfor=GetNearestContainingBlock&vendor=MozillaOrg&product=Firefox10&platform=All&buildid=&sdate=&stime=&edate=&etime=&sortby=bbid

Taking a closer look at past Talkback data, this crash didn't show up in the
topcrash list for previous 1.0.x releases until 1.0.4.  Did we regress somewhere
between 1.0 and 1.0.4?

Comment 38

[Jay Patel [:jay]]

*** Bug 293099 has been marked as a duplicate of this bug. ***

Comment 39

[Jay Patel [:jay]]

*** Bug 291352 has been marked as a duplicate of this bug. ***

Comment 40

[Jay Patel [:jay]]

*** Bug 288790 has been marked as a duplicate of this bug. ***

Comment 41

[Martijn Wargers (dead)]

Bug 288790 has a testcase that consistently crashes for me (when it is seen
locally): https://bugzilla.mozilla.org/attachment.cgi?id=179415

Comment 42

[Worcester12345]

I was going to say it didn't crash, but it finally did when I went to get out of
the browser. HOWEVER, the page on the "Testcase" was blank, and the original
test case pages came up 404, which is why I posted here. Mozilla/5.0 (Windows;
U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050818 Firefox/1.0+

Comment 43

[Adam Guthrie]

*** Bug 309767 has been marked as a duplicate of this bug. ***

Comment 45

[Martijn Wargers (dead)]

Boris, can this be marked fixed now that bug 267833 is fixed?
The testcases in this bug don't crash at all anymore (for a long time), so it seems quite difficult to test this.

Comment 46

[Boris Zbarsky [:bzbarsky]]

> The testcases in this bug don't crash at all anymore

As I pointed out in comment 21, that's easily corrected with minor changes to the testcase.  Allow me to upload some that crash!

Comment 47

[Boris Zbarsky [:bzbarsky]]

Attachment: Testcase that crashes a 2007-03-04 build

Grabbing newer builds now to test.

Comment 48

[Martijn Wargers (dead)]

Thanks Boris! That testcase crashes nicely in trunk builds ;)
That testcase seems indeed to be fixed by the fix for bug 267833.
It does crash with a 2007-03-09 build, but not anymore with a 2007-03-10 build.

Comment 49

[Boris Zbarsky [:bzbarsky]]

Ah, excellent.

Comment 50

[Worcester12345]

Is this still crashing on branch builds? Would it be considered now (after "baking") to be put on branch builds? Thanks.

Comment 51

[Boris Zbarsky [:bzbarsky]]

There wasn't a patch checked in in this bug.

Comment 52

[Worcester12345]

OK, thanks. I thought that was INVALID or WORKSFORME if there wasn't an actual fix to anything then.

Comment 53

[Boris Zbarsky [:bzbarsky]]

There was a fix.  It's just not being tracked in this bug, but in bug 267833, as is obvious from comment 48.