974289 - crash in java.lang.IllegalArgumentException: column ''_data'' does not exist at android.database.AbstractCursor.getColumnIndexOrThrow(AbstractCursor.java)

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Teodora Vermesan (:TeoVermesan)]

This bug was filed from the Socorro interface and is 
report bp-e0932cd0-9a02-4714-a484-9577e2140219.
=============================================================
Device: LG Nexus 4 
Android: 4.4.2
Build: Firefox for Android 30.0a1 (2014-02-18)

Steps:
1.Go to facebook.com
2.Tap on the profile picture
3.Tap "Change profile picture"
3.Choose "Upload photo" -> "Add a photo"
4.Choose "Documents" from the list
5.Choose a photo
The app crashes

Stack Trace:
java.lang.IllegalArgumentException: column '_data' does not exist
	at android.database.AbstractCursor.getColumnIndexOrThrow(AbstractCursor.java:303)
	at android.database.CursorWrapper.getColumnIndexOrThrow(CursorWrapper.java:78)
	at org.mozilla.gecko.FilePickerResultHandler$VideoLoaderCallbacks.onLoadFinished(FilePickerResultHandler.java:117)
	at android.support.v4.app.LoaderManagerImpl$LoaderInfo.callOnLoadFinished(LoaderManager.java:427)
	at android.support.v4.app.LoaderManagerImpl$LoaderInfo.onLoadComplete(LoaderManager.java:395)
	at android.support.v4.content.Loader.deliverResult(Loader.java:103)
	at android.support.v4.content.CursorLoader.deliverResult(CursorLoader.java:81)
	at android.support.v4.content.CursorLoader.deliverResult(CursorLoader.java:35)
	at android.support.v4.content.AsyncTaskLoader.dispatchOnLoadComplete(AsyncTaskLoader.java:221)
	at android.support.v4.content.AsyncTaskLoader$LoadTask.onPostExecute(AsyncTaskLoader.java:61)
	at android.support.v4.content.ModernAsyncTask.finish(ModernAsyncTask.java:461)
	at android.support.v4.content.ModernAsyncTask.access$500(ModernAsyncTask.java:47)
	at android.support.v4.content.ModernAsyncTask$InternalHandler.handleMessage(ModernAsyncTask.java:474)
	at android.os.Handler.dispatchMessage(Handler.java:102)
	at android.os.Looper.loop(Looper.java:136)
	at android.app.ActivityThread.main(ActivityThread.java:5017)
	at java.lang.reflect.Method.invokeNative(Native Method)
	at java.lang.reflect.Method.invoke(Method.java:515)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:779)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:595)
	at dalvik.system.NativeStart.main(Native Method)

Comment 2

[Aaron Train [:aaronmt]]

Probably something from

http://hg.mozilla.org/mozilla-central/rev/20ae4f402369
http://hg.mozilla.org/mozilla-central/rev/94c8a67eee87

Comment 3

[Wesley Johnston (:wesj)]

Attachment: Patch

This tries to get the column earlier in the try catch section here. If it fails, we'll fall back to our old method. I'm also not sure why I was using the wrong Strings as well. Fixed :(

I can't reproduce this using any of the half dozen file pickers I've installed. Do you have any idea what you're using? Is it something that just shipped with the phone or something I can test? I put up a build at:
http://people.mozilla.org/~wjohnston/filepicker.apk

if you wouldn't mind testing it.

Comment 4

[Lucas Rocha (:lucasr)]

Comment on attachment 8380135 [details] [diff] [review]
Patch

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

::: mobile/android/base/FilePickerResultHandler.java
@@ -95,5 @@
>          // Finally, Video pickers and some file pickers may return a content provider.
>          try {
>              // Try a query to make sure the expected columns exist
>              final ContentResolver cr = fa.getContentResolver();
> -            Cursor cursor = cr.query(uri, new String[] { "MediaStore.Video.Media.DATA" }, null, null, null);

Why was it quoted before? Looks quite suspicious. I mean, how was this even working at all? :-)

@@ +96,5 @@
>          try {
>              // Try a query to make sure the expected columns exist
>              final ContentResolver cr = fa.getContentResolver();
> +            Cursor cursor = cr.query(uri, new String[] { MediaStore.Video.Media.DATA }, null, null, null);
> +            cursor.getColumnIndexOrThrow(MediaStore.Video.Media.DATA);

This deserves at least a comment with come more context. A cleaner approach would be to simply use getColumnIndex() instead and only run the rest of the try block if it's >= 0.

@@ +101,1 @@
>              cursor.close();

Not a change in this patch but the close() call should be moved to a finally block.

Comment 5

[Lucas Rocha (:lucasr)]

Notifying Teodora to get some testing on the linked APK.

Comment 6

[Teodora Vermesan (:TeoVermesan)]

(In reply to Wesley Johnston (:wesj) from comment #3)
> I put up a build at:
> http://people.mozilla.org/~wjohnston/filepicker.apk
> if you wouldn't mind testing it.

Using this apk, I cannot reproduce the crash with the steps given.

Comment 7

[Wesley Johnston (:wesj)]

Attachment: Patch

I can't test today, but this implements what you want.

I think the string happened in my final testing of the last patch. I made one last pass using a "fake" column name to make sure things worked if it failed. I must have left the "" in while reverting it.

Comment 8

[Lucas Rocha (:lucasr)]

Comment on attachment 8382607 [details] [diff] [review]
Patch

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

::: mobile/android/base/FilePickerResultHandler.java
@@ +106,1 @@
>              return;

This 'return' should be inside the if above, no?

@@ +106,5 @@
>              return;
> +        } catch(Exception ex) {
> +            // We'll try a different loader below
> +        } finally {
> +            if (cursor != null)

Brace the if.

Comment 9

[Wesley Johnston (:wesj)]

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

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9a04968eb54d