Last Comment Bug 779741 - java.lang.NullPointerException: at org.mozilla.gecko.GeckoApp$<n>.run(GeckoApp.java)
: java.lang.NullPointerException: at org.mozilla.gecko.GeckoApp$<n>.run(GeckoAp...
Status: RESOLVED FIXED
[native-crash][startupcrash]
: crash, regression
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: 17 Branch
: ARM Android
: -- critical (vote)
: Firefox 18
Assigned To: :Margaret Leibovic
:
: Sebastian Kaspari (:sebastian)
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-02 01:38 PDT by Scoobidiver (away)
Modified: 2012-09-13 04:14 PDT (History)
4 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed


Attachments
move character encoding menu logic to BrowserApp (9.89 KB, patch)
2012-08-27 14:42 PDT, :Margaret Leibovic
sriram.mozilla: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review
(Part 2) enable "Settings" menuitem in BrowserApp, not GeckoApp (3.04 KB, patch)
2012-08-29 12:02 PDT, :Margaret Leibovic
wjohnston2000: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Scoobidiver (away) 2012-08-02 01:38:31 PDT
There's one crash in 17.0a1/20120801: bp-f938e0f1-2984-4eca-9072-deb3a2120801.

java.lang.NullPointerException
	at org.mozilla.gecko.GeckoApp$8.run(GeckoApp.java:1018)
	at android.os.Handler.handleCallback(Handler.java:615)
	at android.os.Handler.dispatchMessage(Handler.java:92)
	at android.os.Looper.loop(Looper.java:137)
	at android.app.ActivityThread.main(ActivityThread.java:4896)
	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:788)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:555)
	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.GeckoApp%248.run%28GeckoApp.java%29
Comment 1 Kartikaya Gupta (email:kats@mozilla.com) 2012-08-02 07:51:33 PDT
Happened at this line: http://hg.mozilla.org/mozilla-central/file/582d4c67b3a7/mobile/android/base/GeckoApp.java#l1018

Presumably the char encoding menu item wasn't found?
Comment 2 :Margaret Leibovic 2012-08-09 14:14:36 PDT
Is it sufficient to check that menu isn't null above this line? It looks like we're setting mMenu in onCreateOptionsMenu, then inflating the layout inside it afterwards, so maybe this Runnable is running in between those two things happening.

http://hg.mozilla.org/mozilla-central/file/582d4c67b3a7/mobile/android/base/GeckoApp.java#l448
Comment 3 Sriram Ramasubramanian [:sriram] 2012-08-09 16:37:40 PDT
This could happen for webapps which does not have the "char encoding" menu item.
Comment 4 Scoobidiver (away) 2012-08-27 11:54:45 PDT
There's spike in crashes starting from 17.0a1/20120824101755. The regression range for the spike is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5650196a8c7d&tochange=f2146a6c104e
Comment 6 :Margaret Leibovic 2012-08-27 14:42:54 PDT
Created attachment 655776 [details] [diff] [review]
move character encoding menu logic to BrowserApp

There may have been a recent spike because of the increase in WebApps testing.

Unfortunately, I wasn't able to reproduce the crash, so this is a speculative fix, but I think it's something we should do anyway.
Comment 7 Sriram Ramasubramanian [:sriram] 2012-08-27 14:44:47 PDT
Comment on attachment 655776 [details] [diff] [review]
move character encoding menu logic to BrowserApp

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

Looks good to me as per https://bugzilla.mozilla.org/show_bug.cgi?id=779741#c3
Comment 9 Ed Morley [:emorley] 2012-08-28 06:12:14 PDT
https://hg.mozilla.org/mozilla-central/rev/4a09ef58cba1
Comment 10 Scoobidiver (away) 2012-08-29 07:54:22 PDT
It's not fixed. See bp-fbdf67bc-943b-4ae5-82aa-4ca312120829.
Comment 11 :Margaret Leibovic 2012-08-29 09:14:02 PDT
(In reply to Scoobidiver from comment #10)
> It's not fixed. See bp-fbdf67bc-943b-4ae5-82aa-4ca312120829.

This is a call to get a different menuitem:
http://hg.mozilla.org/mozilla-central/file/790fb17b1fe3/mobile/android/base/GeckoApp.java#l900

But the same idea applies. We should take a look through GeckoApp and move BrowserApp-depdendent menuitem logic over to BrowserApp.
Comment 12 :Margaret Leibovic 2012-08-29 12:02:04 PDT
Created attachment 656546 [details] [diff] [review]
(Part 2) enable "Settings" menuitem in BrowserApp, not GeckoApp

This is the only other menu.findItem() call I see in GeckoApp, so I think we should be good after this.

I didn't realize it at first, but the events we register for in GeckoApp first get handled by BrowserApp's handleMessage, so I didn't need to change any (un)registerEventListener calls for this to work.
Comment 14 Ryan VanderMeulen [:RyanVM] 2012-08-30 19:02:12 PDT
https://hg.mozilla.org/mozilla-central/rev/bad853c6b1e5
Comment 15 Scoobidiver (away) 2012-09-12 03:08:44 PDT
I think it should land in Aurora.
Comment 16 :Margaret Leibovic 2012-09-12 03:18:45 PDT
Comment on attachment 655776 [details] [diff] [review]
move character encoding menu logic to BrowserApp

[Approval Request Comment]
Bug caused by (feature/regressing bug #): n/a
User impact if declined: crashes
Testing completed (on m-c, etc.): landed on m-c 8/28 
Risk to taking this patch (and alternatives if risky): low-risk, moving BrowserApp-specific logic out of GeckoApp
String or UUID changes made by this patch: n/a
Comment 17 :Margaret Leibovic 2012-09-12 03:19:15 PDT
Comment on attachment 656546 [details] [diff] [review]
(Part 2) enable "Settings" menuitem in BrowserApp, not GeckoApp

[Approval Request Comment]
Bug caused by (feature/regressing bug #): n/a
User impact if declined: crashes
Testing completed (on m-c, etc.): landed on m-c 8/30 
Risk to taking this patch (and alternatives if risky): low-risk, moving BrowserApp-specific logic out of GeckoApp
String or UUID changes made by this patch: n/a

Note You need to log in before you can comment on or make changes to this bug.