1354186 - AddonManager.jsm's typeListeners addonListeners and managerListeners arrays should be Sets

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, enhancement, P5)

Description

[Florian Quèze [:florian]]

The code currently uses surprisingly complicated code to check for duplicates in these arrays:

    if (!this.managerListeners.some(i => i == aListener))
      this.managerListeners.push(aListener);

And the removal code is also surprising:

    let pos = 0;
    while (pos < this.managerListeners.length) {
      if (this.managerListeners[pos] == aListener)
        this.managerListeners.splice(pos, 1);
      else
        pos++;
    }

Comment 1

[Robert Helmer [:rhelmer]]

I agree, this would be a decent thing to cleanup and shouldn't break any externally exposed APIs.

Comment 2

[Santiago Paez [:tiago]]

Attachment: Bug 1354186 - Change managerListeners, installListeners, addonListeners and typeListeners from Array to Set.

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

Comment 3

[Andrew Swan [:aswan]]

First glance looks good, I started a try run for you:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b59e686d4f9c

Comment 4

[Andrew Swan [:aswan]]

Comment on attachment 8869308 [details]
Bug 1354186 - Change managerListeners, installListeners, addonListeners and typeListeners from Array to Set.

https://reviewboard.mozilla.org/r/140862/#review144658

Looks good, thanks!

Comment 5

[Andy McKay]

Thanks for patch. Once you've got a review+, you can add in the keyword, "checkin-needed" and it will attempt to be merged to the tree. I've done this on your behalf, so we'll see how it goes.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/287690a1fedc
Change managerListeners, installListeners, addonListeners and typeListeners from Array to Set. r=aswan

Comment 7

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

https://hg.mozilla.org/mozilla-central/rev/287690a1fedc

Comment 8

[Caitlin Neiman [:caitmuenster]]

Thanks for the patch, Santiago! Your contribution has been added to our recognition wiki here: https://wiki.mozilla.org/Add-ons/Contribute/Recognition#June_2017

Your Mozillians profile has also been vouched. :) 

Welcome onboard! I look forward to seeing you around.