Closed Bug 902339 Opened 7 years ago Closed 7 years ago

crash in mozilla::layers::LayerManagerComposite::AddMaskEffect

Categories

(Core :: Graphics: Layers, defect)

25 Branch
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla26
Tracking Status
firefox24 --- unaffected
firefox25 + verified
firefox26 + verified

People

(Reporter: scoobidiver, Assigned: mattwoodrow)

References

Details

(4 keywords, Whiteboard: [native-crash])

Crash Data

Attachments

(1 file)

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
More reports also at:
https://crash-stats.mozilla.com/report/list?product=FennecAndroid&signature=mozilla%3A%3Alayers%3A%3AImageLayerComposite%3A%3AGetCompositableHost%28%29
Crash Signature: [@ _ZThn488_N7mozilla6layers19ImageLayerComposite19GetCompositableHostEv] → [@ _ZThn488_N7mozilla6layers19ImageLayerComposite19GetCompositableHostEv] [@ mozilla::layers::ImageLayerComposite::GetCompositableHost() ]
OS: Mac OS X → All
Hardware: x86_64 → All
Whiteboard: [native-crash]
Summary: crash in mozilla::layers::LayerManagerComposite::AddMaskEffect @ _ZThn488_N7mozilla6layers19ImageLayerComposite19GetCompositableHostEv → crash in mozilla::layers::LayerManagerComposite::AddMaskEffect
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
Keywords: reproducible
It's #1 top crasher in Fennec 26.0a1 and in Firefox 26.0a1 on Mac OS X.
tracking-fennec: --- → ?
Keywords: topcrash
Attached patch Check mImageHostSplinter Review
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.
Attachment #788727 - Flags: review?(ncameron)
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
Attachment #788727 - Flags: review?(ncameron) → 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.

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.
(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.
Assignee: nobody → matt.woodrow
Blocks: 874721
Version: 26 Branch → 25 Branch
https://hg.mozilla.org/mozilla-central/rev/85802e27349c
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
Ready for an uplift nomination, to resolve this top crash. Basically just need a risk evaluation.
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.
Attachment #788727 - Flags: approval-mozilla-aurora?
Comment on attachment 788727 [details] [diff] [review]
Check mImageHost

FF25 topcrash regression, approving for Aurora 25.
Attachment #788727 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
tracking-fennec: ? → ---
You need to log in before you can comment on or make changes to this bug.