995420 - crash in java.lang.NullPointerException: at org.mozilla.gecko.home.DynamicPanel$PanelDatasetLoader.loadCursor(DynamicPanel.java)

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P1)

Description

[Wesley Johnston (:wesj)]

I managed to crash Firefox with my TwitHub addon (shameless plug! https://addons.mozilla.org/en-US/android/addon/twithub/) This happened because I forced myself into about:home and scrolled over to the Twitter panel. I had "authenticated" (i.e. logged in to twitter), but I'm not sure what happened after that. i.e. we may have had no data in the panel. Or something horrible may have happened getting that data to corrupt it.

Stack:
    E/GeckoAppShell( 7074): java.lang.NullPointerException
    E/GeckoAppShell( 7074):         at org.mozilla.gecko.home.DynamicPanel$PanelDatasetLoader.loadCursor(DynamicPanel.java:417)
    E/GeckoAppShell( 7074):         at org.mozilla.gecko.home.SimpleCursorLoader.loadInBackground(SimpleCursorLoader.java:44)
    E/GeckoAppShell( 7074):         at org.mozilla.gecko.home.SimpleCursorLoader.loadInBackground(SimpleCursorLoader.java:26)
    E/GeckoAppShell( 7074):         at android.support.v4.content.AsyncTaskLoader.onLoadInBackground(AsyncTaskLoader.java:242)
    E/GeckoAppShell( 7074):         at android.support.v4.content.AsyncTaskLoader$LoadTask.doInBackground(AsyncTaskLoader.java:51)
    E/GeckoAppShell( 7074):         at android.support.v4.content.AsyncTaskLoader$LoadTask.doInBackground(AsyncTaskLoader.java:40)
    E/GeckoAppShell( 7074):         at android.support.v4.content.ModernAsyncTask$2.call(ModernAsyncTask.java:123)
    E/GeckoAppShell( 7074):         at java.util.concurrent.FutureTask.run(FutureTask.java:234)
    E/GeckoAppShell( 7074):         at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1080)
    E/GeckoAppShell( 7074):         at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:573)
    E/GeckoAppShell( 7074):         at java.lang.Thread.run(Thread.java:841)

Comment 1

[Wesley Johnston (:wesj)]

This was with a private build from this morning. Maybe your new cursor stuff Lucas?

Comment 2

[Teodora Vermesan (:TeoVermesan)]

On the 04/14 build I got a crash with this add-on, only by closing all tabs and going to the twitter panel in about:home, with no authentification.

Comment 4

[:Margaret Leibovic]

Attachment: Make sure cursor returned from HomeProvider isn't null

ContentResolver.query() can return null, so we should do this to be safe, but this will cause issues since our cursor won't ever get updated if the dataset changed.

I looked into why this is happening, and it looks like SQLiteBridgeContentProvider.query() can return null if it doesn't find a database:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/db/SQLiteBridgeContentProvider.java#371

This can happen if no add-on has ever stored any data. So, this is definitely an edge case, but maybe we should do something to create an empty database the first time a consumer tries to access it.

Comment 5

[Lucas Rocha (:lucasr)]

Comment on attachment 8406899 [details] [diff] [review]
Make sure cursor returned from HomeProvider isn't null

Review of attachment 8406899 [details] [diff] [review]:
-----------------------------------------------------------------

Probably worth auditing our code for other spots where we could have problems (i.e. crash) with null cursors.

::: mobile/android/base/home/DynamicPanel.java
@@ +411,5 @@
>              }
>  
>              // XXX: You can use CONTENT_FAKE_URI for development to pull items from fake_home_items.json.
>              final Cursor c = cr.query(HomeItems.CONTENT_URI, null, selection, selectionArgs, null);
>  

Add a comment explaining that the SQLiteBridgeContentProvider might return a null Cursor if the database hasn't been created yet.

Comment 7

[:Margaret Leibovic]

https://hg.mozilla.org/integration/fx-team/rev/ed34a62621f8

Comment 8

[Ryan VanderMeulen [:RyanVM]]

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

Comment 9

[Teodora Vermesan (:TeoVermesan)]

I am not able to reproduce the crash described, so verified fixed on:
Device: LG Nexus 4 (Android 4.4.2)
Build: Firefox for Android 31.0a1 (2014-04-16)

Comment 10

[:Margaret Leibovic]

Comment on attachment 8406899 [details] [diff] [review]
Make sure cursor returned from HomeProvider isn't null

Note to release managers: I'm requesting uplift for a series of Firefox Hub bugs. The main fixes we need are in bugs at the end of the series, but trying to rebase those patches proved difficult and risky, so I think we should just uplift the dependecies.

Note to sheriffs: I have a local patch series that I can land on aurora when these bugs all get approval.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): dependency for initial Firefox Hub release (promoted feed add-ons in fx30)
User impact if declined: panels won't update when data changes
Testing completed (on m-c, etc.): baked on m-c
Risk to taking this patch (and alternatives if risky): only affects dynamic panels, need an add-on to trigger this
String or IDL/UUID changes made by this patch: none

Comment 11

[:Margaret Leibovic]

https://hg.mozilla.org/releases/mozilla-aurora/rev/4e17bb626ec1

Comment 12

[:Margaret Leibovic]

Setting P1 hub bugs to block hub v1 EPIC bug (targeting fx30 release).

Filter on epic-hub-bugs.