620799 - [32-bit Mac OS X] Flash Crash taking down browser [@ gfxContext::Translate]

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Bret Reckard]

Build identifier: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.5; rv:2.0b9pre) Gecko/20101221 Firefox/4.0b9pre

Mac OS 10.5.8 to be exact.

1) Load http://www.xfinity.com/entertainment/celebrity-romance/BDAdxhvGRB0hlTPpRI5d_R9ERBZT8P24/Bret-Michaels-Engaged/?xcr=1&referrer=
2) Enter Full-screen Mode
3) Watch some of the awesome program
4) hit the ESC key to return to browsing
5) CRASH

Note: This Profile is fairly dirty, with 10+ tabs open.  Crashes with Add Block Plus enabled and disabled.  Also have Flashblock installed, but disabled.

Comment 1

[LegNeato]

This isn't 100% reproducible for me on 10.6. I once got a flash crash/oopsie and can only intermittently reproduce that. I have not been able to reproduce the full screen -> esc crash since the first time, and I'm not sure if that was even a browser crash (no report and I was quitting).

Comment 2

[LegNeato]

Bret, can we get the browser crash report from about:crashes?

Comment 3

[Bret Reckard]

http://crash-stats.mozilla.com/report/index/bp-444fa132-befb-4e3f-a9fe-527132101221

http://crash-stats.mozilla.com/report/index/bp-1ac8e972-288a-403b-8436-acc652101221

Comment 4

[LegNeato]

I'm not decent at reading firefox stacks, not sure if this is layout or graphics. Starting with graphics.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Both crashes are at 0x4 on this line in gfxContext::Translate:

  cairo_translate(mCairo, pt.x, pt.y);

mCairo is at offset 4 inside gfxContext on 32-bit systems in opt builds (the refcount is at offset 0, and in opt we don't have the owning thread pointer).  So presumably |this| is null here.

The caller looks like this:

  463  result.mContext->Translate(-gfxPoint(quadrantRect.x, quadrantRect.y));

in ThebesLayerOGL.cpp, where the previous line was:

  462  result.mContext = mTexImage->BeginUpdate(result.mRegionToDraw);

BeginUpdate can return null, in general; e.g. BasicTextureImage::BeginUpdate will return null if the update rect is wrong or if creating an update surface fails...

Bret, you don't have a debug build lying around, do you?

Comment 6

[LegNeato]

I can't get this to crash in my 64 bit nightly, though I do get a reproducible flash oopsie by loading the page, going full screen, escaping out, and then reloading. Perhaps on 32 bit that turns into a crash...

Comment 7

[timeless]

Signature	gfxContext::Translate
UUID	1ac8e972-288a-403b-8436-acc652101221
Time	2010-12-21 14:25:27.178062
Uptime	93
Last Crash	120 seconds before submission
Install Age	111515 seconds (1.3 days) since version was first installed.
Product	Firefox
Version	4.0b9pre
Build ID	20101220030345
Branch	2.0
OS	Mac OS X
OS Version	10.5.8 9L31a
CPU	x86
CPU Info	 family 6 model 23 stepping 10
Crash Reason	EXC_BAD_ACCESS / KERN_PROTECTION_FAILURE
Crash Address	0x4
User Comments	Bret's Crash
App Notes	Renderers: 0x22600,0x20400
Processor Notes	
EMCheckCompatibility	False
Crashing Thread

Frame	Module	Signature [Expand]	Source
0	XUL	gfxContext::Translate	gfx/thebes/gfxContext.cpp:278
1	XUL	mozilla::layers::BasicBufferOGL::BeginPaint	gfx/layers/opengl/ThebesLayerOGL.cpp:463
2	XUL	mozilla::layers::ThebesLayerOGL::RenderLayer	gfx/layers/opengl/ThebesLayerOGL.cpp:546
3	XUL	mozilla::layers::ContainerLayerOGL::RenderLayer	gfx/layers/opengl/ContainerLayerOGL.cpp:236
4	XUL	mozilla::layers::ContainerLayerOGL::RenderLayer	gfx/layers/opengl/ContainerLayerOGL.cpp:236
5	XUL	mozilla::layers::LayerManagerOGL::Render	gfx/layers/opengl/LayerManagerOGL.cpp:600
6	XUL	mozilla::layers::LayerManagerOGL::EndTransaction	gfx/layers/opengl/LayerManagerOGL.cpp:418
7	XUL	nsDisplayList::PaintForFrame	layout/base/nsDisplayList.cpp:477
8	XUL	nsDisplayList::PaintRoot	layout/base/nsDisplayList.cpp:384
9	XUL	nsLayoutUtils::PaintFrame	layout/base/nsLayoutUtils.cpp:1433
10	XUL	PresShell::Paint	layout/base/nsPresShell.cpp:6095
11	XUL	nsViewManager::RenderViews	view/src/nsViewManager.cpp:447
12	XUL	nsViewManager::Refresh	view/src/nsViewManager.cpp:413
13	XUL	nsViewManager::DispatchEvent	view/src/nsViewManager.cpp:912
14	XUL	HandleEvent	view/src/nsView.cpp:161
15	XUL	nsChildView::DispatchEvent	widget/src/cocoa/nsChildView.mm:1786
16	XUL	nsChildView::DispatchWindowEvent	widget/src/cocoa/nsChildView.mm:1796
17	XUL	-[ChildView drawRect:inContext:]	widget/src/cocoa/nsChildView.mm:2728
18	XUL	-[ChildView drawRect:]	widget/src/cocoa/nsChildView.mm:2632
19	AppKit	-[NSView _drawRect:clip:]	
20	AppKit	-[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:]	
21	AppKit	-[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:]	
22	AppKit	-[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:]	
23	AppKit	-[NSThemeFrame _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:]	
24	AppKit	-[NSView _displayRectIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:]	
25	AppKit	-[NSView displayIfNeeded]	
26	AppKit	-[NSWindow displayIfNeeded]	
27	AppKit	_handleWindowNeedsDisplay	
28	CoreFoundation	__CFRunLoopDoObservers	
29	CoreFoundation	CFRunLoopRunSpecific	
30	CoreFoundation	CFRunLoopRunInMode	
31	HIToolbox	RunCurrentEventLoopInMode	
32	HIToolbox	ReceiveNextEventCommon	
33	HIToolbox	BlockUntilNextEventMatchingListInMode	
34	AppKit	_DPSNextEvent	
35	AppKit	-[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:]	
36	XUL	nsAppShell::ProcessNextNativeEvent	widget/src/cocoa/nsAppShell.mm:652
37	XUL	nsBaseAppShell::OnProcessNextEvent	widget/src/xpwidgets/nsBaseAppShell.cpp:173
38	XUL	nsAppShell::OnProcessNextEvent	widget/src/cocoa/nsAppShell.mm:833
39	XUL	nsThread::ProcessNextEvent	xpcom/threads/nsThread.cpp:590
40	XUL	NS_ProcessPendingEvents_P	 nsThreadUtils.cpp:200
41	XUL	nsBaseAppShell::NativeEventCallback	widget/src/xpwidgets/nsBaseAppShell.cpp:132
42	XUL	nsAppShell::ProcessGeckoEvents	widget/src/cocoa/nsAppShell.mm:399
43	CoreFoundation	CFRunLoopRunSpecific	
44	CoreFoundation	CFRunLoopRunInMode

Comment 9

[Josh Aas]

This needs to block. Bug 609049 has more info. Repros with the test case in that bug, latest stable Silverlight, and Minefield on Mac OS X 10.6 running in 32-bit mode. I'm not sure if it repros on 10.5 or not.

Comment 10

[Benjamin Smedberg]

A hunch says this looks like another manifestation of windowless plugins making DOM modifications while painting. Silverlight is a huge culprit here, but Flash can be made to do something similar.

On 10.5 we're running Flash in-process, and out-of-process on 10.6, so that might account for some differences in behavior.

Comment 12

[Josh Aas]

Before we resolve this bug as fixed we need to be sure to verify that the test cases in the duped bugs are fixed and if they are not we may need to un-dupe.

Comment 13

[Josh Aas]

Here is the crash report from reproducing bug 606374, which is now duped against this bug:

http://crash-stats.mozilla.com/report/index/67a49014-064b-466f-b744-56b452101223

Comment 14

[Nickolay_Ponomarev]

Judging from my experience and from the crash stats over the last month <http://crash-stats.mozilla.com/report/list?product=Firefox&query_search=signature&query_type=exact&query=gfxContext%3A%3ATranslate&date=12%2F28%2F2010%2012%3A56%3A49&range_value=30&range_unit=days&hang_type=any&process_type=any&plugin_field=&plugin_query_type=&plugin_query=&do_query=1&admin=&signature=gfxContext%3A%3ATranslate>, _this_ crash is a 10.5-only regression introduced around 20101217030324 build. I did not yet test previous nightly to confirm.

Comment 15

[Josh Aas]

It's not 10.5 only, it's 32-bit only. It just seems to be specific to 10.5 because 32-bit is the default on 10.5 and 64-bit is default on 10.6. If you run Minefield in 32-bit mode on 10.6 you'll see the crash.

Comment 16

[Nickolay_Ponomarev]

Josh: sorry, I probably shouldn't have changed the summary.

My main point was that this is a recent regression, unlike bug 609049 (as initially filed, but maybe the testcase there can be used to reproduce this bug now). And it's not at all uncommon - I'm getting this every time I try to use fullscreen (sometimes on entering, sometimes on exiting it) in various Flash-based video players, including Youtube's.

I just checked and for me the regression range is between 20101216 and 20101217 builds: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a5413c3c1013&tochange=d1da1005b6d6

Comment 17

[Oleg Romashin (:romaxa)]

I think problem here is aRegion in BeginUpdate is outside of image, and with debug build you should see NS_ERROR("update outside of image");

another possibility is that GetSurfaceForUpdate returning null, and that is possible only if fMapBuffer return null, or CreateOffscreenSurface return null.

Comment 18

[Joe Drew (not getting mail)]

Whoops, I thought Josh had a fix in hand for this.

I wonder if this is related to recent changes to use buffer objects rather than textures?

Comment 19

[Damon Sicore (:damons)]

Can we get an owner for this right now, please?

Comment 20

[Marcia Knous [:marcia]]

Crashes started showing up in crash stats using the 2010122100 build. 41 crashes in this stack in the last week.

Comment 21

[Matt Woodrow (:mattwoodrow)]

Attachment: Possible fix.

I can't reproduce this locally.

I think at the very least we should have our null check for mContext before we start using it.

I've also added better fallback handling for pixel buffers in case thats the issue. 

Debug output would be useful in case Oleg is correct, which would imply a more serious issue in layout.

Comment 22

[Joe Drew (not getting mail)]

Comment on attachment 501867 [details] [diff] [review]
Possible fix.

I don't think we need any of the GL fGetError checking in GetSurfaceForUpdate, because shouldn't glMapBuffer return NULL if there's an error? We're not too concerned about _what_ the error is.

(Plus, we need to call glGetError until it returns no error, to ensure you're not getting a stale error, then glBufferData, then glFinish() to flush the buffer, and then glGetError again to see if you've generated an error. Much easier to just NULL-check glMapBuffer, I think.)

Comment 23

[Matt Woodrow (:mattwoodrow)]

Attachment: Fix v2

Fixed review suggestions

Comment 24

[Matt Woodrow (:mattwoodrow)]

http://hg.mozilla.org/mozilla-central/rev/853736ad6144

Comment 25

[Sunil]

Verified as fixed for Silverlight. What release of Firefox will this fix be made available in?

Comment 26

[Benjamin Smedberg]

This patch should have made it into Firefox 4... do you have evidence to the contrary?

Comment 27

[Sunil]

Sorry. I was updating another bug that was tested with private bits. This bug has been verified with FF4 RTM.