904551 - crash in java.lang.NullPointerException: at android.content.ContentResolver.acquireUnstableProvider(ContentResolver.java)

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Martijn Wargers (dead)]

This bug was filed from the Socorro interface and is 
report bp-9d9dd0cc-ef5d-4b94-9bf3-743d82130813 .
 ============================================================= 
0 	libmozalloc.so 	mozalloc_abort(char const*) 	memory/mozalloc/mozalloc_abort.cpp
1 	libxul.so 	Java_org_mozilla_gecko_GeckoAppShell_reportJavaCrash 	widget/android/AndroidJNI.cpp
2 	libmozglue.so 	Java_org_mozilla_gecko_GeckoAppShell_reportJavaCrash 	mozglue/android/jni-stubs.inc
3 	libdvm.so 	libdvm.so@0x1dc4e 	
4 	data@[e-mail]@classes.dex 	data@[e-mail]@classes.dex@0x1ec0f5 	
5 	dalvik-heap (deleted) 	dalvik-heap (deleted)@0x8e5b36 	
6 	libdvm.so 	libdvm.so@0x4ded1 	
7 	data@[e-mail]@classes.dex 	data@[e-mail]@classes.dex@0x1ec0f3 	
8 	libmozglue.so 	Java_org_mozilla_gecko_GeckoAppShell_onSurfaceTextureFrameAvailable 	mozglue/android/jni-stubs.inc
9 		@0xe670001b 	
10 	libmozglue.so 	Java_org_mozilla_gecko_GeckoAppShell_onSurfaceTextureFrameAvailable 	mozglue/android/jni-stubs.inc

I'm getting this crash with current Fennec trunk build on the Samsung Galaxy Nexus on: http://people.mozilla.org/~mwargers/tests/dom/focus/test_input_file_picker.html
- Tap on the 3rd input type=file button
- Choose Camcorder as option and record something and save it.

Result: crash

Comment 1

[Scoobidiver (away)]

Here is the Java stack trace:
java.lang.NullPointerException
	at android.content.ContentResolver.acquireUnstableProvider(ContentResolver.java:1144)
	at android.content.ContentResolver.query(ContentResolver.java:398)
	at android.content.ContentResolver.query(ContentResolver.java:357)
	at android.app.Activity.managedQuery(Activity.java:1765)
	at org.mozilla.gecko.CameraVideoResultHandler.onActivityResult(CameraVideoResultHandler.java:49)
	at org.mozilla.gecko.ActivityHandlerHelper.handleActivityResult(ActivityHandlerHelper.java:358)
	at org.mozilla.gecko.GeckoApp.onActivityResult(GeckoApp.java:2342)
	at org.mozilla.gecko.BrowserApp.onActivityResult(BrowserApp.java:984)
	at android.app.Activity.dispatchActivityResult(Activity.java:5322)
	at android.app.ActivityThread.deliverResults(ActivityThread.java:3363)
	at android.app.ActivityThread.handleSendResult(ActivityThread.java:3410)
	at android.app.ActivityThread.access$1100(ActivityThread.java:141)
	at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1304)
	at android.os.Handler.dispatchMessage(Handler.java:99)
	at android.os.Looper.loop(Looper.java:137)
	at android.app.ActivityThread.main(ActivityThread.java:5103)
	at java.lang.reflect.Method.invokeNative(Native Method)
	at java.lang.reflect.Method.invoke(Method.java:525)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:737)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:553)
	at dalvik.system.NativeStart.main(Native Method)

More reports at:
https://crash-stats.mozilla.com/report/list?product=FennecAndroid&signature=java.lang.NullPointerException%3A+at+android.content.ContentResolver.acquireUnstableProvider%28ContentResolver.java%29

Comment 3

[Robert Kaiser]

Could you find out if this only affects trunk or if it affects older versions as well? Ideally we'd like a regression range on trunk, of course. :)

Comment 4

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

If we get a regression window, we can re-nom

Comment 5

[Teodora Vermesan (:TeoVermesan)]

I can reproduce this on 11/09 Nightly build, but I cannot reproduce this on latest Nightly 12/09 build.

Comment 6

[Aaron Train [:aaronmt]]

This still crashes for me on my Nexus 4 (Android 4.3) using today's 09/13's Nightly

https://crash-stats.mozilla.com/report/index/d4841a73-2b6f-438a-8875-caeda2130913

Comment 7

[binaryjohn]

I believe this is an OS issue:
1) Crash also occurs in mobile Chrome
2) Bug is not reproducible in either browser in OS 4.2.2

Comment 8

[Teodora Vermesan (:TeoVermesan)]

I still cannot reproduce this on LG Nexus 4 with Android 4.2.2 on 09/17's Nightly build.

Comment 9

[Robert Kaiser]

Those are the devices and OS versions (API levels) this signature has been sen with in the last 28 days:

Manufacturer 	Model 	API Version 	CPU ABI 	Report Count 	Percentage
LGE 	Nexus 4	18 (REL) 	armeabi-v7a 	10 	50.000 %
asus 	Nexus 7	18 (REL) 	armeabi-v7a 	7 	35.000 %
samsung 	Galaxy Nexus	18 (REL) 	armeabi-v7a 	2 	10.000 %
asus 	ME301T	17 (REL) 	armeabi-v7a 	1 	5.000 %

API level 18 is Android 4.3 and level 17 is 4.2, so it's pretty clear that this is not reproducible on Nexus devices with 4.2, but only with 4.3 so far.

Also, comment #5 gives an interesting hint on a Nightly regression range, but crash-stats say this happens with at least Firefox for Android 26, 24, and 23, and there's even one case in 20 in the last few weeks.

Comment 10

[Teodora Vermesan (:TeoVermesan)]

(In reply to Teodora Vermesan (:TeoVermesan) from comment #5)
> I can reproduce this on 11/09 Nightly build, but I cannot reproduce this on
> latest Nightly 12/09 build.

It was a Samsung Galaxy Nexus with Android 4.1.1

Comment 11

[Wesley Johnston (:wesj)]

Attachment: Patch 1/2

I'm guessing here that on some platforms we're getting back an intent with a null uri? That's possible, so we should probably check for it. This just fails if that's the case.

Comment 12

[Wesley Johnston (:wesj)]

Attachment: Patch 2/2

This doesn't fix anything, but I noticed acquireUnstablePovider is deprecated in favor of CursorLoaders, so this flips us to use them where we can.

Comment 13

[Lucas Rocha (:lucasr)]

Comment on attachment 818589 [details] [diff] [review]
Patch 1/2

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

Make sense.

::: mobile/android/base/CameraVideoResultHandler.java
@@ +40,5 @@
>      }
>  
>      @Override
>      public void onActivityResult(int resultCode, Intent data) {
> +        if (data == null || data.getData() == null || resultCode != Activity.RESULT_OK) {

It's probably worth adding a comment explaining the reason for the null check with a reference to this bug.

Comment 14

[Lucas Rocha (:lucasr)]

Comment on attachment 818591 [details] [diff] [review]
Patch 2/2

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

Looks nice, just needs to use support library instead.

::: mobile/android/base/CameraVideoResultHandler.java
@@ +8,5 @@
>  
>  import android.app.Activity;
> +import android.app.LoaderManager;
> +import android.content.CursorLoader;
> +import android.content.Loader;

You have to import these from the support library instead.

@@ +49,5 @@
>              sendResult("");
>              return;
>          }
>  
> +        if (android.os.Build.VERSION.SDK_INT >= 11) {

If you use the support library imports you can always use CursorLoader and remove the managedQuery stuff.

@@ +51,5 @@
>          }
>  
> +        if (android.os.Build.VERSION.SDK_INT >= 11) {
> +            final LoaderManager lm = GeckoAppShell.getGeckoInterface().getActivity().getLoaderManager();
> +            lm.initLoader(data.hashCode(), null, new LoaderManager.LoaderCallbacks<Cursor>() {

nit: maybe import LoaderCallbacks to make this code look a bit cleaner?

@@ +65,4 @@
>  
> +                @Override
> +                public void onLoadFinished(Loader<Cursor> loader, Cursor cursor) {
> +                    cursor.moveToFirst();

I wonder if you should be doing something like:

if (cursor.moveToFirst()) {
    sendResult(...)
}

To protect against empty cursors? Is an empty cursor a possible situation or a bug we want to catch as a crash?

Comment 15

[Wesley Johnston (:wesj)]

Attachment: Patch 2/2

This uses the compat libraries. The docs don't seem clear on whether the returned cursor can be null or not, but I added the check for it being empty.

Comment 16

[Wesley Johnston (:wesj)]

Comment on attachment 825459 [details] [diff] [review]
Patch 2/2

Lost something here....

Comment 17

[Wesley Johnston (:wesj)]

Attachment: Patch 2

Builds and runs :)

Comment 18

[Lucas Rocha (:lucasr)]

Comment on attachment 825502 [details] [diff] [review]
Patch 2

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

Yep.

::: mobile/android/base/CameraVideoResultHandler.java
@@ +51,5 @@
>              sendResult("");
>              return;
>          }
>  
> +        FragmentActivity fa = (FragmentActivity) GeckoAppShell.getGeckoInterface().getActivity();

final

@@ +56,5 @@
> +        final LoaderManager lm = fa.getSupportLoaderManager();
> +        lm.initLoader(data.hashCode(), null, new LoaderCallbacks<Cursor>() {
> +            @Override
> +            public Loader<Cursor> onCreateLoader(int id, Bundle args) {
> +                return new CursorLoader(GeckoAppShell.getGeckoInterface().getActivity(),

Just use fa?

@@ +62,5 @@
> +                                        new String[] { MediaStore.Video.Media.DATA },
> +                                        null,  // selection
> +                                        null,  // selectionArgs
> +                                        null); // sortOrder
> +            }

nit: empty line here?

Comment 19

[Wesley Johnston (:wesj)]

https://hg.mozilla.org/integration/fx-team/rev/4b72693d0856
https://hg.mozilla.org/integration/fx-team/rev/395f6bf32ca0

Comment 20

[Wesley Johnston (:wesj)]

Comment on attachment 818589 [details] [diff] [review]
Patch 1/2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Beginning of time?
User impact if declined: Possible crash. Hard to reproduce
Testing completed (on m-c, etc.): Landed on mc today. 
Risk to taking this patch (and alternatives if risky): Low risk. Just a null check. 
String or IDL/UUID changes made by this patch: None.

Comment 21

[Jim Mathies [:jimm]]

https://hg.mozilla.org/mozilla-central/rev/395f6bf32ca0

Comment 22

[Jim Mathies [:jimm]]

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

Comment 23

[(PTO June 19-July 7) Ryan VanderMeulen]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ee03911b8e1d
https://hg.mozilla.org/releases/mozilla-beta/rev/39efce81a714

Comment 24

[(PTO June 19-July 7) Ryan VanderMeulen]

https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/39efce81a714

Comment 25

[Flaviu Cos]

Verified as fixed on build: nightly 28.0a1 (11.14.2013).
Device: Nexus 7 (Android 4.3)

Comment 26

[Flaviu Cos]

Verified as fixed on builds: 
- aurora 27.0a2 (11.14.2013).
- beta 26.0 build 1
Device: Nexus 7 (Android 4.3)