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

RESOLVED FIXED in Firefox 38

Status

Firefox for Android Graveyard
Search Activity
--
critical
RESOLVED FIXED
4 years ago
4 months ago

People

(Reporter: aaronmt, Assigned: Margaret)

Tracking

({crash})

36 Branch
Firefox 38
All
Android
crash

Details

(crash signature)

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

4 years ago
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)
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.
Assignee: nobody → mark.finkle
tracking-fennec: ? → 36+
I don't know why I assigned this to myself. I didn't have the cycles to get a fix.
Assignee: mark.finkle → margaret.leibovic
(Assignee)

Comment 3

3 years ago
Created attachment 8556423 [details]
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
Attachment #8556423 - Flags: review?(mark.finkle)
Comment on attachment 8556423 [details]
MozReview Request: bz://1089653/margaret

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

Ship It!
Attachment #8556423 - Flags: review?(mark.finkle) → review+
https://hg.mozilla.org/mozilla-central/rev/89a045773847
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 38
(Assignee)

Comment 7

3 years ago
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
Attachment #8556423 - Flags: approval-mozilla-beta?
Attachment #8556423 - Flags: approval-mozilla-aurora?
status-firefox37: --- → affected
status-firefox38: --- → fixed
Attachment #8556423 - Flags: approval-mozilla-beta?
Attachment #8556423 - Flags: approval-mozilla-beta+
Attachment #8556423 - Flags: approval-mozilla-aurora?
Attachment #8556423 - Flags: approval-mozilla-aurora+
(Assignee)

Updated

3 years ago
Depends on: 1128521
(Assignee)

Comment 9

3 years ago
Comment on attachment 8556423 [details]
MozReview Request: bz://1089653/margaret
Attachment #8556423 - Attachment is obsolete: true
Attachment #8618474 - Flags: review+
(Assignee)

Comment 10

3 years ago
Created attachment 8618474 [details]
MozReview Request: Bug 1089653 - Make sure the search activity hasn't been destroyed before touching views in onAnimationEnd

Updated

4 months ago
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.