467914 - Crash [@ nsIFrame::GetOverflowRectRelativeToSelf] with clip-path and -moz-transform on MathML

Status: RESOLVED FIXED

(Core :: MathML, defect)

Description

[Martijn Wargers (dead)]

Attachment: testcase

See testcase, which crashes current trunk build on load.

http://crash-stats.mozilla.com/report/index/5c0a33de-da70-4e65-bb5b-f3a472081204?p=1
0  	xul.dll  	nsIFrame::GetOverflowRectRelativeToSelf  	 layout/generic/nsFrame.cpp:3946
1 	xul.dll 	nsDisplaySVGEffects::nsDisplaySVGEffects 	layout/base/nsDisplayList.cpp:1323
2 	xul.dll 	nsIFrame::BuildDisplayListForStackingContext 	layout/generic/nsFrame.cpp:1307
3 	xul.dll 	nsIFrame::BuildDisplayListForChild 	layout/generic/nsFrame.cpp:1528
4 	xul.dll 	nsBoxFrame::BuildDisplayListForChildren 	layout/xul/base/src/nsBoxFrame.cpp:1317
5 	xul.dll 	nsBoxFrame::BuildDisplayList 	layout/xul/base/src/nsBoxFrame.cpp:1299
6 	xul.dll 	BuildDisplayListWithOverflowClip 	layout/generic/nsFrame.cpp:1141
7 	xul.dll 	nsIFrame::BuildDisplayListForChild 	layout/generic/nsFrame.cpp:1509
8 	xul.dll 	nsBoxFrame::BuildDisplayListForChildren 	layout/xul/base/src/nsBoxFrame.cpp:1317
9 	xul.dll 	nsRootBoxFrame::BuildDisplayList 	layout/xul/base/src/nsRootBoxFrame.cpp:250
10 	xul.dll 	nsIFrame::BuildDisplayListForChild 	layout/generic/nsFrame.cpp:1511
11 	xul.dll 	ViewportFrame::BuildDisplayList 	layout/generic/nsViewportFrame.cpp:109
12 	xul.dll 	nsIFrame::BuildDisplayListForStackingContext 	layout/generic/nsFrame.cpp:1228
13 	xul.dll 	PresShell::RenderDocument 	layout/base/nsPresShell.cpp:4986
14 	xul.dll 	nsCanvasRenderingContext2D::DrawWindow 	content/canvas/src/nsCanvasRenderingContext2D.cpp:3400
15 	xul.dll 	NS_InvokeByIndex_P 	xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:101
16 	xul.dll 	XPCWrappedNative::CallMethod 	js/src/xpconnect/src/xpcwrappednative.cpp:2013
17 	xul.dll 	XPCWrappedNative::CallMethod 	js/src/xpconnect/src/xpcwrappednative.cpp:2422
18 	xul.dll 	XPC_WN_CallMethod 	js/src/xpconnect/src/xpcwrappednativejsops.cpp:1477
19 	js3250.dll 	js_Invoke 	js/src/jsinterp.cpp:1313
20 	js3250.dll 	js_Interpret 	js/src/jsinterp.cpp:5135
21 	js3250.dll 	js_Invoke 	js/src/jsinterp.cpp:1331
22 	xul.dll 	nsXPCWrappedJSClass::CallMethod 	js/src/xpconnect/src/xpcwrappedjsclass.cpp:1610
23 	xul.dll 	nsXPCWrappedJS::CallMethod 	js/src/xpconnect/src/xpcwrappedjs.cpp:563
24 	xul.dll 	PrepareAndDispatch 	xpcom/reflect/xptcall/src/md/win32/xptcstubs.cpp:114
25 	xul.dll 	SharedStub 	xpcom/reflect/xptcall/src/md/win32/xptcstubs.cpp:141
26 	xul.dll 	nsEventListenerManager::HandleEventSubType 	content/events/src/nsEventListenerManager.cpp:1091

Comment 1

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

Looks like a null-dereference:  GetProperty(nsGkAtoms::preEffectsBBoxProperty) is returning null, so we crash trying to copy-construct the null.

The frame in question is an nsMathMLmunderFrame.

Comment 2

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

(It wouldn't surprise me if the cause is that the frame in question doesn't call FinishAndStoreOverflow during its reflow.)

Comment 3

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

Attachment: simplified testcase

The binding isn't necessary.

Comment 4

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

Attachment: fix

It doesn't seem wise to be vulnerable to a crash anytime a frame fails to call FinishAndStoreOverflow. In particular, error exits from Reflow could easily get us into this situation. So let's just tolerate the missing preEffectsBBox.

Also, there's really no guarantee that there is a preEffectsBBox; it just so happens that the only current caller of GetOverflowRectRelativeToSelf is nsDisplaySVGEffects. So this fixes that bad assumption too.

Comment 5

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

Comment on attachment 391275 [details] [diff] [review]
fix

I'd think failing to call FinishAndStoreOverflow at least deserves an NS_WARNING, though.

r=dbaron with that

Comment 6

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

We can't easily check for that here. There will be legitimate callers of GetOverflowRectRelativeToSelf that find there is no preEffectsBBox.

Comment 7

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

http://hg.mozilla.org/mozilla-central/rev/25dff0b211a9