Crash in java.lang.NullPointerException: key == null at android.util.LruCache.get(LruCache.java)

RESOLVED FIXED in Firefox 53

Status

()

Firefox for Android
General
--
critical
RESOLVED FIXED
6 months ago
5 months ago

People

(Reporter: Carmen Fat, Assigned: jwu)

Tracking

({regression, reproducible})

53 Branch
Firefox 53
ARM
Android
regression, reproducible
Points:
---

Firefox Tracking Flags

(firefox51 unaffected, firefox52 unaffected, firefox53 fixed)

Details

(crash signature)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Reporter)

Description

6 months ago
Environment: 
Device: Nexus 5 (Android 6.0.1);
Build: Nightly 53.0a1 (2016-12-22);

Steps to reproduce:
1. Launch Firefox;
2. Go to imdb.com;
3. Long tap in the search field to bring up the action bar; 
4. Choose "Add Search Engine"(the magnifying glass icon);
5. Go to Settings -> Search.

Expected result:
Search panel is displayed.

Actual result:
Firefox crashes.

Notes:
Crash report: https://crash-stats.mozilla.com/report/index/682d239a-a436-4546-b46b-48aad2161223
(Reporter)

Updated

6 months ago
Crash Signature: [@ java.lang.NullPointerException: key == null at android.util.LruCache.get(LruCache.java) ]

Comment 1

6 months ago
This is 100% reproducible with the 9gag.com search bar also added.
This is currently the #2 top crash on nightly fennec.
Keywords: regression, regressionwindow-wanted, reproducible
(Reporter)

Comment 3

6 months ago
Tested on:
 - Huawei Honor 5x (Android 5.1.1)

Regression Window performed:
Last known good build: 2016-12-18
First known bad build: 2016-12-19

Pushlog: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=f22c6dc53276d9cb1e7365e47dc52657facc5e48&tochange=12f4a1387fbc20a4696cd0cc9ea1ae7cddeb213c
(Reporter)

Updated

6 months ago
Keywords: regressionwindow-wanted
Another search button/etc bug
See Also: → bug 1328104, bug 1326826
(In reply to Randell Jesup [:jesup] from comment #4)
> Another search button/etc bug

Not sure what you tried to do, but I don't think you intended to connect a Firefox desktop Linux-only search bug with a java crash on fennec.
Flags: needinfo?(rjesup)

Updated

5 months ago
Flags: needinfo?(rjesup)
See Also: bug 1328104, bug 1326826
(Assignee)

Comment 6

5 months ago
According to https://developer.android.com/reference/android/util/LruCache.html, null cannot be a key for LruCache. We should check if key is null before call LruCache.get(key);
Assignee: nobody → topwu.tw
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)
Comment on attachment 8826515 [details]
Bug 1325586 - Prevent creating IconDescriptor with null url,

https://reviewboard.mozilla.org/r/104474/#review105324

This fixes the sympton, but looking at the bigger picture something is wrong here: We shouldn't end up with a 'null' value here in the first place.

It would be interesting to follow the path back and see how we got a null URL into the IconDescriptor object. Looking at it we maybe should at some @NonNull annotations to the IconDescriptor methods too. And it might be worth adding null checks into IconDescriptor so that we fail earlier and can fix the calling code that inserts those nulls.
Attachment #8826515 - Flags: review?(s.kaspari) → review-
(Assignee)

Comment 9

5 months ago
mozreview-review-reply
Comment on attachment 8826515 [details]
Bug 1325586 - Prevent creating IconDescriptor with null url,

https://reviewboard.mozilla.org/r/104474/#review105324

After deep check, the crash is caused by SearchEnginePreference#setSearchEngineFromBundle(GeckoBundle). Because iconURI provided by GeckoBundle could be nullable and no null check before creating an IconDescriptor. 

I'll add null check before creating an IconDescriptor. Also adding @NonNull annotations to the IconDescriptor methods.
Comment hidden (mozreview-request)
Comment on attachment 8826515 [details]
Bug 1325586 - Prevent creating IconDescriptor with null url,

https://reviewboard.mozilla.org/r/104474/#review105934

Nice! This looks good.

I have only one doubt: Now we are displaying no icon, I guess? This is fine, but how does this behave in the search engine bar if you enter text and scroll to this search engine - do we show an icon here? Just reflag me for review on bugzilla after you have tried this and if you decide no further change is needed. :)
Attachment #8826515 - Flags: review?(s.kaspari)
(Assignee)

Comment 12

5 months ago
Created attachment 8827781 [details]
device-2017-01-18-151626.png

You can see the attachment there is a default icon when the search engine. Do you think we should also show the default icon in search preference page?
Flags: needinfo?(s.kaspari)
(In reply to Jing-wei Wu [:jwu] from comment #12)
> You can see the attachment there is a default icon when the search engine.
> Do you think we should also show the default icon in search preference page?

Okay! I just wanted to make sure that this is working too. I'll r+ the patch.
status-firefox51: --- → ?
status-firefox52: --- → ?
Flags: needinfo?(s.kaspari)
Comment on attachment 8826515 [details]
Bug 1325586 - Prevent creating IconDescriptor with null url,

https://reviewboard.mozilla.org/r/104474/#review106712
Attachment #8826515 - Flags: review+
(Assignee)

Comment 15

5 months ago
try result: https://treeherder.mozilla.org/#/jobs?repo=try&revision=886b95bcdccee4651fc08af84a938817b8792dd8
Keywords: checkin-needed

Comment 16

5 months ago
Pushed by ihsiao@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ba650eee2f3b
Prevent creating IconDescriptor with null url, r=sebastian
Keywords: checkin-needed

Comment 17

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/ba650eee2f3b
Status: ASSIGNED → RESOLVED
Last Resolved: 5 months ago
status-firefox53: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
status-firefox51: ? → unaffected
status-firefox52: ? → unaffected
You need to log in before you can comment on or make changes to this bug.