1239787 - crash in java.lang.NullPointerException: at org.mozilla.gecko.tabqueue.TabQueueService$3.onClick(TabQueueService.java)

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Overlays, defect)

Description

[Sebastian Kaspari (:sebastian; :pocmo)]

This bug was filed from the Socorro interface and is 
report bp-18b5d974-9862-4c22-81e8-d243f2160113.
=============================================================

e.g.: https://crash-stats.mozilla.com/report/index/18b5d974-9862-4c22-81e8-d243f2160113

Comment 1

[Sebastian Kaspari (:sebastian; :pocmo)]

> java.lang.NullPointerException
>     at org.mozilla.gecko.tabqueue.TabQueueService$3.onClick(TabQueueService.java:187)

Comment 2

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: MozReview Request: Bug 1239787 - TabQueueService: Do not clear tabQueueHandler reference. r?ahunt

Review commit: https://reviewboard.mozilla.org/r/31071/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/31071/

Comment 3

[Sebastian Kaspari (:sebastian; :pocmo)]

https://reviewboard.mozilla.org/r/31071/#review27901

::: mobile/android/base/java/org/mozilla/gecko/tabqueue/TabQueueService.java
(Diff revision 1)
> -        tabQueueHandler = null;

I don't see the need to clear the Handler reference in onDestroy. The reference is not static so it's going away with the service instance anyways. Currently it can happen that we clear the reference and at the same time the user clicks on "Open" (OnClickListener implementation is accessing the Handler).

Comment 4

[Andrzej Hunt :ahunt]

Comment on attachment 8708511 [details]
MozReview Request: Bug 1239787 - TabQueueService: Do not clear tabQueueHandler reference. r?ahunt

https://reviewboard.mozilla.org/r/31071/#review27923

Looks good - that does seem unnecessary!

Comment 5

[Sebastian Kaspari (:sebastian; :pocmo)]

https://hg.mozilla.org/integration/fx-team/rev/38ba884086c7411f7b20cc3023e038939e1ced3d
Bug 1239787 - TabQueueService: Do not clear tabQueueHandler reference. r=ahunt

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/38ba884086c7

Comment 7

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8708511 [details]
MozReview Request: Bug 1239787 - TabQueueService: Do not clear tabQueueHandler reference. r?ahunt

Approval Request Comment

[Feature/regressing bug #]: Saw this bug in our crashstats and fixed it. Requesting uplift to lower the crash rate. Crash reports: https://crash-stats.mozilla.com/report/list?range_unit=days&range_value=28&signature=java.lang.NullPointerException%3A+at+org.mozilla.gecko.tabqueue.TabQueueService%243.onClick%28TabQueueService.java%29#tab-reports

[User impact if declined]: With tab queues the app can crash after the user clicked on "OPEN" in the tab queue overlay.

[Describe test coverage new/current, TreeHerder]: This is not easy reproducible because you need to time the click exactly to trigger this NullPointerException (With lots of users this statistically happens nevertheless).

[Risks and why]: Low - The patch avoid clearing the reference to early causing the NullPointerException.

[String/UUID change made/needed]: -

Comment 8

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8708511 [details]
MozReview Request: Bug 1239787 - TabQueueService: Do not clear tabQueueHandler reference. r?ahunt

One-liner Fennec crash fix. Taking it in Fennec 44.0 RC1

Comment 9

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

https://hg.mozilla.org/releases/mozilla-release/rev/e2b8b3494e56

Comment 10

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8708511 [details]
MozReview Request: Bug 1239787 - TabQueueService: Do not clear tabQueueHandler reference. r?ahunt

Just like 44!

Comment 11

[Carsten Book [:Tomcat]]

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