There's one crash in 20.0a1/20121128: bp-17d7d0f1-6689-41d9-9ea9-430322121129. java.lang.NullPointerException at org.mozilla.gecko.LightweightTheme.getCroppedBitmap(LightweightTheme.java:179) at org.mozilla.gecko.LightweightTheme.getDrawableWithAlpha(LightweightTheme.java:269) at org.mozilla.gecko.AboutHomeContent.onLightweightThemeChanged(AboutHomeContent.java:687) at org.mozilla.gecko.LightweightTheme$2.run(LightweightTheme.java:134) at android.os.Handler.handleCallback(Handler.java:605) at android.os.Handler.dispatchMessage(Handler.java:92) at android.os.Looper.loop(Looper.java:137) at android.app.ActivityThread.main(ActivityThread.java:4514) at java.lang.reflect.Method.invokeNative(Native Method) at java.lang.reflect.Method.invoke(Method.java:511) at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:980) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:747) at dalvik.system.NativeStart.main(Native Method) More reports at: https://crash-stats.mozilla.com/report/list?signature=java.lang.NullPointerException%3A+at+org.mozilla.gecko.LightweightTheme.getCroppedBitmap%28LightweightTheme.java%29

/summon Sriram

Asking for tracking because themes are a new feature in 19.0.

1. Is there an STR for this? 2. Could we find the persona that's causing this?

(In reply to Sriram Ramasubramanian [:sriram] from comment #3) > 1. Is there an STR for this? Adding qawanted here. Kairo/QA is there any way we can check if a specific persona/theme is causing the crash from crash-reports ?Also device co-relations will be helpful here . > 2. Could we find the persona that's causing this? Discussed offline with Sriram and persona's could be causing this combining the fact a)Recent changes taken b)Crash Signature c)on checking URL's in the crash report most of them are theme related.

Most of the crashes happen at https://hg.mozilla.org/releases/mozilla-beta/file/4d4116685156/mobile/android/base/LightweightTheme.java#l207 But there's a safety check on "mBitmap" at the start of the function. Which means, mBitmap is reset this function is being executed. getCroppedBitmap() is called from UI thread. resetLightweightTheme() happens in the background thread. One way to fix this is to lock on mBitmap. But I'm not sure if that would cause any performance regressions. Probably test case would be: 1. Tap on a persona in a.m.o. Let it be in preview mode. 2. Try closing/opening the sidebar, when the preview timer is about to stop. This may or may not crash. But, like I said, solution is to block on mBitmap as it is touched by 2 threads.

I've hit this twice when clicking around on addons.mozilla.org Android themes. I don't have any str other than to click around on the site a lot. I'll spend a little while longer on it trying to get str. bp-0634e3c9-8539-4260-ae8a-9df3a2130131 bp-9e8b17d9-c67f-4d6c-bb43-6047a2130131

(In reply to Kevin Brosnan [:kbrosnan] from comment #6) > I've hit this twice when clicking around on addons.mozilla.org Android > themes. I don't have any str other than to click around on the site a lot. > I'll spend a little while longer on it trying to get str. > > bp-0634e3c9-8539-4260-ae8a-9df3a2130131 > bp-9e8b17d9-c67f-4d6c-bb43-6047a2130131 Happy to hear this is only intermittent and recoverable - if this ends up going unfixed for FF19, it won't be the end of the world.

(In reply to Sriram Ramasubramanian [:sriram] from comment #5) > This may or may not crash. But, like I said, solution is to block on mBitmap > as it is touched by 2 threads. Can you prepare a patch for FF20 in that case?

(In reply to bhavana bajaj [:bajaj] from comment #4) > Adding qawanted here. Kairo/QA is there any way we can check if a specific > persona/theme is causing the crash from crash-reports ?Also device > co-relations will be helpful here . I don't think we report those lightweight themes in crash reports. And the devices for the last five days look pretty spread: Samsung GT-P6800 1 Sony Ericsson LT26i 1 Samsung GT-P6200L 1 Samsung GT-P3100 1 HTC Glacier 1 Alps Micromax A110 1 Sony Ericsson LT26i 2 LGE Nexus 4 1 HUAWEI M886 1 HTC Wildfire S A510e 1 HTC Desire V 1 HTC EVO 1 Archos 101G9 1 Samsung GT-N7100 2 Samsung GT-P7510 1 Samsung GT-P5100 1 Samsung SGH-I317 1 Unknown A777 1 Unknown TAB9008GB 1 Unknown MD-003 1 Samsung GT-I9300 1 Asus Nexus 7 1 Alps Micromax A110 1 Kobo Arc 1 Kyocera C5155 1 MID ELF-II 1 Ainol Novo 10 Hero QuadCore 1 TCT ADR3010 1 Unknown IMO ZONE 1 Xiaomi MI-ONE Plus 1 Sony Ericsson LT26i 1 Samsung GT-S7562 1 Motorola MB860 1 Samsung GT-P6210 1 LGE LG-P700 1 Samsung GT-P3100 2 Asus Nexus 7 2 Samsung SGH-T989D 1 Unknown AN7HG3 1 Unknown T9002 1 Samsung GT-N7000 1 Asus PadFone 2 1 Hisense AD683G 1 Motorola MB525 1

When I did my previous investigation I looked at the logcat data and URLs. From what I recall it the amo themes root page was the main page this was happening on.

Sriram - We need this patch ASAP so we can get it in beta

Based on bnicholson's idea, instead of making "synchronized" methods, I made BrowserApp listens for the message from Gecko. BrowserApp knows the UI thread, and the messages from Gecko is on background thread (LWTheme doesn't know the UI thread). Hence BrowserApp will download the image in background thread and inform the LWTheme in UI thread. This ensures that mBitmap is touched only in UI thread. (All methods in LWTheme will run on UI thread -- not a regression from the past). All good. :D

Comment on attachment 716765 [details] [diff] [review] Patch I am a little sad that we are moving handlers and code back into BrowserApp. We like moving handlers out of BrowserApp! :) Could we use this approach: http://stackoverflow.com/questions/6369287/accessing-ui-thread-handler-from-a-service

"Owner takes care of its members" patch: LightweightTheme is made singleton by making the Application hold it. So, I moved the "handleMessage" to Application -- where it should have been! :O Application initialize happens on onCreate() of BrowserApp, where it registers for LWTheme updates. The main thread for the application is created when an application is launched, and is responsible for creating and running activities. Hence Looper.getMainLooper() is available at the initialize(). (I hate using GeckoAppShell.getMainHandler() -- which is basically something Application already knows!) And that's how we keep BrowserApp clean. :)

Back to where it all started. LWTheme listens for events and updates the bitmap. The downloading the image happens in background thread. The setting of mBitmap and resetting happens in UI thread -- the same thread used by views to update their background.

Comment on attachment 718559 [details] [diff] [review] Patch v3: LWTheme as owner Thanks. Ask for approval to uplift as soon as we get it landed.

Comment on attachment 718559 [details] [diff] [review] Patch v3: LWTheme as owner [Approval Request Comment] Bug caused by (feature/regressing bug #): Personas User impact if declined: Occasional crash when installing personas. Testing completed (on m-c, etc.): Landed in m-i on 02/26 Risk to taking this patch (and alternatives if risky): Very less. This constrains the Bitmap to be accessed from the same thread -- which could solve the crash. String or UUID changes made by this patch: None.

(In reply to Sriram Ramasubramanian [:sriram] from comment #17) > https://hg.mozilla.org/integration/mozilla-inbound/rev/0046a607a915 https://hg.mozilla.org/mozilla-central/rev/0046a607a915 This landed with the wrong bug number, can you backout and reland in one push (with DONTBUILD in the topmost commit message) please.

With the minimal knowledge I had, I backed out and pushed the new one! :O https://hg.mozilla.org/integration/mozilla-inbound/rev/7addb08b2e93 https://hg.mozilla.org/integration/mozilla-inbound/rev/ecc7941f57ac

Comment on attachment 718559 [details] [diff] [review] Patch v3: LWTheme as owner Low risk patch for a recent feature regression leading to crashes.Approving for uplift.

https://hg.mozilla.org/releases/mozilla-aurora/rev/3d101fe68408 https://hg.mozilla.org/releases/mozilla-beta/rev/ed30d4f43da0