683712 - Crash [@ CalcQuirkContainingBlockHeight]

Status: RESOLVED FIXED

(Core :: Layout, defect, P2)

Description

[Martijn Wargers (dead)]

I have an unminimized testcase with this crash stack:

https://crash-stats.mozilla.com/report/index/bp-184613e9-30b4-4e36-a653-a75352110831
0 	xul.dll 	CalcQuirkContainingBlockHeight 	layout/generic/nsHTMLReflowState.cpp:1548
1 	xul.dll 	nsHTMLReflowState::InitConstraints 	
2 	xul.dll 	nsTArray<nsIFrame*,nsTArrayDefaultAllocator>::Clear 	obj-firefox/dist/include/nsTArray.h:886
3 	xul.dll 	nsStyleContext::DoGetStylePadding 	obj-firefox/dist/include/nsStyleStructList.h:139
4 	xul.dll 	nsHTMLReflowState::Init 	layout/generic/nsHTMLReflowState.cpp:282

Dumping it here, maybe someone can fix it by looking at the stack.

Comment 1

[Mats Palmgren (inactive)]

Is it a regression?

Comment 2

[Martijn Wargers (dead)]

It's also happening in a 2011-08-24 build, so it's not a regression from bug 679933.
I'll try to get a testcase next week or so.

Comment 3

[Boris Zbarsky [:bzbarsky]]

It's a null deref crash on this code:

  rs->parentReflowState->frame->GetType()

rs is known-null here, so I would guess rs->parentReflowState is null.

Martijn, can you attach the unminimized testcase?  Might well be fixable from that.

Comment 4

[Bob Clary [:bc] (inactive)]

http://www.mxgraph.com/demo/markup/webkitbg.html

Nightly, Aurora, Beta - Linux, Mac, Windows.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Bob, thanks!

As expected, rs->parentReflowState is null.

rs->frame->mContent is a nsSVGForeignObjectElement, and rs->frame is correctly an nsBlockFrame.

Sounds like our basic assumption here is that an nsBlockFrame can't be a reflow root, and that's just not true.

We probably just need to add a null-check for rs->parentReflowState.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Add a null-check to avoid bogus assumptions about blocks not being reflow roots.

Comment 7

[David Baron :dbaron:]

Shouldn't the fixed height of a foreignObject be relevant when computing percentage heights on its kids?  (If it is, why are we in this code?)

Comment 8

[Boris Zbarsky [:bzbarsky]]

> Shouldn't the fixed height of a foreignObject be relevant when computing percentage
> heights on its kids? 

It is, but in this case we have a percent-height kid of an auto height kid of the <foreignObject>.  And the page is in quirks mode, so the innermost block ends up in CalcQuirkContainingBlockHeight.

Or did I misunderstand your question?

Comment 9

[Boris Zbarsky [:bzbarsky]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/898f04b9db6b

Comment 10

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/898f04b9db6b