768900 - java.lang.NullPointerException: at org.mozilla.gecko.AndroidImport.mergeBookmarks(AndroidImport.java)

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Scoobidiver (away)]

It first appeared in 16.0a1/20120626092318 (not an official nightly build). See bp-7669c265-7f1d-4124-989c-17c432120627.
The regression window might be (low volume):
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c659521a7a9a&tochange=5cdbeae14405
It's likely a regression from bug 710330.

java.lang.NullPointerException
	at org.mozilla.gecko.AndroidImport.mergeBookmarks(AndroidImport.java:59)
	at org.mozilla.gecko.AndroidImport.run(AndroidImport.java:142)
	at org.mozilla.gecko.AndroidImportPreference$2.run(AndroidImportPreference.java:71)
	at android.os.Handler.handleCallback(Handler.java:605)
	at android.os.Handler.dispatchMessage(Handler.java:92)
	at android.os.Looper.loop(Looper.java:137)
	at org.mozilla.gecko.GeckoBackgroundThread.run(GeckoBackgroundThread.java:31)

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

Comment 1

[Gian-Carlo Pascutto [:gcp]]

       final int faviconCol = cursor.getColumnIndexOrThrow(Browser.BookmarkColumns.FAVICON);

FAVICON is API level 1, so this probably happens if the user has no actual bookmarks, though I think the column is still supposed to exist in the projection. Maybe this happens if the user has no bookmarks, the database itself isn't even created.

Easy to fix but a bit of a strange bug IMHO.

Comment 3

[Aaron Train [:aaronmt]]

STR: i) disable com.google.android.browser ii) attempt to import history/bookmarks

Comment 4

[Lucas Rocha (:lucasr)]

(In reply to Aaron Train [:aaronmt] from comment #3)
> STR: i) disable com.google.android.browser ii) attempt to import
> history/bookmarks

Hmm, in this case the returned cursor is probably null, not simply pointing to an empty result set. I guess doing a null check on the cursor in this specific case makes sense.

Comment 5

[Gian-Carlo Pascutto [:gcp]]

Attachment: Patch 1. Check for null and catch errors

Not sure if the try/finally is really needed, but in another bug I ran into the problem that some Android versions have bugs that "forget" to create random bookmarks columns, so let's not crash our app if that is the case.

Comment 6

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

Comment on attachment 637911 [details] [diff] [review]
Patch 1. Check for null and catch errors

Seems OK, just move the "cursor" declaration out of the try blocks and fix the ";" on it's own line issue.

Comment 7

[Gian-Carlo Pascutto [:gcp]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/eb110cad75a5

Comment 8

[Ryan VanderMeulen [:RyanVM]]

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

Comment 9

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 637911 [details] [diff] [review]
Patch 1. Check for null and catch errors

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 710330
User impact if declined: App will crash if the user tries to import from the Android Browser while that Browser is disabled in the OS.
Testing completed (on m-c, etc.): Landed a few days ago in m-c.
Risk to taking this patch (and alternatives if risky): Very low.

Comment 10

[Gian-Carlo Pascutto [:gcp]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/23c1df05027f