1296748 - Crash in java.lang.IllegalArgumentException: TextSelection:ActionbarInit was not registered at org.mozilla.gecko.EventDispatcher.unregisterListener(EventDispatcher.java)

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[(inactive) Jim Chen [:jchen] [:darchons]]

This bug was filed from the Socorro interface and is 
report bp-fc718753-1ca8-40dd-af83-4e4e92160819.
=============================================================

Low volume crash (7 total over last 7 days) that first appeared in the 8-14 Nightly. Looking at [1], we don't register listeners when `anchorHandle == null`, so we shouldn't unregister listeners either in that case.

[1] https://hg.mozilla.org/mozilla-central/file/19222e86fb30/mobile/android/base/java/org/mozilla/gecko/ActionBarTextSelection.java#l67

Comment 1

[kevin]

Hi Jimmy I would like to try working on this as my first bug.

Comment 2

[Julian Chu [:walkingice]]

Hi I am also interesting in this issue.

Just curious, to check existence of anchorHandle could around this issue.

However, should it just throw exception in create()? Because other parts of code assumes anchorHandler exists, to throw exception as early as possible might be better?

Comment 3

[(inactive) Jim Chen [:jchen] [:darchons]]

(In reply to Julian Chu from comment #2)
> Hi I am also interesting in this issue.
> 
> Just curious, to check existence of anchorHandle could around this issue.
> 
> However, should it just throw exception in create()? Because other parts of
> code assumes anchorHandler exists, to throw exception as early as possible
> might be better?

Thanks for the interest Julian! The other parts of the code will not be run if anchorHandler is null, so the only thing you have to change is inside destroy().

Comment 4

[(inactive) Jim Chen [:jchen] [:darchons]]

(In reply to kevin from comment #1)
> Hi Jimmy I would like to try working on this as my first bug.

Hi Kevin, looks like you're working on bug 1283704, so I'll assign this bug to Julian instead. Thanks for your interest!

Comment 5

[Julian Chu [:walkingice]]

Attachment: Bug 1296748 - Do not unregister if target does not exist;

in create(), only register listener if anchorHandle exits.
in opposite side, only unregister listener in same condition.

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

Comment 6

[Julian Chu [:walkingice]]

Hi Jim, thanks for your reply. I had created a patch as well as review request. Please correct me if I did anything wrong. Thanks.

https://reviewboard.mozilla.org/r/74988/

Comment 7

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8786001 [details]
Bug 1296748 - Do not unregister if target does not exist;

https://reviewboard.mozilla.org/r/74990/#review72866

Looks good. Thanks! Do you know what the next step is to get your patch checked in?

Comment 8

[Julian Chu [:walkingice]]

Comment on attachment 8786001 [details]
Bug 1296748 - Do not unregister if target does not exist;

https://reviewboard.mozilla.org/r/74990/#review72866

Actually I don't know. Could you please tell me what to do, or point me out which documentation I should read. Thanks a lot!

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/11943e518a67
Do not unregister if target does not exist; r=jchen

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/11943e518a67

Comment 11

[(inactive) Jim Chen [:jchen] [:darchons]]

Sorry for not responding sooner, Julian! Looks like you found what to do though. Thanks for fixing the bug! :)

Comment 12

[Julian Chu [:walkingice]]

No problem. It's my pleasure to make a little contribution. Thanks for your kindly suggestion.