lib: add option to force handling stopped events

PR-URL: #48301 Backport-PR-URL: #49587 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Moshe Atlow <[email protected]>
nodejs · Sep 12, 2023 · db355d1 · db355d1
1 parent 56249b0
commit db355d1
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 11 deletions.
diff --git a/lib/events.js b/lib/events.js
@@ -59,6 +59,7 @@ const {
 } = require('internal/util/inspect');
 
 let spliceOne;
+let kResistStopPropagation;
 
 const {
   AbortError,
@@ -981,7 +982,10 @@ async function once(emitter, name, options = kEmptyObject) {
       }
       resolve(args);
     };
-    eventTargetAgnosticAddListener(emitter, name, resolver, { once: true });
+
+    kResistStopPropagation ??= require('internal/event_target').kResistStopPropagation;
+    const opts = { __proto__: null, once: true, [kResistStopPropagation]: true };
+    eventTargetAgnosticAddListener(emitter, name, resolver, opts);
     if (name !== 'error' && typeof emitter.once === 'function') {
       // EventTarget does not have `error` event semantics like Node
       // EventEmitters, we listen to `error` events only on EventEmitters.
@@ -994,7 +998,7 @@ async function once(emitter, name, options = kEmptyObject) {
     }
     if (signal != null) {
       eventTargetAgnosticAddListener(
-        signal, 'abort', abortListener, { once: true });
+        signal, 'abort', abortListener, { __proto__: null, once: true, [kResistStopPropagation]: true });
     }
   });
 }
@@ -1119,11 +1123,12 @@ function on(emitter, event, options = kEmptyObject) {
   }
 
   if (signal) {
+    kResistStopPropagation ??= require('internal/event_target').kResistStopPropagation;
     eventTargetAgnosticAddListener(
       signal,
       'abort',
       abortListener,
-      { once: true });
+      { __proto__: null, once: true, [kResistStopPropagation]: true });
   }
 
   function abortListener() {

diff --git a/lib/internal/event_target.js b/lib/internal/event_target.js
@@ -59,6 +59,7 @@ const kStop = Symbol('kStop');
 const kTarget = Symbol('kTarget');
 const kHandlers = Symbol('kHandlers');
 const kWeakHandler = Symbol('kWeak');
+const kResistStopPropagation = Symbol('kResistStopPropagation');
 
 const kHybridDispatch = SymbolFor('nodejs.internal.kHybridDispatch');
 const kCreateEvent = Symbol('kCreateEvent');
@@ -403,6 +404,7 @@ const kFlagPassive = 1 << 2;
 const kFlagNodeStyle = 1 << 3;
 const kFlagWeak = 1 << 4;
 const kFlagRemoved = 1 << 5;
+const kFlagResistStopPropagation = 1 << 6;
 
 // The listeners for an EventTarget are maintained as a linked list.
 // Unfortunately, the way EventTarget is defined, listeners are accounted
@@ -413,7 +415,7 @@ const kFlagRemoved = 1 << 5;
 // slower.
 class Listener {
   constructor(previous, listener, once, capture, passive,
-              isNodeStyleListener, weak) {
+              isNodeStyleListener, weak, resistStopPropagation) {
     this.next = undefined;
     if (previous !== undefined)
       previous.next = this;
@@ -431,6 +433,8 @@ class Listener {
       flags |= kFlagNodeStyle;
     if (weak)
       flags |= kFlagWeak;
+    if (resistStopPropagation)
+      flags |= kFlagResistStopPropagation;
     this.flags = flags;
 
     this.removed = false;
@@ -468,6 +472,9 @@ class Listener {
   get weak() {
     return Boolean(this.flags & kFlagWeak);
   }
+  get resistStopPropagation() {
+    return Boolean(this.flags & kFlagResistStopPropagation);
+  }
   get removed() {
     return Boolean(this.flags & kFlagRemoved);
   }
@@ -564,6 +571,7 @@ class EventTarget {
       signal,
       isNodeStyleListener,
       weak,
+      resistStopPropagation,
     } = validateEventListenerOptions(options);
 
     if (!validateEventListener(listener)) {
@@ -588,16 +596,16 @@ class EventTarget {
       // not prevent the event target from GC.
       signal.addEventListener('abort', () => {
         this.removeEventListener(type, listener, options);
-      }, { once: true, [kWeakHandler]: this });
+      }, { __proto__: null, once: true, [kWeakHandler]: this, [kResistStopPropagation]: true });
     }
 
     let root = this[kEvents].get(type);
 
     if (root === undefined) {
-      root = { size: 1, next: undefined };
+      root = { size: 1, next: undefined, resistStopPropagation: Boolean(resistStopPropagation) };
       // This is the first handler in our linked list.
       new Listener(root, listener, once, capture, passive,
-                   isNodeStyleListener, weak);
+                   isNodeStyleListener, weak, resistStopPropagation);
       this[kNewListener](
         root.size,
         type,
@@ -624,8 +632,9 @@ class EventTarget {
     }
 
     new Listener(previous, listener, once, capture, passive,
-                 isNodeStyleListener, weak);
+                 isNodeStyleListener, weak, resistStopPropagation);
     root.size++;
+    root.resistStopPropagation ||= Boolean(resistStopPropagation);
     this[kNewListener](root.size, type, listener, once, capture, passive, weak);
   }
 
@@ -709,14 +718,21 @@ class EventTarget {
     let handler = root.next;
     let next;
 
-    while (handler !== undefined &&
-           (handler.passive || event?.[kStop] !== true)) {
+    const iterationCondition = () => {
+      if (handler === undefined) {
+        return false;
+      }
+      return root.resistStopPropagation || handler.passive || event?.[kStop] !== true;
+    };
+    while (iterationCondition()) {
       // Cache the next item in case this iteration removes the current one
       next = handler.next;
 
-      if (handler.removed) {
+      if (handler.removed || (event?.[kStop] === true && !handler.resistStopPropagation)) {
         // Deal with the case an event is removed while event handlers are
         // Being processed (removeEventListener called from a listener)
+        // And the case of event.stopImmediatePropagation() being called
+        // For events not flagged as resistStopPropagation
         handler = next;
         continue;
       }
@@ -984,6 +1000,7 @@ function validateEventListenerOptions(options) {
     passive: Boolean(options.passive),
     signal: options.signal,
     weak: options[kWeakHandler],
+    resistStopPropagation: options[kResistStopPropagation] ?? false,
     isNodeStyleListener: Boolean(options[kIsNodeStyleListener]),
   };
 }
@@ -1099,5 +1116,6 @@ module.exports = {
   kRemoveListener,
   kEvents,
   kWeakHandler,
+  kResistStopPropagation,
   isEventTarget,
 };
diff --git a/test/parallel/test-events-once.js b/test/parallel/test-events-once.js
@@ -233,6 +233,18 @@ async function eventTargetAbortSignalBefore() {
   });
 }
 
+async function eventTargetAbortSignalBeforeEvenWhenSignalPropagationStopped() {
+  const et = new EventTarget();
+  const ac = new AbortController();
+  const { signal } = ac;
+  signal.addEventListener('abort', (e) => e.stopImmediatePropagation(), { once: true });
+
+  process.nextTick(() => ac.abort());
+  return rejects(once(et, 'foo', { signal }), {
+    name: 'AbortError',
+  });
+}
+
 async function eventTargetAbortSignalAfter() {
   const et = new EventTarget();
   const ac = new AbortController();
@@ -270,6 +282,7 @@ Promise.all([
   abortSignalAfterEvent(),
   abortSignalRemoveListener(),
   eventTargetAbortSignalBefore(),
+  eventTargetAbortSignalBeforeEvenWhenSignalPropagationStopped(),
   eventTargetAbortSignalAfter(),
   eventTargetAbortSignalAfterEvent(),
 ]).then(common.mustCall());
diff --git a/test/parallel/test-eventtarget.js b/test/parallel/test-eventtarget.js
@@ -717,3 +717,15 @@ let asyncTest = Promise.resolve();
     et.removeEventListener(Symbol('symbol'), () => {});
   }, TypeError);
 }
+
+{
+  // Test that event listeners are removed by signal even when
+  // signal's abort event propagation stopped
+  const controller = new AbortController();
+  const { signal } = controller;
+  signal.addEventListener('abort', (e) => e.stopImmediatePropagation(), { once: true });
+  const et = new EventTarget();
+  et.addEventListener('foo', common.mustNotCall(), { signal });
+  controller.abort();
+  et.dispatchEvent(new Event('foo'));
+}