Add AbortSignal support for event listeners (#39406)

Also fixes several issues with code generation when a dom type is part
of a dictionary.

Part of #34866
Fixes #39398

Signed-off-by: Tim van der Lippe <tvanderlippe@gmail.com>

components/script/dom/abortsignal.rs

@@ -3,6 +3,7 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 use std::cell::{Cell, RefCell};
+use std::rc::Rc;
 use std::sync::{Arc, Mutex};
 
 use dom_struct::dom_struct;
@@ -15,9 +16,13 @@ use script_bindings::trace::CustomTraceable;
 
 use crate::dom::bindings::cell::DomRefCell;
 use crate::dom::bindings::codegen::Bindings::AbortSignalBinding::AbortSignalMethods;
+use crate::dom::bindings::codegen::Bindings::EventListenerBinding::EventListener;
+use crate::dom::bindings::codegen::Bindings::EventTargetBinding::EventListenerOptions;
 use crate::dom::bindings::error::{Error, ErrorToJsval};
+use crate::dom::bindings::refcounted::Trusted;
 use crate::dom::bindings::reflector::{DomGlobal, reflect_dom_object_with_proto};
 use crate::dom::bindings::root::{Dom, DomRoot};
+use crate::dom::bindings::str::DOMString;
 use crate::dom::eventtarget::EventTarget;
 use crate::dom::globalscope::GlobalScope;
 use crate::dom::readablestream::PipeTo;
@@ -35,7 +40,7 @@ impl js::gc::Rootable for AbortAlgorithm {}
 #[allow(dead_code)]
 pub(crate) enum AbortAlgorithm {
     /// <https://dom.spec.whatwg.org/#add-an-event-listener>
-    DomEventLister,
+    DomEventListener(RemovableDomEventListener),
     /// <https://streams.spec.whatwg.org/#readable-stream-pipe-to>
     StreamPiping(PipeTo),
     /// <https://fetch.spec.whatwg.org/#dom-global-fetch>
@@ -46,6 +51,15 @@ pub(crate) enum AbortAlgorithm {
     ),
 }
 
+#[derive(Clone, JSTraceable, MallocSizeOf)]
+pub(crate) struct RemovableDomEventListener {
+    pub(crate) event_target: Trusted<EventTarget>,
+    pub(crate) ty: DOMString,
+    #[conditional_malloc_size_of]
+    pub(crate) listener: Option<Rc<EventListener>>,
+    pub(crate) options: EventListenerOptions,
+}
+
 /// <https://dom.spec.whatwg.org/#abortsignal>
 #[dom_struct]
 pub(crate) struct AbortSignal {
@@ -176,9 +190,15 @@ impl AbortSignal {
                     .unwrap()
                     .abort_fetch(reason.handle(), cx, can_gc);
             },
-            _ => {
+            AbortAlgorithm::DomEventListener(removable_listener) => {
-                // TODO: match on variant and implement algo steps.
+                removable_listener
-                // See the various items of #34866
+                    .event_target
+                    .root()
+                    .remove_event_listener(
+                        removable_listener.ty.clone(),
+                        &removable_listener.listener,
+                        &removable_listener.options,
+                    );
             },
         }
     }

components/script/dom/event.rs

@@ -1282,7 +1282,7 @@ fn inner_invoke(
         }
 
         // Step 2.9 If listener’s passive is true, then set event's in passive listener flag.
-        event.set_in_passive_listener(event_target.is_passive(&event.type_(), listener));
+        event.set_in_passive_listener(event_target.is_passive(listener));
 
         // Step 2.10 If global is a Window object, then record timing info for event listener
         // given event and listener.

components/script/dom/eventtarget.rs

@@ -23,6 +23,7 @@ use style::str::HTML_SPACE_CHARACTERS;
 use stylo_atoms::Atom;
 
 use crate::conversions::Convert;
+use crate::dom::abortsignal::{AbortAlgorithm, RemovableDomEventListener};
 use crate::dom::beforeunloadevent::BeforeUnloadEvent;
 use crate::dom::bindings::callback::{CallbackContainer, CallbackFunction, ExceptionHandling};
 use crate::dom::bindings::cell::DomRefCell;
@@ -46,6 +47,7 @@ use crate::dom::bindings::codegen::UnionTypes::{
 };
 use crate::dom::bindings::error::{Error, Fallible, report_pending_exception};
 use crate::dom::bindings::inheritance::Castable;
+use crate::dom::bindings::refcounted::Trusted;
 use crate::dom::bindings::reflector::{
     DomGlobal, DomObject, Reflector, reflect_dom_object_with_proto,
 };
@@ -431,7 +433,7 @@ pub(crate) struct EventListenerEntry {
     phase: ListenerPhase,
     listener: EventListenerType,
     once: bool,
-    passive: Option<bool>,
+    passive: bool,
     removed: bool,
 }
 
@@ -605,7 +607,7 @@ impl EventTarget {
     /// <https://html.spec.whatwg.org/multipage/#event-handler-attributes:event-handlers-11>
     fn set_inline_event_listener(&self, ty: Atom, listener: Option<InlineEventListener>) {
         let mut handlers = self.handlers.borrow_mut();
-        let entries = match handlers.entry(ty) {
+        let entries = match handlers.entry(ty.clone()) {
             Occupied(entry) => entry.into_mut(),
             Vacant(entry) => entry.insert(EventListeners(vec![])),
         };
@@ -631,7 +633,7 @@ impl EventTarget {
                         phase: ListenerPhase::Bubbling,
                         listener: EventListenerType::Inline(listener.into()),
                         once: false,
-                        passive: None,
+                        passive: self.default_passive_value(&ty),
                         removed: false,
                     })));
                 }
@@ -650,11 +652,8 @@ impl EventTarget {
     }
 
     /// Determines the `passive` attribute of an associated event listener
-    pub(crate) fn is_passive(&self, ty: &Atom, listener: &Rc<RefCell<EventListenerEntry>>) -> bool {
+    pub(crate) fn is_passive(&self, listener: &Rc<RefCell<EventListenerEntry>>) -> bool {
-        listener
+        listener.borrow().passive
-            .borrow()
-            .passive
-            .unwrap_or(self.default_passive_value(ty))
     }
 
     fn get_inline_event_listener(&self, ty: &Atom, can_gc: CanGc) -> Option<CommonEventHandler> {
@@ -943,18 +942,36 @@ impl EventTarget {
     }
 
     /// <https://dom.spec.whatwg.org/#dom-eventtarget-addeventlistener>
+    /// and <https://dom.spec.whatwg.org/#add-an-event-listener>
     pub(crate) fn add_event_listener(
         &self,
         ty: DOMString,
         listener: Option<Rc<EventListener>>,
         options: AddEventListenerOptions,
     ) {
+        if let Some(signal) = options.signal.as_ref() {
+            // Step 2. If listener’s signal is not null and is aborted, then return.
+            if signal.aborted() {
+                return;
+            }
+            // Step 6. If listener’s signal is not null, then add the following abort steps to it:
+            signal.add(&AbortAlgorithm::DomEventListener(
+                RemovableDomEventListener {
+                    event_target: Trusted::new(self),
+                    ty: ty.clone(),
+                    listener: listener.clone(),
+                    options: options.parent.clone(),
+                },
+            ));
+        }
+        // Step 3. If listener’s callback is null, then return.
         let listener = match listener {
             Some(l) => l,
             None => return,
         };
         let mut handlers = self.handlers.borrow_mut();
-        let entries = match handlers.entry(Atom::from(ty)) {
+        let ty = Atom::from(ty);
+        let entries = match handlers.entry(ty.clone()) {
             Occupied(entry) => entry.into_mut(),
             Vacant(entry) => entry.insert(EventListeners(vec![])),
         };
@@ -964,27 +981,32 @@ impl EventTarget {
         } else {
             ListenerPhase::Bubbling
         };
+        // Step 4. If listener’s passive is null, then set it to the default passive value given listener’s type and eventTarget.
         let new_entry = Rc::new(RefCell::new(EventListenerEntry {
             phase,
             listener: EventListenerType::Additive(listener),
             once: options.once,
-            passive: options.passive,
+            passive: options.passive.unwrap_or(self.default_passive_value(&ty)),
             removed: false,
         }));
 
+        // Step 5. If eventTarget’s event listener list does not contain
+        // an event listener whose type is listener’s type, callback is listener’s callback,
+        // and capture is listener’s capture, then append listener to eventTarget’s event listener list.
         if !entries.contains(&new_entry) {
             entries.push(new_entry);
         }
     }
 
-    // https://dom.spec.whatwg.org/#dom-eventtarget-removeeventlistener
+    /// <https://dom.spec.whatwg.org/#dom-eventtarget-removeeventlistener>
+    /// and <https://dom.spec.whatwg.org/#remove-an-event-listener>
     pub(crate) fn remove_event_listener(
         &self,
         ty: DOMString,
-        listener: Option<Rc<EventListener>>,
+        listener: &Option<Rc<EventListener>>,
-        options: EventListenerOptions,
+        options: &EventListenerOptions,
     ) {
-        let Some(ref listener) = listener else {
+        let Some(listener) = listener else {
             return;
         };
         let mut handlers = self.handlers.borrow_mut();
@@ -994,10 +1016,12 @@ impl EventTarget {
             } else {
                 ListenerPhase::Bubbling
             };
-            if let Some(position) = entries.iter().position(|e| {
+            let listener_type = EventListenerType::Additive(listener.clone());
-                e.borrow().listener == EventListenerType::Additive(listener.clone()) &&
+            if let Some(position) = entries
-                    e.borrow().phase == phase
+                .iter()
-            }) {
+                .position(|e| e.borrow().listener == listener_type && e.borrow().phase == phase)
+            {
+                // Step 2. Set listener’s removed to true and remove listener from eventTarget’s event listener list.
                 entries.remove(position).borrow_mut().removed = true;
             }
         }
@@ -1102,7 +1126,7 @@ impl EventTargetMethods<crate::DomTypeHolder> for EventTarget {
         listener: Option<Rc<EventListener>>,
         options: EventListenerOptionsOrBoolean,
     ) {
-        self.remove_event_listener(ty, listener, options.convert())
+        self.remove_event_listener(ty, &listener, &options.convert())
     }
 
     // https://dom.spec.whatwg.org/#dom-eventtarget-dispatchevent
@@ -1122,13 +1146,20 @@ impl VirtualMethods for EventTarget {
 }
 
 impl Convert<AddEventListenerOptions> for AddEventListenerOptionsOrBoolean {
+    /// <https://dom.spec.whatwg.org/#event-flatten-more>
     fn convert(self) -> AddEventListenerOptions {
+        // Step 1. Let capture be the result of flattening options.
+        // Step 5. Return capture, passive, once, and signal.
         match self {
+            // Step 4. If options is a dictionary:
             AddEventListenerOptionsOrBoolean::AddEventListenerOptions(options) => options,
             AddEventListenerOptionsOrBoolean::Boolean(capture) => AddEventListenerOptions {
                 parent: EventListenerOptions { capture },
+                // Step 2. Let once be false.
                 once: false,
+                // Step 3. Let passive and signal be null.
                 passive: None,
+                signal: None,
             },
         }
     }

components/script/dom/mediaquerylist.rs

@@ -107,6 +107,7 @@ impl MediaQueryListMethods<crate::DomTypeHolder> for MediaQueryList {
                 parent: EventListenerOptions { capture: false },
                 once: false,
                 passive: None,
+                signal: None,
             },
         );
     }
@@ -115,8 +116,8 @@ impl MediaQueryListMethods<crate::DomTypeHolder> for MediaQueryList {
     fn RemoveListener(&self, listener: Option<Rc<EventListener>>) {
         self.upcast::<EventTarget>().remove_event_listener(
             DOMString::from_string("change".to_owned()),
-            listener,
+            &listener,
-            EventListenerOptions { capture: false },
+            &EventListenerOptions { capture: false },
         );
     }
 

components/script_bindings/codegen/Bindings.conf

@@ -812,6 +812,10 @@ Dictionaries = {
     'derives': ['Clone', 'MallocSizeOf'],
 },
 
+'EventListenerOptions': {
+    'derives': ['Clone', 'MallocSizeOf'],
+},
+
 'FocusOptions': {
     'derives': ['Clone', 'MallocSizeOf']
 },

components/script_bindings/codegen/codegen.py

@@ -671,7 +671,7 @@ def typeIsSequenceOrHasSequenceMember(type: IDLType) -> bool:
 
 def union_native_type(t: IDLType) -> str:
     name = t.unroll().name
-    generic = "<D>" if containsDomInterface(t) else ""
+    generic = "::<D>" if containsDomInterface(t) else ""
     return f'GenericUnionTypes::{name}{generic}'
 
 
@@ -2414,7 +2414,9 @@ class CGImports(CGWrapper):
                 if getIdentifier(t) in [c.identifier for c in callbacks]:
                     continue
             # Importing these types in the same module that defines them is an error.
-            if t in dictionaries or t in enums:
+            if t.isDictionary() and t in dictionaries:
+                continue
+            if t.isEnum() and t in enums:
                 continue
             if t.isInterface() or t.isNamespace():
                 name = getIdentifier(t).name
@@ -2422,7 +2424,9 @@ class CGImports(CGWrapper):
                 parentName = descriptor.getParentName()
                 while parentName:
                     descriptor = descriptorProvider.getDescriptor(parentName)
-                    extras += [descriptor.bindingPath]
+                    # Importing these types in the same module that defines them is an error.
+                    if descriptor not in descriptors:
+                        extras += [descriptor.bindingPath]
                     parentName = descriptor.getParentName()
             elif isIDLType(t) and t.isRecord():
                 extras += ['crate::record::Record']
@@ -7438,7 +7442,7 @@ class CGDictionary(CGThing):
             memberName = self.makeMemberName(m[0].identifier.name)
             members += [f"            {memberName}: self.{memberName}.clone(),"]
         if self.dictionary.parent:
-            members += ["            parent: parent.clone(),"]
+            members += ["            parent: self.parent.clone(),"]
         members = "\n".join(members)
         return f"""
 #[allow(clippy::clone_on_copy)]
@@ -8020,7 +8024,7 @@ class CGBindingRoot(CGThing):
 
             if t.innerType.isUnion() and not t.innerType.nullable():
                 # Allow using the typedef's name for accessing variants.
-                typeDefinition = f"pub use self::{type.replace('<D>', '')} as {name};"
+                typeDefinition = f"pub use self::{type.replace('::<D>', '')} as {name};"
             else:
                 generic = "<D>" if containsDomInterface(t.innerType) else ""
                 replacedType = type.replace("D::", "<D as DomTypes>::")
@@ -8054,7 +8058,7 @@ class CGBindingRoot(CGThing):
 
         # Add imports
         # These are the global imports (outside of the generated module)
-        curr = CGImports(curr, descriptors=callbackDescriptors, callbacks=mainCallbacks,
+        curr = CGImports(curr, descriptors=callbackDescriptors + descriptors, callbacks=mainCallbacks,
                          dictionaries=dictionaries, enums=enums, typedefs=typedefs,
                          imports=['crate::import::base::*'], config=config)
 

components/script_bindings/webidls/EventTarget.webidl

@@ -24,11 +24,14 @@ interface EventTarget {
   boolean dispatchEvent(Event event);
 };
 
+// https://dom.spec.whatwg.org/#dictdef-eventlisteneroptions
 dictionary EventListenerOptions {
   boolean capture = false;
 };
 
+// https://dom.spec.whatwg.org/#dictdef-addeventlisteneroptions
 dictionary AddEventListenerOptions : EventListenerOptions {
   boolean passive;
   boolean once = false;
+  AbortSignal signal;
 };

tests/wpt/meta/dom/events/AddEventListenerOptions-signal.any.js.ini

@@ -1,68 +0,0 @@
-[AddEventListenerOptions-signal.any.html]
-  [Passing an AbortSignal to addEventListener works with the once flag]
-    expected: FAIL
-
-  [Passing an AbortSignal to addEventListener works with the capture flag]
-    expected: FAIL
-
-  [Aborting from a listener does not call future listeners]
-    expected: FAIL
-
-  [Passing an AbortSignal to multiple listeners]
-    expected: FAIL
-
-  [Passing an AbortSignal to addEventListener options should allow removing a listener]
-    expected: FAIL
-
-  [Passing null as the signal should throw]
-    expected: FAIL
-
-  [Passing null as the signal should throw (listener is also null)]
-    expected: FAIL
-
-  [Passing an AbortSignal to addEventListener does not prevent removeEventListener]
-    expected: FAIL
-
-  [Removing a once listener works with a passed signal]
-    expected: FAIL
-
-  [Adding then aborting a listener in another listener does not call it]
-    expected: FAIL
-
-  [Aborting from a nested listener should remove it]
-    expected: FAIL
-
-
-[AddEventListenerOptions-signal.any.worker.html]
-  [Passing an AbortSignal to addEventListener works with the once flag]
-    expected: FAIL
-
-  [Passing an AbortSignal to addEventListener works with the capture flag]
-    expected: FAIL
-
-  [Aborting from a listener does not call future listeners]
-    expected: FAIL
-
-  [Passing an AbortSignal to multiple listeners]
-    expected: FAIL
-
-  [Passing an AbortSignal to addEventListener options should allow removing a listener]
-    expected: FAIL
-
-  [Passing null as the signal should throw]
-    expected: FAIL
-
-  [Passing null as the signal should throw (listener is also null)]
-    expected: FAIL
-
-  [Passing an AbortSignal to addEventListener does not prevent removeEventListener]
-    expected: FAIL
-
-  [Removing a once listener works with a passed signal]
-    expected: FAIL
-
-  [Adding then aborting a listener in another listener does not call it]
-    expected: FAIL
-
-  [Aborting from a nested listener should remove it]
-    expected: FAIL

tests/wpt/meta/dom/events/__dir__.ini

@@ -0,0 +1,3 @@
+prefs: [
+  "dom_abort_controller_enabled:true",
+]