822686 - java.lang.NullPointerException: at org.mozilla.gecko.Favicons$LoadFaviconTask.downloadFavicon(Favicons.java)

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Scoobidiver (away)]

There are one crash in 20.0a1/20121218, bp-e395e6fe-b018-4596-bdbc-6ca6b2121218, and two crashes in 19.0a2.

java.lang.NullPointerException
	at org.mozilla.gecko.Favicons$LoadFaviconTask.downloadFavicon(Favicons.java:225)
	at org.mozilla.gecko.Favicons$LoadFaviconTask.doInBackground(Favicons.java:297)
	at org.mozilla.gecko.Favicons$LoadFaviconTask.doInBackground(Favicons.java:182)
	at android.os.AsyncTask$2.call(AsyncTask.java:287)
	at java.util.concurrent.FutureTask.run(FutureTask.java:234)
	at android.os.AsyncTask$SerialExecutor$1.run(AsyncTask.java:230)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1080)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:573)
	at java.lang.Thread.run(Thread.java:856)

More reports at:
https://crash-stats.mozilla.com/report/list?signature=java.lang.NullPointerException%3A+at+org.mozilla.gecko.Favicons%24LoadFaviconTask.downloadFavicon%28Favicons.java%29

Comment 1

[Chris Peterson [:cpeterson]]

Lucas, can mContext be null on line 244 here? GeckoJarReader.getBitmapDrawable() can return null, but if that was the cause of the crash, I would expect the stack trace to point to line 245.

https://hg.mozilla.org/mozilla-central/annotate/47876c1d1c45/mobile/android/base/Favicons.java#l244

Comment 2

[Chris Peterson [:cpeterson]]

I think null BitmapDrawable is the problem. In the following bug's raw dump, GeckoJarReader logs a "file not found" error.

https://crash-stats.mozilla.com/report/index/8ae4272a-6ee3-4ce7-bbb0-92cff2130226

java.io.FileNotFoundException: /data/app/org.mozilla.firefox_beta-1.apk: open failed: ENOENT (No such file or directory)
   at libcore.io.IoBridge.open(IoBridge.java:416)
   at java.io.RandomAccessFile.<init>(RandomAccessFile.java:118)
   at java.io.RandomAccessFile.<init>(RandomAccessFile.java:150)
   at java.util.zip.ZipFile.<init>(ZipFile.java:130)
   at java.util.zip.ZipFile.<init>(ZipFile.java:103)
   at org.mozilla.gecko.util.GeckoJarReader.getZipFile(GeckoJarReader.java:104)
   at org.mozilla.gecko.util.GeckoJarReader.getBitmapDrawable(GeckoJarReader.java:39)
   at org.mozilla.gecko.Favicons$LoadFaviconTask.downloadFavicon(Favicons.java:252)
   at org.mozilla.gecko.Favicons$LoadFaviconTask.doInBackground(Favicons.java:325)
   at org.mozilla.gecko.Favicons$LoadFaviconTask.doInBackground(Favicons.java:207)
   at android.os.AsyncTask$2.call(AsyncTask.java:287)
   at java.util.concurrent.FutureTask.run(FutureTask.java:234)
   at android.os.AsyncTask$SerialExecutor$1.run(AsyncTask.java:230)
   at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1080)
   at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:573)
   at java.lang.Thread.run(Thread.java:856)

Comment 3

[Chris Peterson [:cpeterson]]

Attachment: handle-null-bitmapdrawable.patch

Check for null return from getBitmapDrawable() and consolidate null checks into new getBitmap() utility method.

Comment 4

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

Comment on attachment 721187 [details] [diff] [review]
handle-null-bitmapdrawable.patch

Nice cleanup and fix

Comment 5

[Chris Peterson [:cpeterson]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4afd31d13e7f

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4afd31d13e7f

Comment 7

[Chris Peterson [:cpeterson]]

Comment on attachment 721187 [details] [diff] [review]
handle-null-bitmapdrawable.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 808027
User impact if declined: Low volume crash. 90% of these crashes are in Firefox 19, so some code change in Beta 20 greatly reduced the volume of this crash, but it continues in Aurora 21 and Nightly 22.
Testing completed (on m-c, etc.): Local testing.
Risk to taking this patch (and alternatives if risky): Low risk patch that adds a null check (and adds a new function to make null checking easier)
String or UUID changes made by this patch: None

Comment 8

[bhavana bajaj [:bajaj]]

Comment on attachment 721187 [details] [diff] [review]
handle-null-bitmapdrawable.patch

null check, avoids rare crashes on aurora.Approving for uplift.

Comment 9

[Chris Peterson [:cpeterson]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/7ad686d04fa3