1247293 - crash in java.lang.NullPointerException: Attempt to invoke virtual method ''android.view.View android.view.ViewStub.inflate()'' on a null object reference at org.mozilla.gecko.home.BrowserSearch.access$500(BrowserSearch.java)

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[:Margaret Leibovic]

This bug was filed from the Socorro interface and is 
report bp-d44d915a-72f8-4ca2-afa8-2a5ea2160209.
=============================================================

I see this crash moving up the Nightly list. This sounds like bug 1100403, but clearly the patch there didn't resolve this issue.

java.lang.NullPointerException: Attempt to invoke virtual method 'android.view.View android.view.ViewStub.inflate()' on a null object reference
	at org.mozilla.gecko.home.BrowserSearch.access$500(BrowserSearch.java:77)
	at org.mozilla.gecko.home.BrowserSearch$4.run(BrowserSearch.java:410)
	at android.os.Handler.handleCallback(Handler.java:739)
	at android.os.Handler.dispatchMessage(Handler.java:95)
	at android.os.Looper.loop(Looper.java:211)
	at android.app.ActivityThread.main(ActivityThread.java:5335)
	at java.lang.reflect.Method.invoke(Native Method)
	at java.lang.reflect.Method.invoke(Method.java:372)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1016)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:811)

Comment 1

[Andrzej Hunt :ahunt]

Based on the stack, I think what's happening is:

1) we're in the setSearchEngines callback:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/home/BrowserSearch.java#410

(Going from the BrowserSearch$4.run at line 410)

2) We end up calling showSuggestionOptIn:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/home/BrowserSearch.java#705

3) And we somehow messed up our state, resulting in an NPE at:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/home/BrowserSearch.java#755

This seems to be related to Bug 1230568, but that doesn't change the state in a way that would cause this:
https://hg.mozilla.org/mozilla-central/rev/97cbdba9513d

Comment 2

[Andrzej Hunt :ahunt]

We set mSuggestionsOptInPrompt to null in:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/home/BrowserSearch.java#854

So it's entirely possible that mSuggestionsOptInPrompt is null _after_ we've already shown it (in which case findViewById also returns null). It seems we'd have to set the "suggestionsPrompted" pref somewhere, to avoid us showing at again?

Comment 3

[Andrzej Hunt :ahunt]

Actually, it seems that I can reproduce this on current nightly:
1) Start nightly with fresh profile
2) Select location bar, type any letter
3) Wait for "Would you like to turn on search suggestions?" to appear, select "No"
4) Press "X" on the right side of the URL bar (exiting via the hardware back button won't reproduce for me)
5) Open tab tray, open new tab
6) Start typing -> crash

Comment 4

[Andrzej Hunt :ahunt]

What seems to be happening is:
- We rely on "prompted" being set once the prompt has been dismissed by selecting "Yes" or "No"
- We do this by setting "browser.search.suggest.enabled":
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/home/BrowserSearch.java#818
- This should implicitly set prompted:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#1922

But based on debugging with the WebIDE we never reach that part of browser.js, hence prompted isn't set to true, so the next time we run setSearchEngines, we try to show the prompt again - which fails because we've already shown it, and discarded it.

Debugging on the Java side we successfully reach "nativeSetPref" when setting browser.search.suggest.enabled, so it seems like problem lies somewhere in the C++ or JS side.


We could add some extra checks on the Java side to prevent the NPE, it doesn't seem very robust to depend on an implicit pref to not crash - but I'm guessing the preference issue will need fixing too.

Comment 5

[Andrzej Hunt :ahunt]

(In reply to Andrzej Hunt :ahunt from comment #4)
> What seems to be happening is:
> - We rely on "prompted" being set once the prompt has been dismissed by
> selecting "Yes" or "No"
> - We do this by setting "browser.search.suggest.enabled":
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/
> mozilla/gecko/home/BrowserSearch.java#818
> - This should implicitly set prompted:
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/
> browser.js#1922
> 
> But based on debugging with the WebIDE we never reach that part of
> browser.js, hence prompted isn't set to true, so the next time we run
> setSearchEngines, we try to show the prompt again - which fails because
> we've already shown it, and discarded it.
> 
> Debugging on the Java side we successfully reach "nativeSetPref" when
> setting browser.search.suggest.enabled, so it seems like problem lies
> somewhere in the C++ or JS side.

Jim, I've noticed you were working on the PrefsHelper ( http://hg.mozilla.org/mozilla-central/filelog/ac39fba33c6d/widget/android/PrefsHelper.h ) - is it possible one of those changes stopped us from receiving the expected events? browser.js seems to expect android-set-pref in browser.js when browser.search.suggest.enabled is set, but that never seems to arrive.

Comment 6

[:Margaret Leibovic]

If this is indeed related to the prefs changes we should fix this in 47.

Comment 7

[(inactive) Jim Chen [:jchen] [:darchons]]

(In reply to Andrzej Hunt :ahunt from comment #5)
> 
> Jim, I've noticed you were working on the PrefsHelper (
> http://hg.mozilla.org/mozilla-central/filelog/ac39fba33c6d/widget/android/
> PrefsHelper.h ) - is it possible one of those changes stopped us from
> receiving the expected events? browser.js seems to expect android-set-pref
> in browser.js when browser.search.suggest.enabled is set, but that never
> seems to arrive.

Ah right, android-set-pref is never sent because "browser.search.suggest.enabled" is an actual pref. browser.js should add a pref change observer in order to handle "browser.search.suggest.enabled" changes.

Comment 8

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Set search suggestion prompted pref along with enabled pref (v1)

Set the prompted pref along with the enabled prefs so that we don't have
to detect setting the enabled pref elsewhere in order to set the
prompted pref.

Comment 9

[Andrzej Hunt :ahunt]

Comment on attachment 8718912 [details] [diff] [review]
Set search suggestion prompted pref along with enabled pref (v1)

Looks good and fixes the crash! (I don't know why we didn't just do this originally?)

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ba1089eb9981

Comment 12

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/ba1089eb9981