1089653 - crash in java.lang.NullPointerException: at org.mozilla.search.SearchActivity.updateSettingsButtonVisibility(SearchActivity.java)

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Search Activity, defect)

Description

[Aaron Train [:aaronmt]]

This bug was filed from the Socorro interface and is 
report bp-6d187090-4cc4-4672-8dae-adb882141026.
=============================================================

java.lang.NullPointerException
	at org.mozilla.search.SearchActivity.updateSettingsButtonVisibility(SearchActivity.java:384)
	at org.mozilla.search.SearchActivity.setSearchState(SearchActivity.java:373)
	at org.mozilla.search.SearchActivity.access$400(SearchActivity.java:40)
	at org.mozilla.search.SearchActivity$6.onAnimationEnd(SearchActivity.java:333)
	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:157)
	at android.app.ActivityThread.main(ActivityThread.java:5335)
	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:1265)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1081)
	at dalvik.system.NativeStart.main(Native Method)

Comment 1

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

The line numbers seem a little off (SearchActivity.setSearchState calls updateSettingsButtonVisibility on line 376), but it seems like settingsButton is null on line 387:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/search/java/org/mozilla/search/SearchActivity.java#387

I see this callstack is happening during an onAnimationEnd, and settingsButton is set to null in onDestroy leading me to think the animation is ending after the activity is destroyed. I think a simple null check in updateSettingsButtonVisibility should be enough.

But i see that animationCard is accessed in onAnimationEnd too, and that will throw an NPE as well.

I suppose we should either add some check in onAnimationEnd or just wrap it in an exception handler.

Comment 2

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

I don't know why I assigned this to myself. I didn't have the cycles to get a fix.

Comment 3

[:Margaret Leibovic]

Attachment: MozReview Request: bz://1089653/margaret

/r/3125 - Bug 1089653 - Make sure the search activity hasn't been destroyed before touching views in onAnimationEnd

Pull down this commit:

hg pull review -r 468ac02a87a348e01e5b8eaa9f1f508849be937a

Comment 4

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

Comment on attachment 8556423 [details]
MozReview Request: bz://1089653/margaret

https://reviewboard.mozilla.org/r/3123/#review2487

Ship It!

Comment 5

[:Margaret Leibovic]

https://hg.mozilla.org/integration/fx-team/rev/89a045773847

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/89a045773847

Comment 7

[:Margaret Leibovic]

Comment on attachment 8556423 [details]
MozReview Request: bz://1089653/margaret

Approval Request Comment
[Feature/regressing bug #]: search activity (shipped in 35)
[User impact if declined]: possible crashes
[Describe test coverage new/current, TreeHerder]: no automated testing, tested locally (but couldn't reproduce the crash originally, so uncertain as to the effectiveness)
[Risks and why]: low-risk, just adds a check to make sure views are around before referencing them
[String/UUID change made/needed]: none

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/afa9f14823bc
https://hg.mozilla.org/releases/mozilla-beta/rev/734264bcd6d1

Comment 10

[:Margaret Leibovic]

Attachment: MozReview Request: Bug 1089653 - Make sure the search activity hasn't been destroyed before touching views in onAnimationEnd