Closed Bug 596245 Opened 9 years ago Closed 9 years ago

Firefox 4.0b5 Crash Report [@ nsStyleContext::Release() ] [@ nsStyleContext::~nsStyleContext ] Search Mozilla Support for Help

Categories

(Core :: CSS Parsing and Computation, defect, P2, critical)

defect

Tracking

()

RESOLVED FIXED
mozilla2.0b7
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: chofmann, Assigned: dbaron)

Details

(Keywords: crash, regression, Whiteboard: [tbird crash])

Crash Data

Attachments

(2 files)

might have been around for a while, but volume could be increasing in 4.0b...

checking --- nsStyleContext::Release.. 20100913-crashdata.csv
found in: 4.0b5 3.6.9 4.0b3 4.0b6pre 3.6.8 3.6.6
release total-crashes
              nsStyleContext::Release.. crashes
                         pct.
all     358954  72      0.000200583
4.0b5   42210   59      0.00139777
3.6.9   167129  7       4.18838e-05
4.0b3   1597    3       0.00187852
4.0b6pre5444    1       0.000183688
3.6.8   49757   1       2.00977e-05
3.6.6   7874    1       0.000127


In a quick look I spotted a couple of forms of the stack.

http://crash-stats.mozilla.com/report/index/67c972bf-ab33-4ed8-843d-ca7c82100908

Frame  	Module  	Signature [Expand]  	Source
0 	xul.dll 	nsStyleContext::Release 	layout/style/nsStyleContext.h:102
1 	xul.dll 	nsStyleContext::Release 	layout/style/nsStyleContext.h:102
2 	xul.dll 	nsComputedDOMStyle::GetPropertyCSSValue 	layout/style/nsComputedDOMStyle.cpp:557
3 	xul.dll 	nsComputedDOMStyle::GetPropertyValue 	layout/style/nsComputedDOMStyle.cpp:310
4 	xul.dll 	nsIDOMCSSStyleDeclaration_GetPropertyValue 	obj-firefox/js/src/xpconnect/src/dom_quickstubs.cpp:13286
5 	xul.dll 	js::Interpret 	js/src/jsinterp.cpp:4696
6 	xul.dll 	js::InvokeCommon<int > 	js/src/jsinterp.cpp:577
7 	xul.dll 	js::Invoke 	js/src/jsinterp.cpp:696
8 	xul.dll 	js::InternalInvoke 	js/src/jsinterp.cpp:736
9 	xul.dll 	nsJSContext::CallEventHandler 	dom/base/nsJSEnvironment.cpp:2248
10 	xul.dll 	nsGlobalWindow::RunTimeout 	dom/base/nsGlobalWindow.cpp:8585
11 	xul.dll 	nsGlobalWindow::TimerCallback 	dom/base/nsGlobalWindow.cpp:8930
12 	xul.dll 	nsTimerImpl::Fire 	xpcom/threads/nsTimerImpl.cpp:425
13 	xul.dll 	nsTimerEvent::Run 	xpcom/threads/nsTimerImpl.cpp:517
14 	xul.dll 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:547
15 	xul.dll 	mozilla::ipc::MessagePump::Run 	ipc/glue/MessagePump.cpp:110
16 	xul.dll 	xul.dll@0xb94e03 	
17 	xul.dll 	MessageLoop::RunInternal 	ipc/chromium/src/base/message_loop.cc:219
18 	xul.dll 	MessageLoop::RunHandler 	ipc/chromium/src/base/message_loop.cc:202
19 	xul.dll 	XPCConvert::NativeData2JS 	js/src/xpconnect/src/xpcconvert.cpp:484
20 	xul.dll 	_SEH_epilog4 	
21 	xul.dll 	MessageLoop::Run 	ipc/chromium/src/base/message_loop.cc:176
22 	xul.dll 	nsBaseAppShell::Run 	widget/src/xpwidgets/nsBaseAppShell.cpp:175
23 	xul.dll 	nsAppShell::Run 	widget/src/windows/nsAppShell.cpp:243
24 	nss3.dll 	pkix_pl_Pk11CertStore_CertQuery 	security/nss/lib/libpkix/pkix_pl_nss/module/pkix_pl_pk11certstore.c:220

http://crash-stats.mozilla.com/report/index/5099dc66-a8fa-40e6-a1f0-ac0d52100914

Frame  	Module  	Signature [Expand]  	Source
0 	xul.dll 	nsStyleContext::Release 	layout/style/nsStyleContext.h:102
1 	xul.dll 	nsStyleContext::~nsStyleContext 	layout/style/nsStyleContext.cpp:119
2 	xul.dll 	nsStyleContext::Release 	layout/style/nsStyleContext.h:102
3 	xul.dll 	nsStyleContext::~nsStyleContext 	layout/style/nsStyleContext.cpp:119
4 	xul.dll 	nsStyleContext::Release 	layout/style/nsStyleContext.h:102
5 	xul.dll 	nsStyleContext::~nsStyleContext 	layout/style/nsStyleContext.cpp:119
6 	xul.dll 	nsStyleContext::Release 	layout/style/nsStyleContext.h:102
7 	xul.dll 	nsStyleContext::~nsStyleContext 	layout/style/nsStyleContext.cpp:119
8 	xul.dll 	nsStyleContext::Release 	layout/style/nsStyleContext.h:102
9 	xul.dll 	nsComputedDOMStyle::GetPropertyCSSValue 	layout/style/nsComputedDOMStyle.cpp:557
10 	xul.dll 	nsComputedDOMStyle::GetPropertyValue 	layout/style/nsComputedDOMStyle.cpp:310
11 	xul.dll 	NS_InvokeByIndex_P 	xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:102
12 	xul.dll 	js::InvokeCommon<int > 	js/src/jsinterp.cpp:566
13 	xul.dll 	js::Invoke 	js/src/jsinterp.cpp:696
14 	xul.dll 	js::InternalInvoke 	js/src/jsinterp.cpp:736
15 	xul.dll 	JS_CallFunctionValue 	js/src/jsapi.cpp:4874
16 	xul.dll 	XPC_NW_FunctionWrapper 	js/src/xpconnect/src/XPCNativeWrapper.cpp:471
17 	xul.dll 	js::InvokeCommon<int > 	js/src/jsinterp.cpp:566
18 	xul.dll 	js::Invoke 	js/src/jsinterp.cpp:696
19 	xul.dll 	js::Interpret 	js/src/jsinterp.cpp:4707
20 	xul.dll 	js::InvokeCommon<int > 	js/src/jsinterp.cpp:577
21 	xul.dll 	js::Invoke 	js/src/jsinterp.cpp:696
22 	xul.dll 	js_fun_apply 	js/src/jsfun.cpp:2430
23 	xul.dll 	js::Interpret 	js/src/jsinterp.cpp:4696
24 	xul.dll 	js::InvokeCommon<int > 	js/src/jsinterp.cpp:577
25 	xul.dll 	js::Invoke 	js/src/jsinterp.cpp:696
26 	xul.dll 	array_extra 	js/src/jsarray.cpp:2807
27 	xul.dll 	array_forEach 	js/src/jsarray.cpp:2862
28 	xul.dll 	js::Interpret 	js/src/jsinterp.cpp:4696
29 	xul.dll 	js::InvokeCommon<int > 	js/src/jsinterp.cpp:577
30 	xul.dll 	js::Invoke 	js/src/jsinterp.cpp:696
31 	xul.dll 	js_fun_apply 	js/src/jsfun.cpp:2430
32 	xul.dll 	js::Interpret 	js/src/jsinterp.cpp:4696
33 	xul.dll 	js::InvokeCommon<int > 	js/src/jsinterp.cpp:577
34 	xul.dll 	js::Invoke 	js/src/jsinterp.cpp:696
35 	xul.dll 	js::InternalInvoke 	js/src/jsinterp.cpp:736
36 	xul.dll 	nsXPCWrappedJSClass::CallMethod 	js/src/xpconnect/src/xpcwrappedjsclass.cpp:1688
37 	xul.dll 	nsXPCWrappedJS::CallMethod 	js/src/xpconnect/src/xpcwrappedjs.cpp:570
38 	xul.dll 	PrepareAndDispatch 	xpcom/reflect/xptcall/src/md/win32/xptcstubs.cpp:114
39 	xul.dll 	SharedStub 	xpcom/reflect/xptcall/src/md/win32/xptcstubs.cpp:141
40 	xul.dll 	nsBrowserStatusFilter::OnStateChange 	toolkit/components/statusfilter/nsBrowserStatusFilter.cpp:183

more reports at

http://crash-stats.mozilla.com/report/list?signature=nsStyleContext::Release%28%29

a lot of users talk about doing translation and urls reflect visits to the microsoft translator


   1 http://www.microsofttranslator.com/bv.aspx?from=zh-CHS&to=tr&a=http%3a%2f%2fwww.microsofttranslator.com%2f
   1 http://www.microsofttranslator.com/bv.aspx?from=en&to=ar&a=http%3A%2F%2Fhaifa6.com%2Fshowthread.php%3Ft%3D17108
   1 http://www.microsofttranslator.com/bv.aspx?from=en&to=ar&a=http%3A%2F%2Fhaifa6.com%2Fforumdisplay.php%3Ff%3D41
   1 http://www.microsofttranslator.com/bv.aspx?from=en&to=ar&a=http%3A%2F%2Fhaifa6.com%2Fforumdisplay.php%3Ff%3D27
   1 http://www.microsofttranslator.com/bv.aspx?from=&to=en&a=http%3A%2F%2Fwww.znews.su%2Fgo_slin.php%3Fid%3D33183%26sour%3D968
   1 http://www.microsofttranslator.com/

multiple crashes also on 

   3 http://www.webocodes.com/inc/pubv.php
   2 http://static.ak.facebook.com/common/redirectiframe.html

os breakdown
nsStyleContext::Release..Total 72
Win5.1  0.38
Win6.0  0.06
Win6.1  0.57
volume started to climb just after b43 got out in circulation

20100808 21  ,7 3.6.6,6 3.6.8,2 4.0b4pre,2 4.0b2,2 4.0b1,1 3.6.3,1 3.6,
20100809 9  ,4 3.6.8,2 3.6.6,1 4.0b1,1 3.6,1 3.5.11,
20100810 17  ,5 3.6.8,2 4.0b4pre,2 4.0b1,2 3.6.3,1 4.0b3,1 3.6.6,1 3.6,
20100811 15  ,6 3.6.8,3 4.0b3,2 4.0b2,2 3.6.6,1 3.6.3,1 3.6,
20100815 26  ,14 4.0b3,4 3.6.8,3 4.0b4pre,1 4.0b2,1 4.0b1,1 3.6b5,1 3.6.3,
20100816 26  ,10 4.0b3,10 3.6.8,2 3.6.6,1 4.0b2,1 4.0b1,1 3.6.4,1 3.6,
20100817 26  ,15 4.0b3,3 3.6.8,3 3.6.6,1 4.0b2,1 3.6,1 3.5b4,1 3.5.6,1 3.5.11,
20100818 18  ,6 3.6.8,4 3.6.6,3 4.0b1,2 4.0b3,1 4.0b4pre,1 3.6.4,1 3.6,
20100819 26  ,18 4.0b3,4 3.6.8,2 4.0b5pre,1 4.0b4pre,1 3.0.5,
20100820 49  ,26 4.0b3,8 3.6.8,4 4.0b5pre,2 4.0b2,2 4.0b1,2 3.5.11,1 4.0b4pre,
20100822 30  ,18 4.0b3,7 3.6.8,1 4.0b5pre,1 4.0b1,1 3.6.6,1 3.6.4,1 3.0.16,
20100823 27  ,19 4.0b3,3 3.6.8,2 4.0b5pre,2 4.0b4pre,1 3.6.3,
20100824 40  ,16 4.0b4,11 4.0b3,7 3.6.8,2 4.0b5pre,2 4.0b1,1 4.0b2,1 3.6,
I can reproduce this on Mac trunk using Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b7pre) Gecko/20100914 Firefox/4.0b7pre and this URL: http://www.microsofttranslator.com/bv.aspx?from=zh-CHS&to=tr&a=http%3a%2f%2fwww.microsofttranslator.com%2f.

The stack is slightly different: [@ nsStyleContext::~nsStyleContext ]

https://crash-stats.mozilla.com/report/index/40451c28-3905-45f1-b8c4-c57902100914
OS: Windows XP → All
Hardware: x86 → All
Summary: Firefox 4.0b5 Crash Report [@ nsStyleContext::Release() ] Search Mozilla Support for Help → Firefox 4.0b5 Crash Report [@ nsStyleContext::Release() ] [@ nsStyleContext::~nsStyleContext ] Search Mozilla Support for Help
Easily 100% reproducible using Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b6) Gecko/20100101 Firefox/4.0b6 as well.
Keywords: regression
I crash easily with today's Minefield nightly on OS X 10.5.8, just
loading one of the URLs from comment #0:

http://www.microsofttranslator.com/bv.aspx?from=zh-CHS&to=tr&a=http%3a%2f%2fwww.microsofttranslator.com%2f

The first time I had to wait 4-5 seconds for the crash.  The second
time it came more or less instantaneously.

bp-ed788708-a240-4a5c-a3b2-f6ca32100914
bp-e45b7b9d-e001-44d4-b5ac-c38e22100914
I've found the regression range for my crash from comment #4:

firefox-2010-08-27-03-mozilla-central
firefox-2010-08-28-03-mozilla-central

http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2010-08-27+03%3A00%3A00&enddate=2010-08-28+03%3A00%3A00

I can't tell which patch in this range might have caused/triggered
this problem.
Attached file unminimized testcase
I'm seeing this assertion multiple times, that I suspect is related to this crash:
###!!! ASSERTION: must be in the same rule tree as parent: 'r1 == r2', file c:/m
ozilla-build/mozilla-central/layout/style/nsStyleContext.cpp, line 92

The problem seems to be coming from the source of the page that is linked in the iframe. I haven't been able to download that site locally and reproduce it.
But the page is having getComputedStyle functions on that page. (and doing document.write stuff among others).
blocking2.0: --- → betaN+
Component: Layout → Style System (CSS)
Priority: -- → P2
QA Contact: layout → style-system
valgrind didn't give any warnings (even the switch in nsPresArena.h flipped)
Boris and I figured this out; the issues are related to using window.getComputedStyle on a node in a document other than window.document.  I need to do a little compatibility testing before figuring out what the right fix is.

http://dev.w3.org/csswg/cssom/#dom-window-getcomputedstyle specs our current behavior, but I want to actually test other browsers first.
(In reply to comment #9)
> Testcase to see what other browsers do:
> http://dbaron.org/css/test/2010/computed-style-cross-window

So, in this testcase, Opera and Chromium agree that the results are pink-blue-pink-blue-black-black.  We do pink-blue-blue-pink-blue-pink.  I'd be interested if somebody could test IE9 beta.
Assignee: nobody → dbaron
Attached patch patchSplinter Review
I think we should do this, and make our display:none behavior consistent with our has-frame behavior.  This also makes us consistent with Opera and WebKit on the third and fourth testcases.

We're the only browser that reports computed style at all for five and six, I think.  (I didn't investigate why they ended up black; could have been a JS error or a report of unstyled-ness or a CSS error.)

If you agree, I'll ask Anne to change the spec, since it'll go from consistent with one implementation to consistent with none (unless IE9 somehow matches it).
Attachment #476175 - Flags: review?(bzbarsky)
(In reply to comment #10)
> (In reply to comment #9)
> > Testcase to see what other browsers do:
> > http://dbaron.org/css/test/2010/computed-style-cross-window
> 
> So, in this testcase, Opera and Chromium agree that the results are
> pink-blue-pink-blue-black-black.  We do pink-blue-blue-pink-blue-pink.  I'd be
> interested if somebody could test IE9 beta.

IE9 beta (9.0.7930.16406)  is same as Opera and Chromium.  pink-blue-pink-blue-black-black
So the behavior implemented here is that someWindow.getComputedStyle(someElement) will, on each property get, check whether someElement is in a document.  If it is, and if the document has a presentation, it will use the styles of that document.  Otherwise, if someWindow.document has a presentation it will use the styles of that presentation.  Otherwise it will throw.

Right?

I'm pretty sure the part about having presentation is not something that maps well to webkit's model; in particular there immediately arise questions about how things should work if someElement is in a data document of some sort.  With the old behavior of using someWindow's styles, at least that issue didn't arise.

I'm probably ok with the behavior change, but I suspect that there will be more spec changes here; iirc the webkit folks had indicated they were considering switching to our behavior at least for nodes not in documents.  But maybe I'm wrong on that...
Comment on attachment 476175 [details] [diff] [review]
patch

r=me as far as the code goes.
Attachment #476175 - Flags: review?(bzbarsky) → review+
So I think the behavior for elements in documents shouldn't change depending on whether the element has a frame (except for the computed styles that actually depend on the frame).

I can't see how we'd fix this without making those consistent and without causing even stranger behavior (reporting one element has one color, and a display:none child that inherits color from it has a different color).

I'm a lot more comfortable changing the behavior for such elements without frames than for those with frames, since I think it's changing the behavior in the less common case.
(In reply to comment #13)
> So the behavior implemented here is that
> someWindow.getComputedStyle(someElement) will, on each property get, check
> whether someElement is in a document.  If it is, and if the document has a
> presentation, it will use the styles of that document.  Otherwise, if
> someWindow.document has a presentation it will use the styles of that
> presentation.  Otherwise it will throw.
> 
> Right?

I think that's right.  I think that means it throws in the exact same set of cases in which it threw before, though.

> I'm pretty sure the part about having presentation is not something that maps
> well to webkit's model; in particular there immediately arise questions about
> how things should work if someElement is in a data document of some sort.  With
> the old behavior of using someWindow's styles, at least that issue didn't
> arise.

So do you think we should change to always using someWindow's styles, and ignoring the frames if the window doesn't match?  It seems like a perfectly sensible behavior, although it seems like a bigger compatibility risk.
> So I think the behavior for elements in documents shouldn't change depending on
> whether the element has a frame

OK, I agree with that premise.

> I'm a lot more comfortable changing the behavior for such elements without
> frames than for those with frames

OK, fair.

> I think that means it throws in the exact same set of
> cases in which it threw before, though.

Yes.

> So do you think we should change to always using someWindow's styles, and
> ignoring the frames if the window doesn't match? 

I want to say yes, but you're right about the compat risk.  Let's go with this patch and get the spec changed.
Whiteboard: [waiting to land until after 4.0b7 freeze]
move up to beta7/8 since sounds like we might have a patch?
http://hg.mozilla.org/mozilla-central/rev/2b78824f6508
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [waiting to land until after 4.0b7 freeze]
Target Milestone: --- → mozilla2.0b8
Target Milestone: mozilla2.0b8 → mozilla2.0b7
This crash is also seen in Thunderbird, eg bp-2110bb75-e627-4b89-96e6-dec2d2101215 TB v3.1.7

there is still a nsStyleContext::Release() crash of a different style, seen in 4.0b8. I didn't look hard to see if it's a new crash
bp-2a5fafb2-9e00-4e71-9339-97d042101215 20101213030326
0	xul.dll	nsStyleContext::Release	layout/style/nsStyleContext.h:102
1	xul.dll	nsCSSFrameConstructor::PropagateScrollToViewport	layout/base/nsCSSFrameConstructor.cpp:2274
Whiteboard: [tbird crash]
Crash Signature: [@ nsStyleContext::Release() ] [@ nsStyleContext::~nsStyleContext ]
You need to log in before you can comment on or make changes to this bug.