776331 - crash in nsNPAPIPluginInstance::RedrawPlugin on ICS and JB

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Scoobidiver (away)]

It first appeared in 17.0a1/20120721041038. The regression range is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=3a05d298599e&tochange=446b788ab99d
It's likely a regression from bug 687267.

Signature 	nsNPAPIPluginInstance::RedrawPlugin More Reports Search
UUID	1763e40b-f62e-4d83-bfab-4adcc2120722
Date Processed	2012-07-22 06:28:12
Uptime	664
Last Crash	5.0 weeks before submission
Install Age	9.4 hours since version was first installed.
Install Time	2012-07-21 21:03:01
Product	FennecAndroid
Version	17.0a1
Build ID	20120721041038
Release Channel	nightly
OS	Linux
OS Version	0.0.0 Linux 2.6.39.4+ #1 SMP PREEMPT Fri May 11 18:57:07 CST 2012 armv7l
Build Architecture	arm
Build Architecture Info	
Crash Reason	SIGSEGV
Crash Address	0x0
App Notes 	
AdapterDescription: 'NVIDIA Corporation -- NVIDIA Tegra 3 -- OpenGL ES 2.0 14.01002 -- Model: A700, Product: a700_emea_de, Manufacturer: Acer, Hardware: picasso_mf'
EGL? EGL+ GL Context? GL Context+ GL Layers? GL Layers+ 
Acer A700
acer/a700_emea_de/picasso_mf:4.0.4/IMM76D/1336732816:user/release-keys
EMCheckCompatibility	True
Adapter Vendor ID	NVIDIA Corporation
Adapter Device ID	NVIDIA Tegra 3

Frame 	Module 	Signature 	Source
0 	libxul.so 	nsNPAPIPluginInstance::RedrawPlugin 	dom/plugins/base/nsNPAPIPluginInstance.cpp:802
1 	libxul.so 	nsRunnableMethodImpl<void , true>::Run 	nsThreadUtils.h:349
2 	libxul.so 	nsSurfaceTexture::NotifyFrameAvailable 	gfx/thebes/nsSurfaceTexture.cpp:258
3 	libxul.so 	Java_org_mozilla_gecko_GeckoAppShell_onSurfaceTextureFrameAvailable 	widget/android/AndroidJNI.cpp:1036
4 	libmozglue.so 	Java_org_mozilla_gecko_GeckoAppShell_onSurfaceTextureFrameAvailable 	mozglue/android/APKOpen.cpp:326
5 	libdvm.so 	libdvm.so@0x1ec32 	
6 	dalvik-heap (deleted) 	dalvik-heap @0x9533ce 	
7 	libdvm.so 	libdvm.so@0x58eed 	
8 	data@[e-mail]@classes.dex 	data@[e-mail]@classes.dex@0x136a24 	
9 	libmozglue.so 	Java_org_mozilla_gecko_GeckoAppShell_getNextMessageFromQueue 	mozglue/android/APKOpen.cpp:325
10 		@0x5ebad5fe 	
11 	libxul.so 	nsCOMPtr_base::assign_with_AddRef 	obj-firefox/xpcom/build/nsCOMPtr.cpp:49
12 	libxul.so 	nsEventListenerManager::HandleEventInternal 	nsCOMPtr.h:614 

More reports at:
https://crash-stats.mozilla.com/report/list?signature=nsNPAPIPluginInstance%3A%3ARedrawPlugin

Comment 1

[James Willcox (:snorp) ([e-mail]) (he/him)]

I can't seem to reproduce, adding qawanted for STR. I do have a plausible fix that I will post, however.

Comment 2

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Avoid crash when trying to redraw a destroyed plugin

Comment 3

[Josh Aas]

Comment on attachment 645488 [details] [diff] [review]
Avoid crash when trying to redraw a destroyed plugin

If this check wasn't required before then it seems like it shouldn't be required now. Can you explain why it is required now? If not then I suspect it's just hiding a much worse problem and I'd rather not do it, especially without an ifdef making it android-only. These things can seem like harmless safety checks that are hard to argue with but they end up covering up deeper issues.

Comment 4

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to Josh Aas (Mozilla Corporation) from comment #3)
> Comment on attachment 645488 [details] [diff] [review]
> Avoid crash when trying to redraw a destroyed plugin
> 
> If this check wasn't required before then it seems like it shouldn't be
> required now. Can you explain why it is required now? If not then I suspect
> it's just hiding a much worse problem and I'd rather not do it, especially
> without an ifdef making it android-only. These things can seem like harmless
> safety checks that are hard to argue with but they end up covering up deeper
> issues.

Indeed, better patch coming.

Comment 5

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Avoid crash when trying to redraw a destroyed plugin on Android

Comment 6

[Josh Aas]

Comment on attachment 645756 [details] [diff] [review]
Avoid crash when trying to redraw a destroyed plugin on Android

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

Looks good, thanks!

Comment 7

[Scoobidiver (away)]

It's #1 top crasher in today's Nightly and Aurora.

Comment 8

[James Willcox (:snorp) ([e-mail]) (he/him)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/873bd2f652b7

Comment 9

[James Willcox (:snorp) ([e-mail]) (he/him)]

I'll ask for Aurora and Beta approval as soon as we're sure this patch fixes it on Nightly.

Comment 10

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/mozilla-central/rev/873bd2f652b7

Comment 11

[Robert Kaiser]

This signature completely stopped on 17.0a1 after the 2012072605 build ID, I think the patch is ready for uplift to Aurora and Beta, where this is the topcrash right now.

Comment 12

[Alex Keybl [:akeybl]]

(In reply to Robert Kaiser (:[e-mail]) from comment #11)
> This signature completely stopped on 17.0a1 after the 2012072605 build ID, I
> think the patch is ready for uplift to Aurora and Beta, where this is the
> topcrash right now.

That's fantastic news - yes, let's move forward with the uplift (preferably before tomorrow's beta build).

Comment 13

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Comment on attachment 645756 [details] [diff] [review]
Avoid crash when trying to redraw a destroyed plugin on Android

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: 
Testing completed (on m-c, etc.): 
Risk to taking this patch (and alternatives if risky): 
String or UUID changes made by this patch:

Comment 14

[Alex Keybl [:akeybl]]

(In reply to Naoki Hirata :nhirata from comment #13)
> Bug caused by (feature/regressing bug #): 
> Risk to taking this patch (and alternatives if risky): 

Before approving, we'll still need these filled out by snorp.

Comment 15

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to Naoki Hirata :nhirata from comment #13)
> Comment on attachment 645756 [details] [diff] [review]
> Avoid crash when trying to redraw a destroyed plugin on Android
> 
> [Approval Request Comment]
Fixes top crash, low risk.

Comment 16

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 645756 [details] [diff] [review]
Avoid crash when trying to redraw a destroyed plugin on Android

Please land to branches, before tomorrow morning PT if possible, so we can get the most beta user data on the crashes.

Comment 17

[James Willcox (:snorp) ([e-mail]) (he/him)]

https://hg.mozilla.org/releases/mozilla-beta/rev/d37e5476fb73

Comment 18

[James Willcox (:snorp) ([e-mail]) (he/him)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/4853c37d2229

Comment 19

[Alex Keybl [:akeybl]]

Comment on attachment 645756 [details] [diff] [review]
Avoid crash when trying to redraw a destroyed plugin on Android

[Triage Comment]
Based upon the latest crash volume, we've decided to take this fix in a mobile-only 14.0.2. Please land on mozilla-release as soon as possible.

Comment 20

[Alex Keybl [:akeybl]]

Comment on attachment 645756 [details] [diff] [review]
Avoid crash when trying to redraw a destroyed plugin on Android

sorry, wrong JB bug.