fix abortsignal event names (#5269)

Fixes #5260.

This makes AbortSignal.addEventListener ignore non abort event names
instead of throwing.
It keeps the abort listener path the same and adds tests for ignored
event names.

Tests:
- cargo test -p boa_runtime abort --lib
- cargo test -p boa_runtime --lib
- cargo clippy -p boa_runtime --all-features --all-targets -- -D
warnings

Author: Monti-27

core/runtime/src/abort/mod.rs

@@ -5,8 +5,8 @@ use boa_engine::job::GenericJob;
 use boa_engine::object::builtins::JsFunction;
 use boa_engine::realm::Realm;
 use boa_engine::{
-    Context, Finalize, JsData, JsError, JsNativeError, JsObject, JsResult, JsValue, Trace,
-    boa_class, boa_module, js_error, js_string,
+    Context, Finalize, JsData, JsError, JsNativeError, JsObject, JsResult, JsString, JsValue,
+    Trace, boa_class, boa_module, js_error, js_string,
 };
 use boa_gc::GcRefCell;
 use std::cell::Cell;
@@ -52,11 +52,17 @@ pub struct JsAbortSignal {
     #[unsafe_ignore_trace]
     aborted: Cell<bool>,
     reason: GcRefCell<Option<JsValue>>,
-    listeners: GcRefCell<Vec<JsFunction>>,
+    listeners: GcRefCell<Vec<AbortEventListener>>,
     #[unsafe_ignore_trace]
     cancel_token: CancellationToken,
 }
 
+#[derive(Debug, Clone, Trace, Finalize)]
+struct AbortEventListener {
+    event_type: JsString,
+    callback: JsFunction,
+}
+
 impl Default for JsAbortSignal {
     fn default() -> Self {
         Self {
@@ -79,7 +85,14 @@ impl JsAbortSignal {
         self.aborted.set(true);
         *self.reason.borrow_mut() = Some(reason);
 
-        let listeners: Vec<JsFunction> = self.listeners.borrow_mut().drain(..).collect();
+        let abort = js_string!("abort");
+        let listeners: Vec<JsFunction> = self
+            .listeners
+            .borrow()
+            .iter()
+            .filter(|listener| listener.event_type == abort)
+            .map(|listener| listener.callback.clone())
+            .collect();
 
         let realm = context.realm().clone();
         for listener in listeners {
@@ -154,28 +167,28 @@ impl JsAbortSignal {
 
     fn add_event_listener(
         &self,
-        event_type: boa_engine::JsString,
+        event_type: JsString,
         callback: JsFunction,
-        context: &mut Context,
-    ) -> JsResult<()> {
-        if event_type.to_std_string_escaped() != "abort" {
-            return Err(js_error!(TypeError: "AbortSignal only supports the 'abort' event type"));
-        }
-        if self.aborted.get() {
-            callback.call(&JsValue::undefined(), &[], context)?;
-        } else {
-            self.listeners.borrow_mut().push(callback);
+        _context: &mut Context,
+    ) {
+        {
+            let listeners = self.listeners.borrow();
+            if listeners.iter().any(|listener| {
+                listener.event_type == event_type && JsObject::equals(&listener.callback, &callback)
+            }) {
+                return;
+            }
         }
-        Ok(())
+        self.listeners.borrow_mut().push(AbortEventListener {
+            event_type,
+            callback,
+        });
     }
 
-    fn remove_event_listener(&self, event_type: boa_engine::JsString, callback: JsFunction) {
-        if event_type.to_std_string_escaped() != "abort" {
-            return;
-        }
-        self.listeners
-            .borrow_mut()
-            .retain(|f| !JsObject::equals(f, &callback));
+    fn remove_event_listener(&self, event_type: JsString, callback: JsFunction) {
+        self.listeners.borrow_mut().retain(|listener| {
+            listener.event_type != event_type || !JsObject::equals(&listener.callback, &callback)
+        });
     }
 }
 

core/runtime/src/abort/tests.rs

@@ -117,6 +117,120 @@ fn add_event_listener_fires_on_abort() {
     ]);
 }
 
+#[test]
+fn add_event_listener_ignores_unknown_event_names() {
+    run_test_actions([
+        TestAction::run(
+            r"
+            let ctrl = new AbortController();
+            let called = false;
+            ctrl.signal.addEventListener('nope', function() {
+                called = true;
+            });
+            ctrl.abort();
+            ",
+        ),
+        TestAction::inspect_context(|ctx| {
+            ctx.run_jobs().unwrap();
+        }),
+        TestAction::run(
+            r"
+            if (called) {
+                throw new Error('unknown event listener should not fire');
+            }
+            ",
+        ),
+    ]);
+}
+
+#[test]
+fn add_event_listener_does_not_fire_abort_listener_added_after_abort() {
+    run_test_actions([
+        TestAction::run(
+            r"
+            let ctrl = new AbortController();
+            ctrl.abort();
+
+            let called = false;
+            ctrl.signal.addEventListener('abort', function() {
+                called = true;
+            });
+            ",
+        ),
+        TestAction::inspect_context(|ctx| {
+            ctx.run_jobs().unwrap();
+        }),
+        TestAction::run(
+            r"
+            if (called) {
+                throw new Error('abort listener should not fire when added after abort');
+            }
+            ",
+        ),
+    ]);
+}
+
+#[test]
+fn add_event_listener_deduplicates_same_type_and_callback() {
+    run_test_actions([
+        TestAction::run(
+            r"
+            let ctrl = new AbortController();
+            let count = 0;
+
+            function onAbort() {
+                count += 1;
+            }
+
+            ctrl.signal.addEventListener('abort', onAbort);
+            ctrl.signal.addEventListener('abort', onAbort);
+            ctrl.abort();
+            ",
+        ),
+        TestAction::inspect_context(|ctx| {
+            ctx.run_jobs().unwrap();
+        }),
+        TestAction::run(
+            r"
+            if (count !== 1) {
+                throw new Error('expected duplicate abort listener to fire once, got: ' + count);
+            }
+            ",
+        ),
+    ]);
+}
+
+#[test]
+fn remove_event_listener_uses_event_type() {
+    run_test_actions([
+        TestAction::run(
+            r"
+            let ctrl = new AbortController();
+            let count = 0;
+
+            function listener() {
+                count += 1;
+            }
+
+            ctrl.signal.addEventListener('nope', listener);
+            ctrl.signal.addEventListener('abort', listener);
+            ctrl.signal.removeEventListener('nope', listener);
+            ctrl.abort();
+            ",
+        ),
+        TestAction::inspect_context(|ctx| {
+            ctx.run_jobs().unwrap();
+        }),
+        TestAction::run(
+            r"
+            if (count !== 1) {
+                throw new Error('removing a different event type should not remove the abort listener');
+            }
+            ",
+        ),
+    ]);
+}
+
 #[test]
 fn multiple_listeners() {
     run_test_actions([