629035 - Crash Report [@ nsCSSFrameConstructor::SetUpDocElementContainingBlock(nsIContent*) ][@ nsCSSFrameConstructor::SetUpDocElementContainingBlock ]

Status: RESOLVED FIXED

(Core :: Layout, defect, P1)

Description

[Carsten Book [:Tomcat]]

windows topcrash #10 for Crash Report [@ nsCSSFrameConstructor::SetUpDocElementContainingBlock(nsIContent*) ] 

http://crash-stats.mozilla.com/report/list?signature=nsCSSFrameConstructor::SetUpDocElementContainingBlock%28nsIContent*%29 Windows only so far - no comments so far

Crashing Thread
Frame 	Module 	Signature [Expand] 	Source
0 	xul.dll 	nsCSSFrameConstructor::SetUpDocElementContainingBlock 	layout/base/nsCSSFrameConstructor.cpp:2643
1 	xul.dll 	nsCSSFrameConstructor::ConstructDocElementFrame 	layout/base/nsCSSFrameConstructor.cpp:2296
2 	xul.dll 	nsCSSFrameConstructor::ContentRangeInserted 	layout/base/nsCSSFrameConstructor.cpp:6920
3 	xul.dll 	nsCSSFrameConstructor::ContentInserted 	layout/base/nsCSSFrameConstructor.cpp:6807
4 	xul.dll 	nsCSSFrameConstructor::RecreateFramesForContent 	layout/base/nsCSSFrameConstructor.cpp:9176
5 	xul.dll 	nsCSSFrameConstructor::ReconstructDocElementHierarchy 	layout/base/nsCSSFrameConstructor.cpp:5508
6 	xul.dll 	PresShell::ReconstructFrames 	layout/base/nsPresShell.cpp:5159
7 	xul.dll 	nsPresContext::SetBidi 	
8 	xul.dll 	NS_InvokeByIndex_P 	xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:102
9 	xul.dll 	XPCWrappedNative::CallMethod 	js/src/xpconnect/src/xpcwrappednative.cpp:2298
10 	xul.dll 	XPC_WN_GetterSetter 	js/src/xpconnect/src/xpcwrappednativejsops.cpp:1635
11 	mozjs.dll 	js::Invoke 	js/src/jsinterp.cpp:700
12 	mozjs.dll 	js::ExternalInvoke 	js/src/jsinterp.cpp:858
13 	mozjs.dll 	js::ExternalGetOrSet 	js/src/jsinterp.cpp:898
14 	mozjs.dll 	js::Shape::set 	js/src/jsscopeinlines.h:266
15 	xul.dll 	xul.dll@0xe837f 	
16 	mozjs.dll 	js::mjit::stubs::SetName<0> 	js/src/methodjit/StubCalls.cpp:261
17 	mozjs.dll 	mozjs.dll@0x134e6f 	
18 	mozjs.dll 	js::mjit::ic::SetProp 	js/src/methodjit/PolyIC.cpp:1741
19 	mozjs.dll 	js::mjit::ic::Call 	js/src/methodjit/MonoIC.cpp:859
20 	mozjs.dll 	js::mjit::EnterMethodJIT 	js/src/methodjit/MethodJIT.cpp:748
21 	mozjs.dll 	CheckStackAndEnterMethodJIT 	js/src/methodjit/MethodJIT.cpp:774
22 	mozjs.dll 	js::mjit::JaegerShot 	js/src/methodjit/MethodJIT.cpp:791
23 	mozjs.dll 	js::RunScript 	js/src/jsinterp.cpp:654
24 	mozjs.dll 	js::Execute 	js/src/jsinterp.cpp:1023
25 	mozjs.dll 	JS_EvaluateUCScriptForPrincipals 	js/src/jsapi.cpp:4930
26 	mozjs.dll 	JS_EvaluateUCScriptForPrincipalsVersion 	js/src/jsapi.cpp:4906
27 	xul.dll 	nsJSContext::EvaluateString 	dom/base/nsJSEnvironment.cpp:1551

Comment 1

[Boris Zbarsky [:bzbarsky]]

Uh... how did JS code land us in SetBidi?

In any case, all those crashes are at 0x18.  The line they're on is:

  nsStyleContext* viewportPseudoStyle = viewportFrame->GetStyleContext();

offsetof(nsIFrame, mStyleContext == 0x18 in a 32-bit build.  So presumably mFixedContainingBlock is null.

I think nsPresShell::ReconstructFrames should return early if !mDidInitialReflow.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Don't try to reconstruct frames if we don't have any.

Comment 3

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

Comment on attachment 507187 [details] [diff] [review]
Don't try to reconstruct frames if we don't have any.

r=dbaron, though you might want to assert that FrameManager()->GetRootFrame() is null (or maybe even just make the test a test of GetRootFrame()).

Comment 4

[Boris Zbarsky [:bzbarsky]]

> though you might want to assert that FrameManager()->GetRootFrame() is null

Did that.

Comment 5

[(no longer active)]

(In reply to comment #1)
> Uh... how did JS code land us in SetBidi?

I'd guess by setting the dir property: <http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsDocument.cpp#5619>, even though nsDocument::SetDir doesn't seem to be anywhere on the stack...

Comment 6

[Boris Zbarsky [:bzbarsky]]

Yeah, that was sorta my concern, but inlining could be weirding things out here.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/fe8180d48b5d and pushed a test as http://hg.mozilla.org/mozilla-central/rev/979ef5bad8c4

Comment 8

[Boris Zbarsky [:bzbarsky]]

And http://hg.mozilla.org/mozilla-central/rev/37bf42a252ef to push the actual test.

Comment 9

[Tony Chung [:tchung]]

How to test this: check if tinderbox is green and monitor 4.0b11 crash-stats?

Comment 10

[Boris Zbarsky [:bzbarsky]]

Yep, since the latter is how we discovered the bug in the first place.