472919 - Crash [@ GetContentRect] with -moz-column, frame, fieldset, marquee

Status: RESOLVED FIXED

(Core :: Layout, defect, P3)

Description

[Jesse Ruderman]

Attachment: testcase

This testcase causes crashes like:

Thread 0 Crashed:
0   0xdddddddd
1   nsIFrame::GetContentRect() const + 28 (nsFrame.cpp:702)
2   nsComputedDOMStyle::GetWidth(nsIDOMCSSValue**) + 251 (nsComputedDOMStyle.cpp:2924)
...

A variant crashes [@ nsCSSFrameConstructor::FindPrimaryFrameFor] instead.

Comment 1

[Daniel Holbert [:dholbert]]

I can reproduce this crash on Linux:
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a1pre) Gecko/20090112
Minefield/3.2a1pre

http://crash-stats.mozilla.com/report/index/27e721e7-4d9a-4439-84f8-1d0162090112?p=1
http://crash-stats.mozilla.com/report/index/b4f77e7c-3116-4a26-992b-dc6fc2090112?p=1

OS --> All

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: testcase 2 (no marquee)

I've reduced the original testcase, stripping out the marquee -- it turns out the only bit of the marquee that was needed was a "display: inline-block;" style and a getComputedStyle call (from the marquee binding constructor).

Here's a crash report for this new testcase:
http://crash-stats.mozilla.com/report/index/e99d38cc-a519-48eb-9589-9cd232090112?p=1

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: testcase 3 (no marquee, no frame, reduced height)

Here's a further-reduced testcase.  Turns out the <frame> isn't required, either.

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: testcase 4: doesn't crash, but complains about bad heights

This testcase is the same as the last, except that the height of div#col is 0.02in less:

testcase 3 has height = 372827.02 in
testcase 4 has height = 372827 in;

Testcase 4 doesn't crash, but it triggers this debug output:
###!!! ASSERTION: bad height: 'Not Reached', file nsLineLayout.cpp, line 188
Block(fieldset)(0)@0xb1d800f4: Init: bad caller: height WAS 2147483520(0x7fffff80)
nsBlockReflowContext: ColumnSet(div)(0)@0xb1d7dca8 metrics=6000,2147483520!
nsBlockReflowContext: Block(body)(3)@0xb1d7db28 metrics=54540,2147483520!
WARNING: bad value: file nsFloatManager.cpp, line 149
###!!! ASSERTION: bad height: 'Not Reached', file nsLineLayout.cpp, line 188
Block(fieldset)(0)@0xb1d800f4: Init: bad caller: height WAS 2147483520(0x7fffff80)
nsBlockReflowContext: ColumnSet(div)(0)@0xb1d7dca8 metrics=6000,2147483520!
nsBlockReflowContext: Block(body)(3)@0xb1d7db28 metrics=55440,2147483520!
WARNING: bad value: file nsFloatManager.cpp, line 149
###!!! ASSERTION: bad height: 'Not Reached', file nsLineLayout.cpp, line 188
Block(fieldset)(0)@0xb1d800f4: Init: bad caller: height WAS 2147483520(0x7fffff80)
nsBlockReflowContext: ColumnSet(div)(0)@0xb1d7dca8 metrics=6000,2147483520!
nsBlockReflowContext: Block(body)(3)@0xb1d7db28 metrics=55440,2147483520!

Testcase 3 (in contrast) just triggers this debug output before crashing:
###!!! ASSERTION: frame was not removed from primary frame map before destruction or was readded to map after being removed: 'Not Reached', file nsFrameManager.cpp, line 724

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: backtrace of crash on testcase 3

Comment 6

[Daniel Holbert [:dholbert]]

Regression range for testcase 1:
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a7pre) Gecko/2007072504 Minefield/3.0a7pre
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a7pre) Gecko/2007072604 Minefield/3.0a7pre
Bonsai link: http://tinyurl.com/7wf2rl -- probably bug 379349

Regression range for testcases 2 & 3:
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b4pre) Gecko/2008021304 Minefield/3.0b4pre
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b4pre) Gecko/2008021404 Minefield/3.0b4pre
Bonsai link: http://tinyurl.com/9ew7se  -- maybe bug 416018? (cairo upgrade)

(I'll probably split off testcases 2 and 3 into a separate bug, since they seem to have been triggered by a different change.)

Comment 7

[Daniel Holbert [:dholbert]]

(In reply to comment #6)
> (I'll probably split off testcases 2 and 3 into a separate bug, since they seem
> to have been triggered by a different change.)

I just filed bug 473410 for those testcases.

Comment 8

[Boris Zbarsky [:bzbarsky]]

OK, so with testcase 1 I get:

###!!! ASSERTION: frame was not removed from primary frame map before destruction or was readded to map after being removed: 'Not Reached', file /Users/bzbarsky/mozilla/debug/mozilla/layout/base/nsFrameManager.cpp, line 734

a number of times.  Then later we crash making a virtual function call on the last frame that this assert fired for:

#4  0x035eb9b5 in nsComputedDOMStyle::GetWidth (this=0xc0f07c0, aValue=0xbfffab3c) at /Users/bzbarsky/mozilla/debug/mozilla/layout/style/nsComputedDOMStyle.cpp:2924
2924        val->SetAppUnits(mInnerFrame->GetContentRect().width);

(I actually have 3 more stack frames that are complete garbage after that point, so this looks pretty bad to me).

It looks like we go to FlushPendingReflows, and that when getting the width, which ends up destroying a frame like so:

#46 0x034a0c1b in nsBlockFrame::DeleteNextInFlowChild (this=0x211c2810, aPresContext=0x210bfa00, aNextInFlow=0x21213f14, aDeletingEmptyFrames=1) at /Users/bzbarsky/mozilla/debug/mozilla/layout/generic/nsBlockFrame.cpp:5630
#47 0x034ad9ff in nsBlockReflowContext::ReflowBlock (this=0xbfff7b48, aSpace=@0xbfff7bfc, aApplyTopMargin=0, aPrevMargin=@0xbfff8290, aClearance=0, aIsAdjacentWithTop=1, aLine=0x21286aac, aFrameRS=@0xbfff7aa0, aFrameReflowStatus=@0xbfff7bec, aState=@0xbfff8208) at /Users/bzbarsky/mozilla/debug/mozilla/layout/generic/nsBlockReflowContext.cpp:354

Now the bad part is that apparently this ends up destroying some frames that are primary frames for some content.  I would have thought we'd have stolen those before this point.

Comment 9

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

(In reply to comment #8)
> OK, so with testcase 1 I get:
> 
> ###!!! ASSERTION: frame was not removed from primary frame map before
> destruction or was readded to map after being removed: 'Not Reached', file
> /Users/bzbarsky/mozilla/debug/mozilla/layout/base/nsFrameManager.cpp, line 734
> 
> a number of times.

Maybe this is the same problem I described in bug 463350 comment 7?

Comment 10

[Damon Sicore (:damons)]

Who is going to take this?  I think it's time to get an owner for each blocker at this point.

Comment 11

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

I just checked the assertion stack, and it matches the problem I described in bug 463350 comment 7 (stack goes through the call to DeleteNextInFlowChild from nsBlockReflowContext::ReflowBlock), so I think this is probably a dependency/duplicate of bug 463350, and we should just wait on that.

Comment 12

[Daniel Holbert [:dholbert]]

(In reply to comment #11)
> I just checked the assertion stack, and it matches the problem I described in
> bug 463350 comment 7

Just to confirm this hypothesis -- these bugs have the same regression range, as noted in bug 463350 comment 16.

Comment 13

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

Fixed by checkin for bug 463350