Closed Bug 793136 Opened 7 years ago Closed 7 years ago

java.lang.NullPointerException: at org.mozilla.gecko.BrowserApp.onPrepareOptionsMenu(BrowserApp.java)

Categories

(Firefox for Android :: General, defect, P1, critical)

18 Branch
ARM
Android
defect

Tracking

()

VERIFIED FIXED
Firefox 18
Tracking Status
firefox18 + fixed

People

(Reporter: scoobidiver, Assigned: sriram)

References

Details

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

Crash Data

Attachments

(2 files, 1 obsolete file)

There's one crash in 18.0a1/20120921: bp-4c42839b-8fa1-490c-b7e2-97f082120921.

java.lang.NullPointerException
	at org.mozilla.gecko.BrowserApp.onPrepareOptionsMenu(BrowserApp.java:785)
	at org.mozilla.gecko.GeckoApp.invalidateOptionsMenu(GeckoApp.java:451)
	at org.mozilla.gecko.BrowserApp.onTabChanged(BrowserApp.java:82)
	at org.mozilla.gecko.Tabs.notifyListeners(Tabs.java:375)
	at org.mozilla.gecko.Tabs.notifyListeners(Tabs.java:364)
	at org.mozilla.gecko.Tab$5.run(Tab.java:371)
	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:4745)
	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:786)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:553)
	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.BrowserApp.onPrepareOptionsMenu%28BrowserApp.java%29
It has been hit by 11 users in less than 2 hours. The regression range is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1e56d3016820&tochange=48c4938eaf57
Keywords: regression, topcrash
Whiteboard: [native-crash] → [native-crash][startupcrash]
Version: Trunk → Firefox 18
It's #1 top crasher in today's build.
bug 785994?
Assignee: nobody → sriram
Sriram - Please take at this one. It's happening a lot.
Priority: -- → P1
Is there any STR for this? I was running a build with submenu over the weekend and browsed a lot. I never faced this crash.
Keywords: steps-wanted
Sriram, all the crash reports point to BrowserApp.java:785:

https://hg.mozilla.org/mozilla-central/file/e327e66a027d/mobile/android/base/BrowserApp.java#l785

Can tab.getURL() or tab.getContentType() return null? When you are comparing a someString.equals("Some string literal"), a safer style is to reverse the check like "Some string literal".equals(someString) because this style won't crash if someString is null.
Attached patch Patch (obsolete) — Splinter Review
This is a precautionary patch (both at Java and Gecko side). This may work.
Attachment #664152 - Flags: review?(mark.finkle)
Attached patch PatchSplinter Review
This is an updated patch using ternary operator (I failed to refresh my last patch).
Attachment #664152 - Attachment is obsolete: true
Attachment #664152 - Flags: review?(mark.finkle)
Attachment #664154 - Flags: review?(mark.finkle)
Attachment #664154 - Flags: review?(mark.finkle) → review+
https://hg.mozilla.org/mozilla-central/rev/8ee445d27224
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 18
I am able to reproduce this crash - STR:

1. install Phony add-on from addons.mozilla.org
2. go to Add-ons Manager > Phony and disable it
3. tap on back button et voila

Build: Firefox 18.0a1 (2012-09-25)
Device: Samnsun Galaxu Nexus
OS: Android 4.1.1

https://crash-stats.mozilla.com/report/index/bp-9d4f7e1f-dc56-49b1-b47d-a5f102120925
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
This patch only just landed in mozilla-central. It is not in a nightly yet. Check it tomorrow.
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
I can still crash on latest-inbound (09/25) with this fix with the STR in comment #11.

D/AndroidRuntime( 7776): Shutting down VM
W/dalvikvm( 7776): threadid=1: thread exiting with uncaught exception (group=0x40d15300)
E/GeckoAppShell( 7776): >>> REPORTING UNCAUGHT EXCEPTION FROM THREAD 1 ("main")
E/GeckoAppShell( 7776): java.lang.NullPointerException
E/GeckoAppShell( 7776): 	at org.mozilla.gecko.BrowserApp.onPrepareOptionsMenu(BrowserApp.java:794)
E/GeckoAppShell( 7776): 	at org.mozilla.gecko.GeckoApp.invalidateOptionsMenu(GeckoApp.java:451)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to Aaron Train [:aaronmt] from comment #13)
> I can still crash on latest-inbound (09/25) with this fix with the STR in
> comment #11.
> 
> D/AndroidRuntime( 7776): Shutting down VM
> W/dalvikvm( 7776): threadid=1: thread exiting with uncaught exception
> (group=0x40d15300)
> E/GeckoAppShell( 7776): >>> REPORTING UNCAUGHT EXCEPTION FROM THREAD 1
> ("main")
> E/GeckoAppShell( 7776): java.lang.NullPointerException
> E/GeckoAppShell( 7776): 	at
> org.mozilla.gecko.BrowserApp.onPrepareOptionsMenu(BrowserApp.java:794)
> E/GeckoAppShell( 7776): 	at
> org.mozilla.gecko.GeckoApp.invalidateOptionsMenu(GeckoApp.java:451)

I tested this and found the the "Save as PDF" menu is null. How can that be? Well, when disabling the Phony add-on, the "Phony" menu is removed. However, the entire "Tools" menu is removed too. Therefore, there is no longer a "Save as PDF" menu and it is null.

Obviously, the submenu code regressed this. I am looking into why it regressed.
Attached patch additional patchSplinter Review
This patch is required to stop the add-on menu from colliding with a submenu. The ID for the first add-on menu is 0, the same as NO_ID constant used for submenu IDs. This patch uses an offset of 1000 for all add-on menu IDs. The offset is applied in Java since that's the part of the code that cares about the collisions.
Attachment #664778 - Flags: review?(sriram)
I was able to crash without the patch. No crash with the patch.
The 20120926 build seems even worse, or it's that I installed a working Adblock Plus dev build, that adds a menu item. Unfortunately Fennec Android has no Safe Mode, so the browser is really dead, crashes seconds after startup!

https://crash-stats.mozilla.com/query/query?product=FennecAndroid&version=FennecAndroid%3A18.0a1&build_id=20120926030625&do_query=1#
Comment on attachment 664778 [details] [diff] [review]
additional patch

I get the problem. "tools" menu doesn't have an ID, and we remove it, if the ID of addon is "0". Do we need an offset like 1000? I believe an offset of 1 should be fine as we won't enter into the zone that android generates.

r+ with that change (if needed).
Attachment #664778 - Flags: review?(sriram) → review+
Blocks: 785994
(In reply to Sriram Ramasubramanian [:sriram] from comment #18)
> ID of addon is "0". Do we need an offset like 1000?

Using 1 might work fine, but I like to go big, or go home :)
https://hg.mozilla.org/mozilla-central/rev/e80d2a8e2fc0
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
What is the test story for this, is it possible to ensure issues like this are caught in try or tbpl in the future? The mere presence of (recommended) add-ons should never lead to startup problems :(
Status: RESOLVED → VERIFIED
Duplicate of this bug: 795001
Not trying to jinx things, but I see a drop in these crashes over the last few days, and of the crashes I see from Sept 27, none are from the Sept 27th build. All are from previous builds.
Blocks: 788520
You need to log in before you can comment on or make changes to this bug.