902339 - crash in mozilla::layers::LayerManagerComposite::AddMaskEffect

Status: VERIFIED FIXED

(Core :: Graphics: Layers, defect)

Description

[Scoobidiver (away)]

It first showed up in 26.0a1/20130806 and is #1 top crasher on Mac OS X. The regression range is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=482b9d04974a&tochange=a0dd80f800e2
I think it's a regression from bug 899667.

Signature 	_ZThn488_N7mozilla6layers19ImageLayerComposite19GetCompositableHostEv More Reports Search
UUID 	e26f92d6-db57-4c41-aed7-37dc12130807
Date Processed	2013-08-07 00:09:01.334306
Uptime	7018
Last Crash	7266 seconds before submission
Install Age 	13577 since version was first installed.
Install Time 	2013-08-06 20:22:22
Product 	Firefox
Version 	26.0a1
Build ID 	20130806104538
Release Channel 	nightly
OS 	Mac OS X
OS Version 	10.7.5 11G63
Build Architecture 	amd64
Build Architecture Info 	family 6 model 15 stepping 11 | 2
Crash Reason 	EXC_BAD_ACCESS / KERN_INVALID_ADDRESS
Crash Address 	0x30
User Comments 	zooming in the boobmap!
App Notes 	
AdapterVendorID: 0x8086, AdapterDeviceID: 0x2a02GL Layers! GL Context? GL Context+ GL Layers+ WebGL? WebGL+ 

Frame 	Module 	Signature 	Source
0 	XUL 	_ZThn488_N7mozilla6layers19ImageLayerComposite19GetCompositableHostEv 	gfx/layers/composite/CompositableHost.h
1 	XUL 	mozilla::layers::LayerManagerComposite::AddMaskEffect(mozilla::layers::Layer*, mozilla::layers::EffectChain&, bool) 	gfx/layers/composite/LayerManagerComposite.cpp
2 	XUL 	mozilla::layers::CanvasLayerComposite::RenderLayer(nsIntPoint const&, nsIntRect const&) 	gfx/layers/composite/CanvasLayerComposite.cpp
3 	XUL 	void mozilla::layers::ContainerRender<mozilla::layers::ContainerLayerComposite>(mozilla::layers::ContainerLayerComposite*, nsIntPoint const&, mozilla::layers::LayerManagerComposite*, nsIntRect const&) 	gfx/layers/composite/ContainerLayerComposite.cpp
4 	XUL 	void mozilla::layers::ContainerRender<mozilla::layers::ContainerLayerComposite>(mozilla::layers::ContainerLayerComposite*, nsIntPoint const&, mozilla::layers::LayerManagerComposite*, nsIntRect const&) 	gfx/layers/composite/ContainerLayerComposite.cpp
5 	XUL 	mozilla::layers::LayerManagerComposite::Render() 	gfx/layers/composite/LayerManagerComposite.cpp
6 	XUL 	mozilla::layers::LayerManagerComposite::EndTransaction(void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*), void*, mozilla::layers::LayerManager::EndTransactionFlags) 	gfx/layers/composite/LayerManagerComposite.cpp
7 	XUL 	mozilla::layers::LayerManagerComposite::EndEmptyTransaction(mozilla::layers::LayerManager::EndTransactionFlags) 	gfx/layers/composite/LayerManagerComposite.cpp
8 	XUL 	mozilla::layers::CompositorParent::Composite() 	gfx/layers/ipc/CompositorParent.cpp
9 	XUL 	MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask const&) 	ipc/chromium/src/base/message_loop.cc
10 	XUL 	MessageLoop::DoWork() 	ipc/chromium/src/base/message_loop.cc
11 	XUL 	base::MessagePumpDefault::Run(base::MessagePump::Delegate*) 	ipc/chromium/src/base/message_pump_default.cc
12 	XUL 	MessageLoop::Run() 	ipc/chromium/src/base/message_loop.cc
13 	XUL 	base::Thread::ThreadMain() 	ipc/chromium/src/base/thread.cc
14 	XUL 	ThreadFunc 	/builds/slave/fx-team-osx64-0000000000000000/build/obj-firefox/x86_64/ipc/chromium/../../../../ipc/chromium/src/base/platform_thread_posix.cc
15 	libsystem_c.dylib 	libsystem_c.dylib@0x4e8bf 	
16 	libsystem_c.dylib 	libsystem_c.dylib@0x51b75 	
17 	XUL 	XUL@0x171b260

More reports at:
https://crash-stats.mozilla.com/report/list?product=Firefox&signature=_ZThn488_N7mozilla6layers19ImageLayerComposite19GetCompositableHostEv

Comment 1

[Scoobidiver (away)]

More reports also at:
https://crash-stats.mozilla.com/report/list?product=FennecAndroid&signature=mozilla%3A%3Alayers%3A%3AImageLayerComposite%3A%3AGetCompositableHost%28%29

Comment 2

[Aaron Train [:aaronmt]]

Reproducible on Firefox for Android using a WebRTC TokBox demo on Nightly

* On desktop, visit http://tokbox.com/opentok/webrtc/demo, start a WebRTC session (share media) and copy the shared URL

* On mobile, visit the shared URL and start a WebRTC session (sharing media)

* On desktop, close the active WebRTC session tab

* On mobile, see the crash

Comment 3

[Scoobidiver (away)]

It's #1 top crasher in Fennec 26.0a1 and in Firefox 26.0a1 on Mac OS X.

Comment 4

[Matt Woodrow (:mattwoodrow)]

Attachment: Check mImageHost

This is a regression from bug 874721.

Looks like mImageHost in our ImageLayerComposite (the mask layer) is null.

I seem to remember a comment somewhere about Nick investigating this issue and deciding it wasn't a problem. Can't find it now though.

I haven't been able to reproduce this though, so can't do much more.

Comment 5

[Nick Cameron [:nrc]]

Comment on attachment 788727 [details] [diff] [review]
Check mImageHost

Review of attachment 788727 [details] [diff] [review]:
-----------------------------------------------------------------

If by 'checked this and thought it wouldn't be a problem' you mean 'made a school boy error by not checking this and introduced a bug', then yes, I did.

This is the correct fix.

::: gfx/layers/composite/ImageLayerComposite.cpp
@@ +134,5 @@
>  
>  CompositableHost*
>  ImageLayerComposite::GetCompositableHost()
>  {
> +  if (mImageHost && mImageHost->IsAttached())

may as well stick in some {} for good measure

Comment 6

[Matt Woodrow (:mattwoodrow)]

> If by 'checked this and thought it wouldn't be a problem' you mean 'made a
> school boy error by not checking this and introduced a bug', then yes, I did.
> 
> This is the correct fix.

I meant that you investigated if mImageHost being null was a valid state to be in, and we're not just wallpapering over a real bug.

Comment 7

[Nick Cameron [:nrc]]

(In reply to Matt Woodrow (:mattwoodrow) from comment #6)
> > If by 'checked this and thought it wouldn't be a problem' you mean 'made a
> > school boy error by not checking this and introduced a bug', then yes, I did.
> > 
> > This is the correct fix.
> 
> I meant that you investigated if mImageHost being null was a valid state to
> be in, and we're not just wallpapering over a real bug.

Yeah, I think it is OK for the image host to be null, we should definitely be checking it here.

Comment 8

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/85802e27349c

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/85802e27349c

Comment 10

[Alex Keybl [:akeybl]]

Ready for an uplift nomination, to resolve this top crash. Basically just need a risk evaluation.

Comment 11

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 788727 [details] [diff] [review]
Check mImageHost

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 874721
User impact if declined: Crashes.
Testing completed (on m-c, etc.): Been on m-c for over a week.
Risk to taking this patch (and alternatives if risky): Incredibly low risk, just adds a null check.
String or IDL/UUID changes made by this patch: None.

Comment 12

[Alex Keybl [:akeybl]]

Comment on attachment 788727 [details] [diff] [review]
Check mImageHost

FF25 topcrash regression, approving for Aurora 25.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/bd5480525311

Comment 14

[Bogdan Maris, Desktop Test Engineering]

No more crashes in the last 3 months on Aurora 26.0a2 or Firefox 25 beta with the signatures:
[@ _ZThn488_N7mozilla6layers19ImageLayerComposite19GetCompositableHostEv]
[@ mozilla::layers::ImageLayerComposite::GetCompositableHost() ]

https://crash-stats.mozilla.com/query/?product=Firefox&version=Firefox%3A26.0a2&platform=win&platform=mac&platform=lin&range_value=3&range_unit=weeks&date=10%2F11%2F2013+12%3A00%3A00&query_search=signature&query_type=contains&query=%40+_ZThn488_N7mozilla6layers19ImageLayerComposite19GetCompositableHostEv&reason=&release_channels=&build_id=&process_type=any&hang_type=any

https://crash-stats.mozilla.com/query/?product=Firefox&version=Firefox%3A26.0a2&platform=win&platform=mac&platform=lin&range_value=3&range_unit=weeks&date=10%2F11%2F2013+13%3A00%3A00&query_search=signature&query_type=contains&query=mozilla%3A%3Alayers%3A%3AImageLayerComposite%3A%3AGetCompositableHost%28%29&reason=&release_channels=&build_id=&process_type=any&hang_type=any