From ef8d0e9798d3e6412ad5f8417a32275d8181bbe7 Mon Sep 17 00:00:00 2001 From: Benjamin Gruenbaum Date: Wed, 25 Nov 2020 14:58:58 +0200 Subject: [PATCH] events: support signal in EventTarget PR-URL: https://github.com/nodejs/node/pull/36258 Fixes: https://github.com/nodejs/node/issues/36073 Reviewed-By: James M Snell --- lib/internal/event_target.js | 22 +++ .../test-eventtarget-whatwg-signal.js | 155 ++++++++++++++++++ test/parallel/test-eventtarget.js | 9 + 3 files changed, 186 insertions(+) create mode 100644 test/parallel/test-eventtarget-whatwg-signal.js diff --git a/lib/internal/event_target.js b/lib/internal/event_target.js index 1f6ae1c17c8317..c4452b67cf424c 100644 --- a/lib/internal/event_target.js +++ b/lib/internal/event_target.js @@ -200,10 +200,12 @@ class Listener { previous.next = this; this.previous = previous; this.listener = listener; + // TODO(benjamingr) these 4 can be 'flags' to save 3 slots this.once = once; this.capture = capture; this.passive = passive; this.isNodeStyleListener = isNodeStyleListener; + this.removed = false; this.callback = typeof listener === 'function' ? @@ -220,6 +222,7 @@ class Listener { this.previous.next = this.next; if (this.next !== undefined) this.next.previous = this.previous; + this.removed = true; } } @@ -269,6 +272,7 @@ class EventTarget { once, capture, passive, + signal, isNodeStyleListener } = validateEventListenerOptions(options); @@ -286,6 +290,17 @@ class EventTarget { } type = String(type); + if (signal) { + if (signal.aborted) { + return false; + } + // TODO(benjamingr) make this weak somehow? ideally the signal would + // not prevent the event target from GC. + signal.addEventListener('abort', () => { + this.removeEventListener(type, listener, options); + }, { once: true }); + } + let root = this[kEvents].get(type); if (root === undefined) { @@ -382,6 +397,12 @@ class EventTarget { // Cache the next item in case this iteration removes the current one next = handler.next; + if (handler.removed) { + // Deal with the case an event is removed while event handlers are + // Being processed (removeEventListener called from a listener) + handler = next; + continue; + } if (handler.once) { handler.remove(); root.size--; @@ -550,6 +571,7 @@ function validateEventListenerOptions(options) { once: Boolean(options.once), capture: Boolean(options.capture), passive: Boolean(options.passive), + signal: options.signal, isNodeStyleListener: Boolean(options[kIsNodeStyleListener]) }; } diff --git a/test/parallel/test-eventtarget-whatwg-signal.js b/test/parallel/test-eventtarget-whatwg-signal.js new file mode 100644 index 00000000000000..8573d2b4aea5bb --- /dev/null +++ b/test/parallel/test-eventtarget-whatwg-signal.js @@ -0,0 +1,155 @@ +'use strict'; + +require('../common'); + +const { + strictEqual, +} = require('assert'); + +// Manually ported from: wpt@dom/events/AddEventListenerOptions-signal.any.js + +{ + // Passing an AbortSignal to addEventListener does not prevent + // removeEventListener + let count = 0; + function handler() { + count++; + } + const et = new EventTarget(); + const controller = new AbortController(); + et.addEventListener('test', handler, { signal: controller.signal }); + et.dispatchEvent(new Event('test')); + strictEqual(count, 1, 'Adding a signal still adds a listener'); + et.dispatchEvent(new Event('test')); + strictEqual(count, 2, 'The listener was not added with the once flag'); + controller.abort(); + et.dispatchEvent(new Event('test')); + strictEqual(count, 2, 'Aborting on the controller removes the listener'); + et.addEventListener('test', handler, { signal: controller.signal }); + et.dispatchEvent(new Event('test')); + strictEqual(count, 2, 'Passing an aborted signal never adds the handler'); +} + +{ + // Passing an AbortSignal to addEventListener works with the once flag + let count = 0; + function handler() { + count++; + } + const et = new EventTarget(); + const controller = new AbortController(); + et.addEventListener('test', handler, { signal: controller.signal }); + et.removeEventListener('test', handler); + et.dispatchEvent(new Event('test')); + strictEqual(count, 0, 'The listener was still removed'); +} + +{ + // Removing a once listener works with a passed signal + let count = 0; + function handler() { + count++; + } + const et = new EventTarget(); + const controller = new AbortController(); + const options = { signal: controller.signal, once: true }; + et.addEventListener('test', handler, options); + controller.abort(); + et.dispatchEvent(new Event('test')); + strictEqual(count, 0, 'The listener was still removed'); +} + +{ + let count = 0; + function handler() { + count++; + } + const et = new EventTarget(); + const controller = new AbortController(); + const options = { signal: controller.signal, once: true }; + et.addEventListener('test', handler, options); + et.removeEventListener('test', handler); + et.dispatchEvent(new Event('test')); + strictEqual(count, 0, 'The listener was still removed'); +} + +{ + // Passing an AbortSignal to multiple listeners + let count = 0; + function handler() { + count++; + } + const et = new EventTarget(); + const controller = new AbortController(); + const options = { signal: controller.signal, once: true }; + et.addEventListener('first', handler, options); + et.addEventListener('second', handler, options); + controller.abort(); + et.dispatchEvent(new Event('first')); + et.dispatchEvent(new Event('second')); + strictEqual(count, 0, 'The listener was still removed'); +} + +{ + // Passing an AbortSignal to addEventListener works with the capture flag + let count = 0; + function handler() { + count++; + } + const et = new EventTarget(); + const controller = new AbortController(); + const options = { signal: controller.signal, capture: true }; + et.addEventListener('test', handler, options); + controller.abort(); + et.dispatchEvent(new Event('test')); + strictEqual(count, 0, 'The listener was still removed'); +} + +{ + // Aborting from a listener does not call future listeners + let count = 0; + function handler() { + count++; + } + const et = new EventTarget(); + const controller = new AbortController(); + const options = { signal: controller.signal }; + et.addEventListener('test', () => { + controller.abort(); + }, options); + et.addEventListener('test', handler, options); + et.dispatchEvent(new Event('test')); + strictEqual(count, 0, 'The listener was still removed'); +} + +{ + // Adding then aborting a listener in another listener does not call it + let count = 0; + function handler() { + count++; + } + const et = new EventTarget(); + const controller = new AbortController(); + et.addEventListener('test', () => { + et.addEventListener('test', handler, { signal: controller.signal }); + controller.abort(); + }, { signal: controller.signal }); + et.dispatchEvent(new Event('test')); + strictEqual(count, 0, 'The listener was still removed'); +} + +{ + // Aborting from a nested listener should remove it + const et = new EventTarget(); + const ac = new AbortController(); + let count = 0; + et.addEventListener('foo', () => { + et.addEventListener('foo', () => { + count++; + if (count > 5) ac.abort(); + et.dispatchEvent(new Event('foo')); + }, { signal: ac.signal }); + et.dispatchEvent(new Event('foo')); + }, { once: true }); + et.dispatchEvent(new Event('foo')); +} diff --git a/test/parallel/test-eventtarget.js b/test/parallel/test-eventtarget.js index 91df453cfd409a..45c63e0dd18be3 100644 --- a/test/parallel/test-eventtarget.js +++ b/test/parallel/test-eventtarget.js @@ -532,3 +532,12 @@ let asyncTest = Promise.resolve(); target.dispatchEvent(new Event('foo')); deepStrictEqual(output, [1, 2, 3, 4]); } +{ + const et = new EventTarget(); + const listener = common.mustNotCall(); + et.addEventListener('foo', common.mustCall((e) => { + et.removeEventListener('foo', listener); + })); + et.addEventListener('foo', listener); + et.dispatchEvent(new Event('foo')); +}