Closed Bug 803289 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, critical)

19 Branch
ARM
Android
defect
Not set
critical

Tracking

()

RESOLVED FIXED
Firefox 19
Tracking Status
firefox18 --- unaffected
firefox19 + fixed
fennec 19+ ---

People

(Reporter: scoobidiver, Assigned: bnicholson)

References

Details

(4 keywords)

Crash Data

Attachments

(1 file)

It's the third version after bug 793136 and bug 773089.
This one appeared in 19.0a1/20121018. Its regression range is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=dac5700acf8b&tochange=cb573b9307e5

Here is a crash report: bp-26f3f9a7-7a4b-43be-8766-412f02121018.

java.lang.NullPointerException
	at org.mozilla.gecko.BrowserApp.onPrepareOptionsMenu(BrowserApp.java:902)
	at org.mozilla.gecko.GeckoApp.invalidateOptionsMenu(GeckoApp.java:466)
	at org.mozilla.gecko.GeckoApp.onTabChanged(GeckoApp.java:243)
	at org.mozilla.gecko.BrowserApp.onTabChanged(BrowserApp.java:119)
	at org.mozilla.gecko.Tabs.notifyListeners(Tabs.java:409)
	at org.mozilla.gecko.Tabs.notifyListeners(Tabs.java:398)
	at org.mozilla.gecko.Tabs$2.run(Tabs.java:142)
	at android.app.Activity.runOnUiThread(Activity.java:4591)
	at org.mozilla.gecko.Tabs.selectTab(Tabs.java:137)
	at org.mozilla.gecko.Tabs.loadUrl(Tabs.java:512)
	at org.mozilla.gecko.AwesomebarResultHandler.onActivityResult(AwesomebarResultHandler.java:27)
	at org.mozilla.gecko.ActivityHandlerHelper.handleActivityResult(ActivityHandlerHelper.java:220)
	at org.mozilla.gecko.GeckoApp.onActivityResult(GeckoApp.java:2492)
	at android.app.Activity.dispatchActivityResult(Activity.java:5192)
	at android.app.ActivityThread.deliverResults(ActivityThread.java:3137)
	at android.app.ActivityThread.handleSendResult(ActivityThread.java:3184)
	at android.app.ActivityThread.access$1100(ActivityThread.java:130)
	at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1243)
	at android.os.Handler.dispatchMessage(Handler.java:99)
	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
There are about 40 crashes per build so it seems easily reproducible.
Happens for me when I'm trying to open a new tab. I type e.g. 'la rafle', and select the Wikipedia search, and then firefox crashes.
Keywords: reproducible
Sriram, this BrowserApp.onPrepareOptionsMenu() crash is the #1 topcrash for Nightly 19. It looks just like bug 793136.
This makes testing Nightly unusable (blocker status) on my Galaxy S.
Assignee: nobody → sriram
tracking-fennec: ? → 19+
I'll take this - likely a regression from bug 800199.
Assignee: sriram → bnicholson
Blocks: 800199
Bug 800199 sets the URL for tab stubs if userEntered is false. The thinking was that if the URI is not entered by the user, we can assume the URI is valid, so we can set it as the tab's URL. This turns out not to be a good test, though, because a number of actions can result in non-URIs being loaded where userEntered is false (e.g., clicking a search engine).

This patches does more a complete URI check before setting the tab's URL. If the URI is not valid, we wait for Gecko's Tab:Added before setting the tab's URL, which is what we always did before tab stubs.
Attachment #674028 - Flags: review?(mark.finkle)
Comment on attachment 674028 [details] [diff] [review]
Only set stubbed tab URLs for valid URIs

Is there a way we can add a test for this case?
Attachment #674028 - Flags: review?(mark.finkle) → review+
(In reply to Mark Finkle (:mfinkle) from comment #7)
> Comment on attachment 674028 [details] [diff] [review]
> Only set stubbed tab URLs for valid URIs
> 
> Is there a way we can add a test for this case?

Ideally, we'd have a test case for search engines, and that would have caught this. But since the tests are offline, we can't actually visit the search engine sites. Maybe we can still click a search engine entry and verify an engine URL is loaded, even though we'll get a 404?
(In reply to Brian Nicholson (:bnicholson) from comment #8)
> (In reply to Mark Finkle (:mfinkle) from comment #7)
> > Comment on attachment 674028 [details] [diff] [review]
> > Only set stubbed tab URLs for valid URIs
> > 
> > Is there a way we can add a test for this case?
> 
> Ideally, we'd have a test case for search engines, and that would have
> caught this. But since the tests are offline, we can't actually visit the
> search engine sites. Maybe we can still click a search engine entry and
> verify an engine URL is loaded, even though we'll get a 404?

I was thinking of ways to simulate the situation. We could pass URLs on a command line. We could make a fake sessionstore.js with bad URLs. What is the exact situation we get into to cause this problem? What's the code path? We could just call Tabs.loadURL with a bogus URL, right?
(In reply to Mark Finkle (:mfinkle) from comment #9)
> I was thinking of ways to simulate the situation. We could pass URLs on a
> command line. We could make a fake sessionstore.js with bad URLs. What is
> the exact situation we get into to cause this problem? What's the code path?
> We could just call Tabs.loadURL with a bogus URL, right?

To simulate this particular situation, we could edit a bookmark entry to point to a bogus URI, then click it. That also results in the same crash here.
http://hg.mozilla.org/integration/mozilla-inbound/rev/363b1a7f2cae

Leaving open for test case.
Whiteboard: [leave open]
Target Milestone: --- → Firefox 19
(In reply to Brian Nicholson (:bnicholson) from comment #11)
> 
> Leaving open for test case.

Brian, are you still waiting for a test case? This crash looks like it has been fixed because it hasn't shown up in Socorro crash reports since you landed your fix on 2012-10-23.
(In reply to Chris Peterson (:cpeterson) from comment #13)
> (In reply to Brian Nicholson (:bnicholson) from comment #11)
> > 
> > Leaving open for test case.
> 
> Brian, are you still waiting for a test case? This crash looks like it has
> been fixed because it hasn't shown up in Socorro crash reports since you
> landed your fix on 2012-10-23.

Yeah, I was going to add a simple test case as suggested in comment 7. I just filed bug 806465 so we don't need to leave this open.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [leave open]
You need to log in before you can comment on or make changes to this bug.