From 0e65a4543e72a45b3efc90eaacc00a615989a8e0 Mon Sep 17 00:00:00 2001 From: Joey Parrish Date: Mon, 29 Apr 2019 17:07:48 -0700 Subject: [PATCH] Fix EventManager listenOnce with multiple listeners When listening to the same event on the same object from two places, it's important that both listeners get called back. This fixes EventManager's listenOnce() so that the unlisten() call within listenOnce() doesn't remove both listeners at once. Now each listener will be called before it is removed. This bug is over two years old! Change-Id: Id99f3a8e5ab80819921b30e28aa66d8a08b29e86 --- lib/util/event_manager.js | 16 ++- test/util/event_manager_unit.js | 222 ++++++++++++++++++-------------- 2 files changed, 138 insertions(+), 100 deletions(-) diff --git a/lib/util/event_manager.js b/lib/util/event_manager.js index 1fed11131..52babb3c6 100644 --- a/lib/util/event_manager.js +++ b/lib/util/event_manager.js @@ -79,12 +79,13 @@ shaka.util.EventManager.prototype.listen = function(target, type, listener) { shaka.util.EventManager.prototype.listenOnce = function(target, type, listener) { // Install a shim listener that will stop listening after the first event. - this.listen(target, type, function(event) { + const shim = (event) => { // Stop listening to this event. - this.unlisten(target, type); + this.unlisten(target, type, shim); // Call the original listener. listener(event); - }.bind(this)); + }; + this.listen(target, type, shim); }; @@ -92,8 +93,9 @@ shaka.util.EventManager.prototype.listenOnce = * Detaches an event listener from an event target. * @param {EventTarget} target The event target. * @param {string} type The event type. + * @param {shaka.util.EventManager.ListenerType=} listener The event listener. */ -shaka.util.EventManager.prototype.unlisten = function(target, type) { +shaka.util.EventManager.prototype.unlisten = function(target, type, listener) { if (!this.bindingMap_) return; let list = this.bindingMap_.get(type) || []; @@ -102,8 +104,10 @@ shaka.util.EventManager.prototype.unlisten = function(target, type) { let binding = list[i]; if (binding.target == target) { - binding.unlisten(); - this.bindingMap_.remove(type, binding); + if (listener == binding.listener || !listener) { + binding.unlisten(); + this.bindingMap_.remove(type, binding); + } } } }; diff --git a/test/util/event_manager_unit.js b/test/util/event_manager_unit.js index a504098fc..99c23ea07 100644 --- a/test/util/event_manager_unit.js +++ b/test/util/event_manager_unit.js @@ -45,122 +45,156 @@ describe('EventManager', function() { eventManager.release(); }); - it('listens for an event', function() { - let listener = jasmine.createSpy('listener'); + describe('listen', () => { + it('listens for an event', function() { + let listener = jasmine.createSpy('listener'); - eventManager.listen(target1, 'eventtype1', Util.spyFunc(listener)); - target1.dispatchEvent(event1); + eventManager.listen(target1, 'eventtype1', Util.spyFunc(listener)); + target1.dispatchEvent(event1); - expect(listener).toHaveBeenCalled(); + expect(listener).toHaveBeenCalled(); + }); + + it('listens for an event from mutiple targets', function() { + let listener1 = jasmine.createSpy('listener1'); + let listener2 = jasmine.createSpy('listener2'); + + eventManager.listen(target1, 'eventtype1', Util.spyFunc(listener1)); + eventManager.listen(target2, 'eventtype1', Util.spyFunc(listener2)); + + target1.dispatchEvent(event1); + target2.dispatchEvent(event1); + + expect(listener1).toHaveBeenCalled(); + expect(listener2).toHaveBeenCalled(); + }); + + it('listens for multiple events', function() { + let listener1 = jasmine.createSpy('listener1'); + let listener2 = jasmine.createSpy('listener2'); + + eventManager.listen(target1, 'eventtype1', Util.spyFunc(listener1)); + eventManager.listen(target1, 'eventtype2', Util.spyFunc(listener2)); + + target1.dispatchEvent(event1); + target1.dispatchEvent(event2); + + expect(listener1).toHaveBeenCalled(); + expect(listener2).toHaveBeenCalled(); + }); + + it('listens for multiple events from mutiple targets', function() { + let listener1 = jasmine.createSpy('listener1'); + let listener2 = jasmine.createSpy('listener2'); + + eventManager.listen(target1, 'eventtype1', Util.spyFunc(listener1)); + eventManager.listen(target2, 'eventtype2', Util.spyFunc(listener2)); + + target1.dispatchEvent(event1); + target2.dispatchEvent(event2); + + expect(listener1).toHaveBeenCalled(); + expect(listener2).toHaveBeenCalled(); + }); + + it('listens for an event with multiple listeners', function() { + let listener1 = jasmine.createSpy('listener1'); + let listener2 = jasmine.createSpy('listener2'); + + eventManager.listen(target1, 'eventtype1', Util.spyFunc(listener1)); + eventManager.listen(target1, 'eventtype1', Util.spyFunc(listener2)); + + target1.dispatchEvent(event1); + + expect(listener1).toHaveBeenCalled(); + expect(listener2).toHaveBeenCalled(); + }); }); - it('listens for an event from mutiple targets', function() { - let listener1 = jasmine.createSpy('listener1'); - let listener2 = jasmine.createSpy('listener2'); + describe('listenOnce', () => { + it('listens to an event only once', () => { + const listener1 = jasmine.createSpy('listener1'); - eventManager.listen(target1, 'eventtype1', Util.spyFunc(listener1)); - eventManager.listen(target2, 'eventtype1', Util.spyFunc(listener2)); + eventManager.listenOnce(target1, 'eventtype1', Util.spyFunc(listener1)); - target1.dispatchEvent(event1); - target2.dispatchEvent(event1); + target1.dispatchEvent(event1); + expect(listener1).toHaveBeenCalled(); + listener1.calls.reset(); - expect(listener1).toHaveBeenCalled(); - expect(listener2).toHaveBeenCalled(); + target1.dispatchEvent(event1); + expect(listener1).not.toHaveBeenCalled(); + }); + + it('listens to an event with multiple listeners', () => { + const listener1 = jasmine.createSpy('listener1'); + const listener2 = jasmine.createSpy('listener2'); + + eventManager.listenOnce(target1, 'eventtype1', Util.spyFunc(listener1)); + eventManager.listenOnce(target1, 'eventtype1', Util.spyFunc(listener2)); + + target1.dispatchEvent(event1); + + expect(listener1).toHaveBeenCalled(); + expect(listener2).toHaveBeenCalled(); + }); }); - it('listens for multiple events', function() { - let listener1 = jasmine.createSpy('listener1'); - let listener2 = jasmine.createSpy('listener2'); + describe('unlisten', () => { + it('stops listening to an event', function() { + let listener = jasmine.createSpy('listener'); - eventManager.listen(target1, 'eventtype1', Util.spyFunc(listener1)); - eventManager.listen(target1, 'eventtype2', Util.spyFunc(listener2)); + eventManager.listen(target1, 'eventtype1', Util.spyFunc(listener)); + eventManager.unlisten(target1, 'eventtype1'); - target1.dispatchEvent(event1); - target1.dispatchEvent(event2); + target1.dispatchEvent(event1); - expect(listener1).toHaveBeenCalled(); - expect(listener2).toHaveBeenCalled(); + expect(listener).not.toHaveBeenCalled(); + }); + + it('ignores other targets when removing listeners', function() { + let listener1 = jasmine.createSpy('listener1'); + let listener2 = jasmine.createSpy('listener2'); + + eventManager.listen(target1, 'eventtype1', Util.spyFunc(listener1)); + eventManager.listen(target2, 'eventtype1', Util.spyFunc(listener2)); + eventManager.unlisten(target2, 'eventtype1'); + + target1.dispatchEvent(event1); + + expect(listener1).toHaveBeenCalled(); + }); }); - it('listens for multiple events from mutiple targets', function() { - let listener1 = jasmine.createSpy('listener1'); - let listener2 = jasmine.createSpy('listener2'); + describe('removeAll', () => { + it('stops listening to multiple events', function() { + let listener1 = jasmine.createSpy('listener1'); + let listener2 = jasmine.createSpy('listener2'); - eventManager.listen(target1, 'eventtype1', Util.spyFunc(listener1)); - eventManager.listen(target2, 'eventtype2', Util.spyFunc(listener2)); + eventManager.listen(target1, 'eventtype1', Util.spyFunc(listener1)); + eventManager.listen(target1, 'eventtype2', Util.spyFunc(listener2)); - target1.dispatchEvent(event1); - target2.dispatchEvent(event2); + eventManager.removeAll(); - expect(listener1).toHaveBeenCalled(); - expect(listener2).toHaveBeenCalled(); - }); + target1.dispatchEvent(event1); + target1.dispatchEvent(event2); - it('listens for an event with multiple listeners', function() { - let listener1 = jasmine.createSpy('listener1'); - let listener2 = jasmine.createSpy('listener2'); + expect(listener1).not.toHaveBeenCalled(); + expect(listener2).not.toHaveBeenCalled(); + }); - eventManager.listen(target1, 'eventtype1', Util.spyFunc(listener1)); - eventManager.listen(target1, 'eventtype1', Util.spyFunc(listener2)); + it('stops listening for an event with multiple listeners', function() { + let listener1 = jasmine.createSpy('listener1'); + let listener2 = jasmine.createSpy('listener2'); - target1.dispatchEvent(event1); + eventManager.listen(target1, 'eventtype1', Util.spyFunc(listener1)); + eventManager.listen(target1, 'eventtype1', Util.spyFunc(listener2)); - expect(listener1).toHaveBeenCalled(); - expect(listener2).toHaveBeenCalled(); - }); + eventManager.removeAll(); - it('stops listening to an event', function() { - let listener = jasmine.createSpy('listener'); + target1.dispatchEvent(event1); - eventManager.listen(target1, 'eventtype1', Util.spyFunc(listener)); - eventManager.unlisten(target1, 'eventtype1'); - - target1.dispatchEvent(event1); - - expect(listener).not.toHaveBeenCalled(); - }); - - it('ignores other targets when removing listeners', function() { - let listener1 = jasmine.createSpy('listener1'); - let listener2 = jasmine.createSpy('listener2'); - - eventManager.listen(target1, 'eventtype1', Util.spyFunc(listener1)); - eventManager.listen(target2, 'eventtype1', Util.spyFunc(listener2)); - eventManager.unlisten(target2, 'eventtype1'); - - target1.dispatchEvent(event1); - - expect(listener1).toHaveBeenCalled(); - }); - - it('stops listening to multiple events', function() { - let listener1 = jasmine.createSpy('listener1'); - let listener2 = jasmine.createSpy('listener2'); - - eventManager.listen(target1, 'eventtype1', Util.spyFunc(listener1)); - eventManager.listen(target1, 'eventtype2', Util.spyFunc(listener2)); - - eventManager.removeAll(); - - target1.dispatchEvent(event1); - target1.dispatchEvent(event2); - - expect(listener1).not.toHaveBeenCalled(); - expect(listener2).not.toHaveBeenCalled(); - }); - - it('stops listening for an event with multiple listeners', function() { - let listener1 = jasmine.createSpy('listener1'); - let listener2 = jasmine.createSpy('listener2'); - - eventManager.listen(target1, 'eventtype1', Util.spyFunc(listener1)); - eventManager.listen(target1, 'eventtype1', Util.spyFunc(listener2)); - - eventManager.removeAll(); - - target1.dispatchEvent(event1); - - expect(listener1).not.toHaveBeenCalled(); - expect(listener2).not.toHaveBeenCalled(); + expect(listener1).not.toHaveBeenCalled(); + expect(listener2).not.toHaveBeenCalled(); + }); }); });