1202583 - crash in java.lang.NullPointerException: Attempt to read from field ''java.lang.String org.mozilla.gecko.home.SearchEngine.name'' on a null object reference at org.mozilla.gecko.BrowserApp.onSearch(BrowserApp.java)

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[Christian Ascheberg]

This bug was filed from the Socorro interface and is 
report bp-8456b1c9-c6a3-4ec2-8bd3-75d662150908.
=============================================================

STR:

1. type any search text into urlbar
2. click the first suggested item (before bookmarks/history) to trigger a search

crashes every time

Comment 1

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

I think there is a plan to remove this "button", which is a copy of what the user has typed in the URLBar, but until that time let's not crash.

Comment 2

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Mark Finkle (:mfinkle) from comment #1)
> I think there is a plan to remove this "button", which is a copy of what the
> user has typed in the URLBar,

bug 1201670.

Comment 3

[Allison Naaktgeboren :ally]

the crash points to 

http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/BrowserApp.java#3838
 openUrlAndStopEditing(text, engine.name);


http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/home/SearchEngineRow.java#98
mSearchListener.onSearch(mSearchEngine, suggestion); in the onClick() handler.

I have been unable to reproduce this on any of my devices including the nexus family. If I understand the crash report, this is a nexus 4 in the german locale (useragent_locale de). 



I have been unable to reproduce this on any of my devices.

Comment 4

[Allison Naaktgeboren :ally]

going to attempt to put phone in german. See if I can get a test case.

Comment 5

[Allison Naaktgeboren :ally]

tried unicode, nonenglish characters. No luck.

Comment 6

[Allison Naaktgeboren :ally]

phone & nightly localized to german. No crashes.

Comment 7

[Allison Naaktgeboren :ally]

I can I suppose put a null check in around search engine object & its name field, but I'm not comfortable shipping that without a test case to check against, or an understanding why the searchengine object and/or name might be null. 

I don't suppose it is possible to know or ask the user what search engine he/she was using?

Comment 8

[Eugen Sawin [:esawin]]

Attachment: logcat.txt

Logcat from a crash on Nexus 7 (English (Google) keyboard & system language).

Comment 9

[Christian Ascheberg]

The crash report is from my device. I even deleted all Nightly data to have a kind of "clean install", so the default search engine is Google.
(I might not be available for a few days from now.)

Comment 10

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

(In reply to Eugen Sawin [:esawin] from comment #8)
> Created attachment 8658296 [details]
> logcat.txt
> 
> Logcat from a crash on Nexus 7 (English (Google) keyboard & system language).

Eugen - Just so we know exactly what UI item you are tapping, can you given more explicit details? I assumed Christian was tapping the the very first "little button" in the list of search engine (Google in his case) suggestions. Am I wrong?

Comment 11

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

I see that we pass mSearchEngine in here:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/home/SearchEngineRow.java#156

But just a few lines below it, we caution that mSearchEngine might be null:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/home/SearchEngineRow.java#165

Maybe a null check like the one used in line 165 would be enough.

Comment 12

[Eugen Sawin [:esawin]]

This video should make it clear: https://youtu.be/O61fLUG7EEM

Comment 13

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

(In reply to Eugen Sawin [:esawin] from comment #12)
> This video should make it clear: https://youtu.be/O61fLUG7EEM

Yes. Thanks.

It also shows that the "opt-in" request for search suggestions is still active. That could be part of the issue.

Comment 14

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

re comment 12's video, it's also curious the primary search engine's favicon is missing.

Comment 15

[Allison Naaktgeboren :ally]

Attachment: LoggingMissingSearchEngineCrash

https://treeherder.mozilla.org/#/jobs?repo=try&revision=59804d06f2ea

Comment 18

[Allison Naaktgeboren :ally]

one reporter is in canada, with a nonstandard search engine. The other is unknown and has been NI'ed. Given the multiple reports this week, I am going to roll together a fatter test build.

Comment 19

[Allison Naaktgeboren :ally]

Bug 1203053 + this one seem to strongly suggest a missing search engine object.

Comment 20

[Allison Naaktgeboren :ally]

kats, esawin, any one else who can reproduce,

Please try out the following build and get the log back to me. You can filter on the tag "anaaktge" and if/when it crashes, I'd like the crash stack in case the logging has caused it to crash earlier.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=934389777d7d

Output from my test build for comparison: https://pastebin.mozilla.org/8845966

Comment 21

[Allison Naaktgeboren :ally]

Worth noting from today's digging:

-- the search engine list comes from gecko, and is made into an un-modifable list. So if the list were to come from gecko malformed, come too early, or throw during transcription,  we'd be stuck for it.

-- there is a baked in assumption that the default engine and the engine used for suggestions are the same.

Comment 22

[Allison Naaktgeboren :ally]

I have (on loan) a device I can get to crash.

https://pastebin.mozilla.org/8845975  crash log

What I _don't_ see in this log is the setting of mSearchEngine in SearchEngineRow... I think the existing logic assumes that its been set by updateSearchSuggestions function, and if that function hasn't been called yet, the onclick handler is going to call search on a null object..which is what we've got.

Comment 23

[Allison Naaktgeboren :ally]

I think I have a fix.

Comment 24

[Allison Naaktgeboren :ally]

Attachment: MissingEngineCrash

Comment 25

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

Comment on attachment 8660045 [details] [diff] [review]
MissingEngineCrash

># HG changeset patch
># User Allison Naaktgeboren <[e-mail]>
># Parent  f73784293c11abb3f2a4e2272d7aa3423d5f9dbe
>Bug 1202583 - crash in java.lang.NullPointerException: Attempt to read from field ''java.lang.String org.mozilla.gecko.home.SearchEngine.name'' on a null object reference.r=mfinkle
>
>diff --git a/mobile/android/base/home/SearchEngineRow.java b/mobile/android/base/home/SearchEngineRow.java
>--- a/mobile/android/base/home/SearchEngineRow.java
>+++ b/mobile/android/base/home/SearchEngineRow.java
>@@ -239,18 +239,16 @@ class SearchEngineRow extends AnimatedHe
>             }
>         } finally {
>             c.close();
>         }
>         hideRecycledSuggestions(suggestionCounter, recycledSuggestionCount);
>     }
>
>     private int updateFromSearchEngine(SearchEngine searchEngine, boolean animate, int recycledSuggestionCount) {

The existing code uses updateFromSearchEngine as _the_ place where we set mSearchEngine. That is not true anymore, which is fine, but let's not pass in the SearchEngine anymore either. It's unused now.

>-        // Update search engine reference.
>-        mSearchEngine = searchEngine;
>

Remove this blank line too


Also update the comment here:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/home/SearchEngineRow.java#164

updateSuggestions is the place we set mSearchEngine now.


r+, but make the fixes

Comment 26

[Allison Naaktgeboren :ally]

https://hg.mozilla.org/integration/fx-team/rev/b185d4bce7d91958696be9c750ac43252dfa23b0
Bug 1202583 - crash in java.lang.NullPointerException: Attempt to read from field ''java.lang.String org.mozilla.gecko.home.SearchEngine.name'' on a null object reference.r=mfinkle

Comment 27

[Ekanan Ketunuti]

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

Comment 28

[Teodora Vermesan (:TeoVermesan)]

Reproduced the crash with 08/09 Nightly build using LG G4 (Android 5.1). Verified as fixed using Firefox for Android 43.0a2 (2015-09-22).