Closed Bug 1202583 Opened 5 years ago Closed 5 years ago

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)

Categories

(Firefox for Android :: Awesomescreen, defect, critical)

ARM
Android
defect
Not set
critical

Tracking

()

VERIFIED FIXED
Firefox 43
Tracking Status
firefox43 --- verified

People

(Reporter: c.ascheberg, Assigned: ally)

References

Details

(Keywords: crash)

Crash Data

Attachments

(2 files, 1 obsolete file)

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
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.
Assignee: nobody → ally
(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.
See Also: → 1201670
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.
going to attempt to put phone in german. See if I can get a test case.
tried unicode, nonenglish characters. No luck.
phone & nightly localized to german. No crashes.
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?
Flags: needinfo?(c.ascheberg)
Attached file logcat.txt
Logcat from a crash on Nexus 7 (English (Google) keyboard & system language).
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.)
Flags: needinfo?(c.ascheberg)
(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?
Flags: needinfo?(esawin)
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.
This video should make it clear: https://youtu.be/O61fLUG7EEM
Flags: needinfo?(esawin)
(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.
re comment 12's video, it's also curious the primary search engine's favicon is missing.
Duplicate of this bug: 1203360
Duplicate of this bug: 1203447
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.
Bug 1203053 + this one seem to strongly suggest a missing search engine object.
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
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.
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.
I think I have a fix.
Attachment #8659016 - Attachment is obsolete: true
Attachment #8660045 - Flags: review?(mark.finkle)
Comment on attachment 8660045 [details] [diff] [review]
MissingEngineCrash

># HG changeset patch
># User Allison Naaktgeboren <ally@mozilla.com>
># 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
Attachment #8660045 - Flags: review?(mark.finkle) → review+
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
https://hg.mozilla.org/mozilla-central/rev/b185d4bce7d9
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
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).
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.