Skip to content

Commit

Permalink
Fix EventManager listenOnce with multiple listeners
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@joeyparrish
joeyparrish committed Apr 30, 2019
1 parent 91c57e1 commit 0e65a45
Show file tree
Hide file tree
Showing 2 changed files with 122 additions and 84 deletions.
16 changes: 10 additions & 6 deletions lib/util/event_manager.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,21 +79,23 @@ 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);
};


/**
* 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) || [];
Expand All @@ -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);
}
}
}
};
Expand Down
190 changes: 112 additions & 78 deletions test/util/event_manager_unit.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
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));
eventManager.listen(target1, 'eventtype1', Util.spyFunc(listener1));
eventManager.listen(target2, 'eventtype1', Util.spyFunc(listener2));

target1.dispatchEvent(event1);
target2.dispatchEvent(event1);
target1.dispatchEvent(event1);
target2.dispatchEvent(event1);

expect(listener1).toHaveBeenCalled();
expect(listener2).toHaveBeenCalled();
});
expect(listener1).toHaveBeenCalled();
expect(listener2).toHaveBeenCalled();
});

it('listens for multiple events', function() {
let listener1 = jasmine.createSpy('listener1');
let listener2 = jasmine.createSpy('listener2');
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));
eventManager.listen(target1, 'eventtype1', Util.spyFunc(listener1));
eventManager.listen(target1, 'eventtype2', Util.spyFunc(listener2));

target1.dispatchEvent(event1);
target1.dispatchEvent(event2);
target1.dispatchEvent(event1);
target1.dispatchEvent(event2);

expect(listener1).toHaveBeenCalled();
expect(listener2).toHaveBeenCalled();
});
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');
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));
eventManager.listen(target1, 'eventtype1', Util.spyFunc(listener1));
eventManager.listen(target2, 'eventtype2', Util.spyFunc(listener2));

target1.dispatchEvent(event1);
target2.dispatchEvent(event2);
target1.dispatchEvent(event1);
target2.dispatchEvent(event2);

expect(listener1).toHaveBeenCalled();
expect(listener2).toHaveBeenCalled();
});
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');
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));
eventManager.listen(target1, 'eventtype1', Util.spyFunc(listener1));
eventManager.listen(target1, 'eventtype1', Util.spyFunc(listener2));

target1.dispatchEvent(event1);
target1.dispatchEvent(event1);

expect(listener1).toHaveBeenCalled();
expect(listener2).toHaveBeenCalled();
expect(listener1).toHaveBeenCalled();
expect(listener2).toHaveBeenCalled();
});
});

it('stops listening to an event', function() {
let listener = jasmine.createSpy('listener');
describe('listenOnce', () => {
it('listens to an event only once', () => {
const listener1 = jasmine.createSpy('listener1');

eventManager.listen(target1, 'eventtype1', Util.spyFunc(listener));
eventManager.unlisten(target1, 'eventtype1');
eventManager.listenOnce(target1, 'eventtype1', Util.spyFunc(listener1));

target1.dispatchEvent(event1);
target1.dispatchEvent(event1);
expect(listener1).toHaveBeenCalled();
listener1.calls.reset();

expect(listener).not.toHaveBeenCalled();
});
target1.dispatchEvent(event1);
expect(listener1).not.toHaveBeenCalled();
});

it('ignores other targets when removing listeners', function() {
let listener1 = jasmine.createSpy('listener1');
let listener2 = jasmine.createSpy('listener2');
it('listens to an event with multiple listeners', () => {
const listener1 = jasmine.createSpy('listener1');
const listener2 = jasmine.createSpy('listener2');

eventManager.listen(target1, 'eventtype1', Util.spyFunc(listener1));
eventManager.listen(target2, 'eventtype1', Util.spyFunc(listener2));
eventManager.unlisten(target2, 'eventtype1');
eventManager.listenOnce(target1, 'eventtype1', Util.spyFunc(listener1));
eventManager.listenOnce(target1, 'eventtype1', Util.spyFunc(listener2));

target1.dispatchEvent(event1);
target1.dispatchEvent(event1);

expect(listener1).toHaveBeenCalled();
expect(listener1).toHaveBeenCalled();
expect(listener2).toHaveBeenCalled();
});
});

it('stops listening to 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(listener));
eventManager.unlisten(target1, 'eventtype1');

target1.dispatchEvent(event1);

eventManager.listen(target1, 'eventtype1', Util.spyFunc(listener1));
eventManager.listen(target1, 'eventtype2', Util.spyFunc(listener2));
expect(listener).not.toHaveBeenCalled();
});

eventManager.removeAll();
it('ignores other targets when removing listeners', function() {
let listener1 = jasmine.createSpy('listener1');
let listener2 = jasmine.createSpy('listener2');

target1.dispatchEvent(event1);
target1.dispatchEvent(event2);
eventManager.listen(target1, 'eventtype1', Util.spyFunc(listener1));
eventManager.listen(target2, 'eventtype1', Util.spyFunc(listener2));
eventManager.unlisten(target2, 'eventtype1');

expect(listener1).not.toHaveBeenCalled();
expect(listener2).not.toHaveBeenCalled();
target1.dispatchEvent(event1);

expect(listener1).toHaveBeenCalled();
});
});

it('stops listening for an event with multiple listeners', 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(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.listen(target1, 'eventtype1', Util.spyFunc(listener1));
eventManager.listen(target1, 'eventtype1', Util.spyFunc(listener2));

eventManager.removeAll();
eventManager.removeAll();

target1.dispatchEvent(event1);
target1.dispatchEvent(event1);

expect(listener1).not.toHaveBeenCalled();
expect(listener2).not.toHaveBeenCalled();
expect(listener1).not.toHaveBeenCalled();
expect(listener2).not.toHaveBeenCalled();
});
});
});

0 comments on commit 0e65a45

Please sign in to comment.