From 02aab339877d248c3269135c37e650cced224456 Mon Sep 17 00:00:00 2001 From: Tim van der Lippe Date: Sun, 21 Sep 2025 20:57:10 +0200 Subject: [PATCH] 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 --- components/script/dom/abortsignal.rs | 28 ++++++-- components/script/dom/event.rs | 2 +- components/script/dom/eventtarget.rs | 69 ++++++++++++++----- components/script/dom/mediaquerylist.rs | 5 +- .../script_bindings/codegen/Bindings.conf | 4 ++ components/script_bindings/codegen/codegen.py | 16 +++-- .../webidls/EventTarget.webidl | 3 + .../AddEventListenerOptions-signal.any.js.ini | 68 ------------------ tests/wpt/meta/dom/events/__dir__.ini | 3 + 9 files changed, 98 insertions(+), 100 deletions(-) delete mode 100644 tests/wpt/meta/dom/events/AddEventListenerOptions-signal.any.js.ini create mode 100644 tests/wpt/meta/dom/events/__dir__.ini diff --git a/components/script/dom/abortsignal.rs b/components/script/dom/abortsignal.rs index 3444f62958c..a2fc7ce03ef 100644 --- a/components/script/dom/abortsignal.rs +++ b/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 { /// - DomEventLister, + DomEventListener(RemovableDomEventListener), /// StreamPiping(PipeTo), /// @@ -46,6 +51,15 @@ pub(crate) enum AbortAlgorithm { ), } +#[derive(Clone, JSTraceable, MallocSizeOf)] +pub(crate) struct RemovableDomEventListener { + pub(crate) event_target: Trusted, + pub(crate) ty: DOMString, + #[conditional_malloc_size_of] + pub(crate) listener: Option>, + pub(crate) options: EventListenerOptions, +} + /// #[dom_struct] pub(crate) struct AbortSignal { @@ -176,9 +190,15 @@ impl AbortSignal { .unwrap() .abort_fetch(reason.handle(), cx, can_gc); }, - _ => { - // TODO: match on variant and implement algo steps. - // See the various items of #34866 + AbortAlgorithm::DomEventListener(removable_listener) => { + removable_listener + .event_target + .root() + .remove_event_listener( + removable_listener.ty.clone(), + &removable_listener.listener, + &removable_listener.options, + ); }, } } diff --git a/components/script/dom/event.rs b/components/script/dom/event.rs index 627d99b5146..1adebfb38ae 100644 --- a/components/script/dom/event.rs +++ b/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. diff --git a/components/script/dom/eventtarget.rs b/components/script/dom/eventtarget.rs index 25947a01c99..a32d03b80a4 100644 --- a/components/script/dom/eventtarget.rs +++ b/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, + passive: bool, removed: bool, } @@ -605,7 +607,7 @@ impl EventTarget { /// fn set_inline_event_listener(&self, ty: Atom, listener: Option) { 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>) -> bool { - listener - .borrow() - .passive - .unwrap_or(self.default_passive_value(ty)) + pub(crate) fn is_passive(&self, listener: &Rc>) -> bool { + listener.borrow().passive } fn get_inline_event_listener(&self, ty: &Atom, can_gc: CanGc) -> Option { @@ -943,18 +942,36 @@ impl EventTarget { } /// + /// and pub(crate) fn add_event_listener( &self, ty: DOMString, listener: Option>, 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 + /// + /// and pub(crate) fn remove_event_listener( &self, ty: DOMString, - listener: Option>, - options: EventListenerOptions, + listener: &Option>, + 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| { - e.borrow().listener == EventListenerType::Additive(listener.clone()) && - e.borrow().phase == phase - }) { + let listener_type = EventListenerType::Additive(listener.clone()); + if let Some(position) = entries + .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 for EventTarget { listener: Option>, 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 for AddEventListenerOptionsOrBoolean { + /// 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, }, } } diff --git a/components/script/dom/mediaquerylist.rs b/components/script/dom/mediaquerylist.rs index 53b0915c5a5..fa5e9ccf077 100644 --- a/components/script/dom/mediaquerylist.rs +++ b/components/script/dom/mediaquerylist.rs @@ -107,6 +107,7 @@ impl MediaQueryListMethods for MediaQueryList { parent: EventListenerOptions { capture: false }, once: false, passive: None, + signal: None, }, ); } @@ -115,8 +116,8 @@ impl MediaQueryListMethods for MediaQueryList { fn RemoveListener(&self, listener: Option>) { self.upcast::().remove_event_listener( DOMString::from_string("change".to_owned()), - listener, - EventListenerOptions { capture: false }, + &listener, + &EventListenerOptions { capture: false }, ); } diff --git a/components/script_bindings/codegen/Bindings.conf b/components/script_bindings/codegen/Bindings.conf index ef7a1ee9fde..b27daba04fa 100644 --- a/components/script_bindings/codegen/Bindings.conf +++ b/components/script_bindings/codegen/Bindings.conf @@ -812,6 +812,10 @@ Dictionaries = { 'derives': ['Clone', 'MallocSizeOf'], }, +'EventListenerOptions': { + 'derives': ['Clone', 'MallocSizeOf'], +}, + 'FocusOptions': { 'derives': ['Clone', 'MallocSizeOf'] }, diff --git a/components/script_bindings/codegen/codegen.py b/components/script_bindings/codegen/codegen.py index e94da1a2f47..1b45fdfe3db 100644 --- a/components/script_bindings/codegen/codegen.py +++ b/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 = "" if containsDomInterface(t) else "" + generic = "::" 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('', '')} as {name};" + typeDefinition = f"pub use self::{type.replace('::', '')} as {name};" else: generic = "" if containsDomInterface(t.innerType) else "" replacedType = type.replace("D::", "::") @@ -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) diff --git a/components/script_bindings/webidls/EventTarget.webidl b/components/script_bindings/webidls/EventTarget.webidl index 411fb962d32..af669224a4c 100644 --- a/components/script_bindings/webidls/EventTarget.webidl +++ b/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; }; diff --git a/tests/wpt/meta/dom/events/AddEventListenerOptions-signal.any.js.ini b/tests/wpt/meta/dom/events/AddEventListenerOptions-signal.any.js.ini deleted file mode 100644 index a3d315fc157..00000000000 --- a/tests/wpt/meta/dom/events/AddEventListenerOptions-signal.any.js.ini +++ /dev/null @@ -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 diff --git a/tests/wpt/meta/dom/events/__dir__.ini b/tests/wpt/meta/dom/events/__dir__.ini new file mode 100644 index 00000000000..e79af6880d5 --- /dev/null +++ b/tests/wpt/meta/dom/events/__dir__.ini @@ -0,0 +1,3 @@ +prefs: [ + "dom_abort_controller_enabled:true", +]