839854 - java.lang.NullPointerException: at org.mozilla.gecko.AboutHomeContent/TopSitesView$<n>.onActivityResult

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Scoobidiver (away)]

There are two crashes in 21.0a1, one in 21.0a1/20130207 and one in 21.0a1/20130209, bp-ce2d4c11-918b-48e5-9435-e8b352130210.
It's similar to bug 825505.

java.lang.NullPointerException
	at org.mozilla.gecko.AboutHomeContent$17.onActivityResult(AboutHomeContent.java:1050)
	at org.mozilla.gecko.ActivityHandlerHelper.handleActivityResult(ActivityHandlerHelper.java:224)
	at org.mozilla.gecko.GeckoApp.onActivityResult(GeckoApp.java:2492)
	at org.mozilla.gecko.BrowserApp.onActivityResult(BrowserApp.java:361)
	at android.app.Activity.dispatchActivityResult(Activity.java:5347)
	at android.app.ActivityThread.deliverResults(ActivityThread.java:3175)
	at android.app.ActivityThread.handleSendResult(ActivityThread.java:3222)
	at android.app.ActivityThread.access$1100(ActivityThread.java:140)
	at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1276)
	at android.os.Handler.dispatchMessage(Handler.java:99)
	at android.os.Looper.loop(Looper.java:137)
	at android.app.ActivityThread.main(ActivityThread.java:4895)
	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:994)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:761)
	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.AboutHomeContent%2417.onActivityResult%28AboutHomeContent.java%29

Comment 1

[Scoobidiver (away)]

It seems to be restricted to Nightly.

Comment 2

[Scoobidiver (away)]

(In reply to Scoobidiver from comment #1)
> It seems to be restricted to Nightly.
It occurs also on the Aurora channel.

Comment 3

[Chris Peterson [:cpeterson]]

This stack trace looks like mTopSitesGrid.getChildAt() is returning null here:

https://hg.mozilla.org/mozilla-central/annotate/5dbcbd03d7ba/mobile/android/base/AboutHomeContent.java#l1049

Some calls in AboutHomeContent.java check whether mTopSitesGrid.getChildAt() returns null because "the grid view might get temporarily out of sync with the adapter refreshes (e.g. on device rotation". Does that mean *all* calls to mTopSitesGrid.getChildAt() should check for null return?

Comment 5

[:Margaret Leibovic]

(In reply to Chris Peterson (:cpeterson) from comment #3)
> This stack trace looks like mTopSitesGrid.getChildAt() is returning null
> here:
> 
> https://hg.mozilla.org/mozilla-central/annotate/5dbcbd03d7ba/mobile/android/
> base/AboutHomeContent.java#l1049
> 
> Some calls in AboutHomeContent.java check whether mTopSitesGrid.getChildAt()
> returns null because "the grid view might get temporarily out of sync with
> the adapter refreshes (e.g. on device rotation". Does that mean *all* calls
> to mTopSitesGrid.getChildAt() should check for null return?

It sounds like a similar issue is happening in bug 863762 (and I think I've seen other similar sounding crashes in TopSitesView), so you may be right here.

Comment 6

[:Margaret Leibovic]

I started looking into this, but I haven't been able to reproduce (based on comment 3 I was trying to reproduce by rotating the device while editing a top site).

A null check on the result of getChildView() seems like a simple solution, but if we just bail in that case, editing that top site would fail, and that's a sucky experience. It would be better if we could figure out why getChildView() is retuning null, and see if we can do something to prevent that.

Comment 7

[Scoobidiver (away)]

It's currently #11 top crasher in 21.0 (was #49 in 21.0b8).
22.0a2 is unaffected and 23.0a1 stopped after 23.0a1/20130505. The working range is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c8e47b184aba&tochange=b109e2dbf03b

Comment 8

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

Since only fx21 (release version) is affected, I don't think we'd do much here.

Comment 9

[Scoobidiver (away)]

It seems Nightly and Aurora are still affected: #15 in 21.0, #1 in 23.0a2, #39 in 24.0a1.

Here is the stack trace:
java.lang.NullPointerException
	at org.mozilla.gecko.widget.TopSitesView$8.onActivityResult(TopSitesView.java:622)
	at org.mozilla.gecko.ActivityHandlerHelper.handleActivityResult(ActivityHandlerHelper.java:231)
	at org.mozilla.gecko.GeckoApp.onActivityResult(GeckoApp.java:2233)
	at org.mozilla.gecko.BrowserApp.onActivityResult(BrowserApp.java:824)
	at android.app.Activity.dispatchActivityResult(Activity.java:5275)
	at android.app.ActivityThread.deliverResults(ActivityThread.java:3618)
	at android.app.ActivityThread.handleSendResult(ActivityThread.java:3672)
	at android.app.ActivityThread.access$1100(ActivityThread.java:151)
	at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1379)
	at android.os.Handler.dispatchMessage(Handler.java:99)
	at android.os.Looper.loop(Looper.java:155)
	at android.app.ActivityThread.main(ActivityThread.java:5485)
	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:1028)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:795)
	at dalvik.system.NativeStart.main(Native Method)

See https://crash-stats.mozilla.com/report/list?signature=java.lang.NullPointerException%3A+at+org.mozilla.gecko.widget.TopSitesView%248.onActivityResult%28TopSitesView.java%29

Comment 10

[Scoobidiver (away)]

A comment says: "Viewing tabs".

Comment 11

[Scoobidiver (away)]

It's #28 crasher in 21.0, #19 in 23.0a2, and #47 in 24.0a1. 22.0 Beta is not affected.

Comment 12

[Lukas Blakk [:lsblakk] use ?needinfo]

Since this appears to be declining, untracking for 23.

Comment 13

[Scoobidiver (away)]

It's #1 top crasher in 23.0b1 (not yet released).

As stated in bug 862234, the new signature (widget.TopSitesView) first showed up in 23.0a1/20130409. In the worst case it occurs one build out of six so the regression range is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=b5cb88ccd907&tochange=b1fb34b07c17

Comment 14

[Scoobidiver (away)]

It's now #2 top crasher in 23.0a1 and also #1 in 24.0a2.

A comment says: "I added a page to the main menu, Mozilla has stopped working."

Comment 15

[Scoobidiver (away)]

(In reply to Scoobidiver from comment #14)
> in 23.0a1
I meant 23.0b1.

Comment 16

[Alex Keybl [:akeybl]]

Adding qawanted/steps-wanted to try related STR from comment 14.

Comment 17

[Scoobidiver (away)]

Here is the faulty line, https://mxr.mozilla.org/mozilla-beta/source/mobile/android/base/widget/TopSitesView.java#622, i.e. final TopSitesViewHolder holder = (TopSitesViewHolder) v.getTag().

Comment 18

[Wesley Johnston (:wesj)]

Attachment: Bandaid

I don't love doing this. Trying to think of some better way to reproduce or test this...

Comment 19

[Scoobidiver (away)]

Checked in?

Comment 20

[Wesley Johnston (:wesj)]

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

Comment 21

[Wesley Johnston (:wesj)]

landed the wrong version of this. backed out:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7997be475fe8

Comment 22

[Wesley Johnston (:wesj)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1277b7b3de0e

Comment 23

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1277b7b3de0e

Comment 24

[Scoobidiver (away)]

Uplift to Aurora and Beta?

Comment 25

[Wesley Johnston (:wesj)]

Comment on attachment 770299 [details] [diff] [review]
Bandaid

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Lots of bugs
User impact if declined: Crashes that are hard to reproduce or diagnose
Testing completed (on m-c, etc.): Landed on mc a month ago. Apparently we haven't seen crashes since.
Risk to taking this patch (and alternatives if risky): Low risk. Just adds a null check and bails. The risk is that in some cases, we may not show the correct pin site dialog. Failing silently seems better than crashing.
String or IDL/UUID changes made by this patch: none.

Comment 26

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 770299 [details] [diff] [review]
Bandaid

agreed.

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/1dffe1098d37
https://hg.mozilla.org/releases/mozilla-beta/rev/da80d09909f3