826267 - crash @ java.lang.NullPointerException: at org.mozilla.gecko.AllPagesTab.showSuggestionsOptIn(AllPagesTab.java)

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[Robert Kaiser]

This bug was filed from the Socorro interface and is 
report bp-1f0f359c-b6ae-4d8e-807c-364712130103 .
============================================================= 

java.lang.NullPointerException
	at org.mozilla.gecko.AllPagesTab.showSuggestionsOptIn(AllPagesTab.java:605)
	at org.mozilla.gecko.AllPagesTab.setSearchEngines(AllPagesTab.java:592)
	at org.mozilla.gecko.AllPagesTab.access$1500(AllPagesTab.java:62)
	at org.mozilla.gecko.AllPagesTab$7.run(AllPagesTab.java:684)
	at android.os.Handler.handleCallback(Handler.java:608)
	at android.os.Handler.dispatchMessage(Handler.java:92)
	at android.os.Looper.loop(Looper.java:156)
	at android.app.ActivityThread.main(ActivityThread.java:5045)
	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:784)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:551)
	at dalvik.system.NativeStart.main(Native Method)


More crash reports at https://crash-stats.mozilla.com/report/list?signature=java.lang.NullPointerException%3A%20at%20org.mozilla.gecko.AllPagesTab.showSuggestionsOptIn%28AllPagesTab.java%29

This has been happening on FennecAndroid 20.0a1 Nightly apparently since the 2012-12-19 build, apparently.
Right now it's the #2 crash in yesterday's Nightly data.

Comment 1

[Brian Nicholson (:bnicholson)]

Happening at this line:

        promptText.setPrivateMode(Tabs.getInstance().getSelectedTab().isPrivate());

getSelectedTab() is likely the NPE, and this is probably happening whenever users try to launch the AwesomeScreen after a crash restore (which doesn't use tab stubs).

While we could keep adding null checks around these kinds of statements, I think a better fix would be preventing the AwesomeScreen from opening at all until we have at least one tab in Java. Otherwise, if we're doing a restore, we don't know whether the selected tab will be private or non-private, so the AwesomeScreen UI state could be a mismatch between the selected tab's state.

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

Just keep in mind that one of the reason we re-wrote in Java was to avoid blocking on Gecko. We _must_ be able to open the Awesomescreen before Gecko is ready.

Comment 3

[Brian Nicholson (:bnicholson)]

Private tabs are only restored after OOM kills, so it actually shouldn't be possible with our current implementation for private tabs to be included in a crash restore. We could probably just get away with doing a null check after all, which wouldn't block the AwesomeScreen.

Comment 4

[Scoobidiver (away)]

The regression range is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=2e70b718903a&tochange=bfd85c9652fa

Comment 5

[Brian Nicholson (:bnicholson)]

Attachment: Check for null tab before setting private AwesomeScreen state

Comment 6

[Sriram Ramasubramanian [:sriram]]

Comment on attachment 698896 [details] [diff] [review]
Check for null tab before setting private AwesomeScreen state

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

Looks good to me.

Comment 7

[Brian Nicholson (:bnicholson)]

Sorry, I gave bad advice in https://bugzilla.mozilla.org/show_bug.cgi?id=821311#c3

Comment 8

[Brian Nicholson (:bnicholson)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/341e741bec3b

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/341e741bec3b

Comment 10

[Scoobidiver (away)]

It's still 3 top crasher in 20.0a2.

Comment 11

[Brian Nicholson (:bnicholson)]

Comment on attachment 698896 [details] [diff] [review]
Check for null tab before setting private AwesomeScreen state

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 821311
User impact if declined: crash whenever opening awesomescreen immediately after session restore
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): very low risk (just adds null check)
String or UUID changes made by this patch: none

Comment 12

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 698896 [details] [diff] [review]
Check for null tab before setting private AwesomeScreen state

low risk null check, approved for aurora.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ed901509fe6a