847834 - java.lang.NullPointerException: at org.mozilla.gecko.BrowserApp.onActivityResult(BrowserApp.java)

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Scoobidiver (away)]

There are two crashes from the same user in 22.0a1/20130304, including bp-63c6bf69-2087-4736-a312-e2d202130305.

java.lang.NullPointerException
	at org.mozilla.gecko.BrowserApp.onActivityResult(BrowserApp.java:339)
	at android.app.Activity.dispatchActivityResult(Activity.java:5293)
	at android.app.ActivityThread.deliverResults(ActivityThread.java:3439)
	at android.app.ActivityThread.handleSendResult(ActivityThread.java:3486)
	at android.app.ActivityThread.access$1100(ActivityThread.java:150)
	at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1292)
	at android.os.Handler.dispatchMessage(Handler.java:99)
	at android.os.Looper.loop(Looper.java:137)
	at android.app.ActivityThread.main(ActivityThread.java:5193)
	at java.lang.reflect.Method.invokeNative(Native Method)
	at java.lang.reflect.Method.invoke(Method.java:511)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:795)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:562)
	at dalvik.system.NativeStart.main(Native Method)

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

Comment 1

[Scoobidiver (away)]

The regression range might be:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=3362afba690e&tochange=86c98c4d36da

Comment 3

[Kevin Brosnan [Ex-Mozilla]]

From comment 0

Firefox crashes on switching back from camera app.  Reproduced on Firefox nightly on S2 running jelly bean.

Testcase:
1. Open http://html5.merge.nl/file-api.php and click on browse button
2. Choose camera and take a picture 
3. Select the picture and the browser should crash

Comment 4

[Scoobidiver (away)]

It's #6 top crasher in 22.0a1.

Comment 5

[Scoobidiver (away)]

It's #46 crasher in 22.0a2 and #15 in 23.0a1.

Comment 6

[Tracy Walker [:tracy]]

#20 crasher on 23
#60 crasher on 22

Comment 7

[Scoobidiver (away)]

There's a spike in crashes starting in 23.0a1/20130418. The regression range for the spike is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=50ab959f4bd1&tochange=3ada6a2fd0c6

Comment 8

[Scoobidiver (away)]

It spiked also in 22.0a2/20130418. The Aurora regression range for the spike is:
http://hg.mozilla.org/releases/mozilla-aurora/pushloghtml?fromchange=f342569a36f2&tochange=071e2690ff7a
There are two possible candidates that belong to both ranges: bug 858323 and bug 860326.

Comment 9

[Scoobidiver (away)]

Note that after these sharp spikes, it's now #1 top crasher in 22.0a2 and #4 in 23.0a1 over the last three days.

Comment 10

[:Margaret Leibovic]

I'll look into this.

Comment 11

[:Margaret Leibovic]

Attachment: patch

Apparently null checks are hard. The fact that there were crashes before my patch in bug 860326 makes me think that data can be null even if resultCode == Activity.RESULT_OK, so we should check that.

I broke up the logic a bit more here to try to make it all really clear what's going on.

Also, this makes me realize that the fact that we always call fromAwesomeBarSearch() is wrong, since we go through this code path for any activity result, not just the awesome screen. We can address that in a follow-up though.

Comment 12

[:Margaret Leibovic]

It looks like this is originally a regression from bug 839602, and it appears to have been exacerbated by bug 860326 .

Comment 13

[:Margaret Leibovic]

Attachment: better patch

Comment 14

[:Margaret Leibovic]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9cd4b8d02649

Comment 15

[Ryan VanderMeulen [:RyanVM]]

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

Comment 16

[:Margaret Leibovic]

Comment on attachment 740991 [details] [diff] [review]
better patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 839602 (exacerbated by bug 860326)
User impact if declined: crashes
Testing completed (on m-c, etc.): landed on m-c 
Risk to taking this patch (and alternatives if risky): I thought the other patches were low-risk, and they caused this crash! The main risk here is that the crash might not be totally fixed, but I think it should work.
String or IDL/UUID changes made by this patch: n/a

Comment 17

[bhavana bajaj [:bajaj]]

Comment on attachment 740991 [details] [diff] [review]
better patch

looks ok to land as we still have a few weeks on aurora.Lets keep an eye on the crash-volume to see if the crashes go away else we may have to perform a backout here if needed.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/f11233f17239

Comment 19

[Cristina Madaras, QA [:CristinaM]]

I can't verify this bug because the page http://html5.merge.nl/file-api.php has been changed, so I'm not able to reproduce the steps from comment 3.