Closed
Bug 816457
Opened 12 years ago
Closed 12 years ago
java.lang.NullPointerException: at org.mozilla.gecko.LightweightTheme.getCroppedBitmap(LightweightTheme.java)
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox19+ wontfix, firefox20+ fixed, firefox21+ fixed, firefox22 fixed, fennec20+)
People
(Reporter: scoobidiver, Assigned: sriram)
References
Details
(Keywords: crash, regression, Whiteboard: [native-crash])
Crash Data
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
mfinkle
:
review+
bajaj
:
approval-mozilla-aurora+
bajaj
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
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
Reporter | ||
Updated•12 years ago
|
Reporter | ||
Comment 2•12 years ago
|
||
Asking for tracking because themes are a new feature in 19.0.
Updated•12 years ago
|
Assignee | ||
Comment 3•12 years ago
|
||
1. Is there an STR for this?
2. Could we find the persona that's causing this?
Comment 4•12 years ago
|
||
(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.
Updated•12 years ago
|
QA Contact: kbrosnan
Updated•12 years ago
|
Keywords: steps-wanted
Assignee | ||
Comment 5•12 years ago
|
||
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.
Comment 6•12 years ago
|
||
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
Comment 7•12 years ago
|
||
(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.
Updated•12 years ago
|
Comment 8•12 years ago
|
||
(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?
Comment 9•12 years ago
|
||
(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
Flags: needinfo?(kairo)
Comment 10•12 years ago
|
||
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.
Comment 11•12 years ago
|
||
Sriram - We need this patch ASAP so we can get it in beta
tracking-fennec: ? → 20+
Flags: needinfo?(sriram)
Assignee | ||
Comment 12•12 years ago
|
||
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
Attachment #716765 -
Flags: review?(mark.finkle)
Flags: needinfo?(sriram)
Reporter | ||
Updated•12 years ago
|
status-firefox22:
--- → affected
Comment 13•12 years ago
|
||
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
Assignee | ||
Comment 14•12 years ago
|
||
"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. :)
Attachment #718093 -
Flags: review?(mark.finkle)
Assignee | ||
Comment 15•12 years ago
|
||
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.
Attachment #716765 -
Attachment is obsolete: true
Attachment #718093 -
Attachment is obsolete: true
Attachment #716765 -
Flags: review?(mark.finkle)
Attachment #718093 -
Flags: review?(mark.finkle)
Attachment #718559 -
Flags: review?(mark.finkle)
Comment 16•12 years ago
|
||
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.
Attachment #718559 -
Flags: review?(mark.finkle) → review+
Assignee | ||
Comment 17•12 years ago
|
||
Assignee | ||
Comment 18•12 years ago
|
||
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.
Attachment #718559 -
Flags: approval-mozilla-beta?
Attachment #718559 -
Flags: approval-mozilla-aurora?
Comment 19•12 years ago
|
||
(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.
Assignee | ||
Comment 20•12 years ago
|
||
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 21•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 22
Reporter | ||
Updated•12 years ago
|
Comment 22•12 years ago
|
||
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.
Attachment #718559 -
Flags: approval-mozilla-beta?
Attachment #718559 -
Flags: approval-mozilla-beta+
Attachment #718559 -
Flags: approval-mozilla-aurora?
Attachment #718559 -
Flags: approval-mozilla-aurora+
Comment 23•12 years ago
|
||
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
Comment 24•2 years ago
|
||
Removing steps-wanted
keyword because this bug has been resolved.
Keywords: steps-wanted
You need to log in
before you can comment on or make changes to this bug.
Description
•