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

RESOLVED FIXED in Firefox 53

Status

()

--
critical
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: carmenf, 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)

Attachments

(2 attachments)

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
Crash Signature: [@ java.lang.NullPointerException: key == null at android.util.LruCache.get(LruCache.java) ]

Comment 1

2 years 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
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
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)
Flags: needinfo?(rjesup)
See Also: bug 1328104, bug 1326826
(Assignee)

Comment 6

2 years 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 8

2 years ago
mozreview-review
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

2 years 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 11

2 years ago
mozreview-review
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

2 years ago
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 14

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

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

Comment 16

2 years 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

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/ba650eee2f3b
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years 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.