events: support signal in EventTarget

PR-URL: nodejs#36258 Fixes: nodejs#36073 Reviewed-By: James M Snell <[email protected]>
cjihrig · Dec 8, 2020 · f59c6cd · f59c6cd
1 parent b426892
commit f59c6cd
Show file tree

Hide file tree

Showing 3 changed files with 186 additions and 0 deletions.
diff --git 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
@@ -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
@@ -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'));
+}