1131635 - crash in java.lang.NullPointerException: at org.mozilla.gecko.home.RemoteTabsSplitPlaneFragment.updateUiFromClients(RemoteTabsSplitPlaneFragment.java)

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Aaron Train [:aaronmt]]

This bug was filed from the Socorro interface and is 
report bp-ccc377f0-fbbb-4235-bb77-2c3d12150209.
=============================================================

java.lang.NullPointerException
	at org.mozilla.gecko.home.RemoteTabsSplitPlaneFragment.updateUiFromClients(RemoteTabsSplitPlaneFragment.java:214)
	at org.mozilla.gecko.home.RemoteTabsBaseFragment$CursorLoaderCallbacks.onLoadFinishedAfterTransitions(RemoteTabsBaseFragment.java:226)
	at org.mozilla.gecko.home.TransitionAwareCursorLoaderCallbacks$OnLoadFinishedRunnable.run(TransitionAwareCursorLoaderCallbacks.java:53)
	at org.mozilla.gecko.animation.TransitionsTracker.runPendingActions(TransitionsTracker.java:66)
	at org.mozilla.gecko.animation.TransitionsTracker.popTransition(TransitionsTracker.java:86)
	at org.mozilla.gecko.animation.TransitionsTracker$2.onAnimationEnd(TransitionsTracker.java:49)
	at com.nineoldandroids.animation.AnimatorSet$AnimatorSetListener.onAnimationEnd(AnimatorSet.java:756)
	at com.nineoldandroids.animation.ValueAnimator.endAnimation(ValueAnimator.java:1034)
	at com.nineoldandroids.animation.ValueAnimator.access$900(ValueAnimator.java:43)
	at com.nineoldandroids.animation.ValueAnimator$AnimationHandler.handleMessage(ValueAnimator.java:669)
	at android.os.Handler.dispatchMessage(Handler.java:102)
	at android.os.Looper.loop(Looper.java:146)
	at android.app.ActivityThread.main(ActivityThread.java:5603)
	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:1283)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1099)
	at dalvik.system.NativeStart.main(Native Method)

Comment 1

[Nick Alexander :nalexander [he/him]]

This looks like we get an updateUiFromClients call *after* we get onDestroyView.  It's not clear to me what sequence of interactions would produce that: I hoped to avoid such things by having a single fragment with multiple views, so there was no inter-fragment life-cycle to manage.

It is technically possible that we couldn't find the client list -- perhaps we can guard against that in onCreateView?  This could be a small tablet/large phone/ROM issue.

vivek: does http://stackoverflow.com/a/23654224 cast any light?  Will you have time to work on this ticket this week?  I'm pretty sure I will not.  If we need to, we can always disable this for Firefox 38.

Comment 2

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

We have seen similar crashes in different parts of the code. The view can be destroyed before the animation completes. We typically try to check for some data that was nulled on onDestroy, or similar, as a way to bail out of the onAnimationEnd.

Comment 3

[Nick Alexander :nalexander [he/him]]

(In reply to Mark Finkle (:mfinkle) from comment #2)
> We have seen similar crashes in different parts of the code. The view can be
> destroyed before the animation completes. We typically try to check for some
> data that was nulled on onDestroy, or similar, as a way to bail out of the
> onAnimationEnd.

Thanks, Mark.  I remember doing something like this in the code, but I looked and didn't see it.

vivek: Perhaps just checking for mClientList (and others?) being null and bailing with a comment about the life-cycle?

Comment 4

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

I was remembering bug 1089653 and the followup, bug 1128521

Comment 5

[Vivek Balakrishnan[:vivek]]

Attachment: 1131635.patch

Added a check to return immediately if fragment was destroyed before the updateUiFromClients()

Comment 6

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8562925 [details] [diff] [review]
1131635.patch

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

Sure.

Comment 7

[Nick Alexander :nalexander [he/him]]

(In reply to Nick Alexander :nalexander from comment #6)
> Comment on attachment 8562925 [details] [diff] [review]
> 1131635.patch
> 
> Review of attachment 8562925 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Sure.

I'm going to land a slightly different work-around, consistent with what we do at https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/home/RemoteTabsPanel.java#228.

Comment 8

[Nick Alexander :nalexander [he/him]]

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

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7fd2b4ed2a60