Closed
Bug 1354186
Opened 7 years ago
Closed 7 years ago
AddonManager.jsm's typeListeners addonListeners and managerListeners arrays should be Sets
Categories
(Toolkit :: Add-ons Manager, enhancement, P5)
Toolkit
Add-ons Manager
Tracking
()
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: florian, Assigned: tiago, Mentored)
References
Details
(Keywords: good-first-bug, Whiteboard: triaged)
Attachments
(1 file)
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•7 years ago
|
||
I agree, this would be a decent thing to cleanup and shouldn't break any externally exposed APIs.
Keywords: good-first-bug
Updated•7 years ago
|
Priority: -- → P5
Whiteboard: triaged
Updated•7 years ago
|
Mentor: aswan
Comment hidden (mozreview-request) |
Comment 3•7 years ago
|
||
First glance looks good, I started a try run for you: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b59e686d4f9c
Assignee: nobody → tiago.paez11
Comment 4•7 years ago
|
||
mozreview-review |
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!
Attachment #8869308 -
Flags: review?(aswan) → review+
Comment 5•7 years ago
|
||
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.
Keywords: checkin-needed
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/287690a1fedc Change managerListeners, installListeners, addonListeners and typeListeners from Array to Set. r=aswan
Keywords: checkin-needed
Comment 7•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/287690a1fedc
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Comment 8•7 years ago
|
||
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.
You need to log in
before you can comment on or make changes to this bug.
Description
•