Opened on 01/09/2019 at 03:21:48 PM
Closed on 01/11/2019 at 12:03:36 AM
Last modified on 03/12/2019 at 06:20:43 AM
#7202 closed change (fixed)
Add hasListeners method to EventEmitter
Reported by: | mjethani | Assignee: | mjethani |
---|---|---|---|
Priority: | P3 | Milestone: | |
Module: | Core | Keywords: | |
Cc: | Blocked By: | ||
Blocking: | Platform: | Unknown / Cross platform | |
Ready: | yes | Confidential: | no |
Tester: | Unknown | Verified working: | yes |
Review URL(s): |
Description (last modified by mjethani)
Background
The extension contains a hasListener(tabId) method in lib/hitLogger.js. This should be done by Core instead, in lib/events.js, by keeping track of whether an event has any listeners. Most of the time the HitLogger object in the extension does not have any listeners for any events, therefore this could be optimized in Core to avoid a lookup in the internal _listeners map, simply by checking the value of its size property first.
What to change
In lib/events.js modify EventEmitter.prototype.off to remove any entry that has no listeners left.
Add a new method hasListeners(name) that returns true if there are any filters for the given event or, if no name is given, for any event.
Integration notes
The extension should modify its lib/hitLogger.js to redefine the hasListener method as follows:
function hasListener(tabId) { return eventEmitter.hasListeners(tabId); }
Hints
The new hasListener method is not used by the extension as of now. Please see the dependency update ticket for this change for hints on how to test this.
Attachments (0)
Change History (15)
comment:1 Changed on 01/09/2019 at 03:22:00 PM by mjethani
- Component changed from Unknown to Core
comment:3 Changed on 01/09/2019 at 05:30:41 PM by mjethani
- Description modified (diff)
- Summary changed from Remove EventEmitter entries if there are no listeners left for an event to Add hasListeners method to EventEmitter
comment:6 Changed on 01/09/2019 at 06:13:01 PM by mjethani
- Blocking 7000 added
comment:7 Changed on 01/09/2019 at 10:08:42 PM by sebastian
comment:8 Changed on 01/09/2019 at 11:03:38 PM by mjethani
Thanks for the clarification.
As I mentioned in the description, we could still use this because right now the extension is accessing the internal property _listeners. But we don't need to make any more changes, just call eventEmitter.hasListeners() in lib/hitLogger.js.
comment:9 Changed on 01/10/2019 at 09:30:40 PM by abpbot
A commit referencing this issue has landed:
Issue 7202 - Add hasListeners method to EventEmitter
comment:10 Changed on 01/11/2019 at 12:00:18 AM by mjethani
- Description modified (diff)
- Priority changed from Unknown to P3
- Ready set
comment:11 Changed on 01/11/2019 at 12:03:36 AM by mjethani
- Description modified (diff)
- Resolution set to fixed
- Status changed from new to closed
comment:12 Changed on 01/11/2019 at 12:04:45 AM by mjethani
- Owner set to mjethani
comment:13 Changed on 01/15/2019 at 02:30:53 PM by mjethani
- Blocking 7000 removed
comment:14 Changed on 02/07/2019 at 03:24:01 AM by abpbot
A commit referencing this issue has landed:
Issue 7202 - Add hasListeners method to EventEmitter
comment:15 Changed on 03/12/2019 at 06:20:43 AM by rscott
- Verified working set
Thanks for filing this issue, and sorry for the confusion, but I don't think we need that after all.