diff --git a/moli-renderer-v8/src/abort_signal_route.rs b/moli-renderer-v8/src/abort_signal_route.rs index 04f168f142..21ec2602bd 100644 --- a/moli-renderer-v8/src/abort_signal_route.rs +++ b/moli-renderer-v8/src/abort_signal_route.rs @@ -10,7 +10,7 @@ use crate::context_bootstrap::MessagePortEventListenerId; use crate::context_bootstrap::context_host_ptr_from_global_bridge; use crate::types::MessagePortId; -use crate::util::{throw_type_error, v8str}; +use crate::webidl; #[derive(Clone, Copy)] enum AbortSignalOwner { @@ -167,42 +167,30 @@ impl<'s> ResolvedAbortSignal<'s> { } } -/// Parses the `signal` member of `AddEventListenerOptions`. -/// -/// The outer `Option` distinguishes abrupt conversion from an absent member; -/// the inner `Option` distinguishes no signal from a validated Window/worker -/// signal capability. +/// Converts the final AddEventListenerOptions member after capture/once/passive. +/// A dictionary uses Get, not HasProperty: Proxy traps and inherited getters +/// must run exactly once, and their original exceptions must escape unchanged. pub(crate) fn event_listener_signal_from_options_value<'s>( scope: &mut v8::PinScope<'s, '_>, value: v8::Local<'s, v8::Value>, -) -> Option>> { - if value.is_null_or_undefined() || !value.is_object() { - return Some(None); - } +) -> Result>, webidl::WebIdlError> { let Ok(options) = v8::Local::::try_from(value) else { - return Some(None); + return Ok(None); + }; + let context = webidl::Context::member("AddEventListenerOptions", "signal"); + let Some(signal_value) = webidl::property_result(scope, options, "signal", context)? else { + return Ok(None); }; - let signal_key = v8str(scope, "signal"); - if !options.has(scope, signal_key.into()).unwrap_or(false) { - return Some(None); - } - let signal_value = options.get(scope, signal_key.into())?; if signal_value.is_undefined() { - return Some(None); + return Ok(None); } - let Ok(signal) = v8::Local::::try_from(signal_value) else { - throw_type_error( - scope, - "Failed to execute 'addEventListener': options.signal must be an AbortSignal.", - ); - return None; - }; - let Some(signal) = ResolvedAbortSignal::resolve(scope, signal) else { - throw_type_error( - scope, - "Failed to execute 'addEventListener': options.signal must be an AbortSignal.", - ); - return None; - }; - Some(Some(signal)) + let signal = v8::Local::::try_from(signal_value) + .ok() + .and_then(|signal| ResolvedAbortSignal::resolve(scope, signal)) + .ok_or_else(|| { + webidl::WebIdlError::custom_message( + "Failed to execute 'addEventListener': options.signal must be an AbortSignal.", + ) + })?; + Ok(Some(signal)) } diff --git a/moli-renderer-v8/src/context_bootstrap/media_queries/events/simple_event_target/listeners.rs b/moli-renderer-v8/src/context_bootstrap/media_queries/events/simple_event_target/listeners.rs index 488eeb6347..04a30c85f3 100644 --- a/moli-renderer-v8/src/context_bootstrap/media_queries/events/simple_event_target/listeners.rs +++ b/moli-renderer-v8/src/context_bootstrap/media_queries/events/simple_event_target/listeners.rs @@ -1,6 +1,7 @@ use super::*; -use crate::abort_signal_route::{ResolvedAbortSignal, event_listener_signal_from_options_value}; +use crate::abort_signal_route::ResolvedAbortSignal; use crate::callback_invocation::CallbackInvocation; +use crate::event_listener_args::{AddEventListenerArgs, RemoveEventListenerArgs}; use crate::native_bridge::WindowExecutionContextIdentity; use crate::util::{ context_host_ptr_from_global_bridge, get_private_object, get_private_value, @@ -190,117 +191,27 @@ impl<'s> SimpleObjectEventListenerSnapshot<'s> { } } -#[derive(webidl::WebIdlArgs)] -#[webidl(prefix = "EventTarget.addEventListener")] -struct SimpleObjectAddListenerArgs<'s> { - #[webidl(with = simple_object_add_listener_call)] - call: webidl::ParseOutcome>, -} - -#[derive(webidl::WebIdlArgs)] -#[webidl(prefix = "EventTarget.removeEventListener")] -struct SimpleObjectRemoveListenerArgs<'s> { - #[webidl(with = simple_object_remove_listener_call)] - call: webidl::ParseOutcome>, -} - -struct SimpleObjectAddListenerCall<'s> { - event_type: String, - listener: SimpleObjectResolvedEventListener<'s>, - options: webidl::EventListenerOptions, - signal: Option>, -} - -struct SimpleObjectRemoveListenerCall<'s> { - event_type: String, - listener: v8::Local<'s, v8::Value>, - options: webidl::EventListenerOptions, -} - -fn required_simple_object_event_type<'s>( - scope: &mut v8::PinScope<'s, '_>, - args: &v8::FunctionCallbackArguments<'s>, - prefix: &'static str, - missing_message: &'static str, -) -> Result { - if args.length() == 0 { - return Err(webidl::WebIdlError::custom_message(missing_message)); - } - webidl::convert::(scope, args.get(0), webidl::Context::argument(prefix, 1)) - .map(Into::into) -} - -fn simple_object_add_listener_call<'s>( - scope: &mut v8::PinScope<'s, '_>, - args: &v8::FunctionCallbackArguments<'s>, - _index: i32, -) -> Result>, webidl::WebIdlError> { - let event_type = required_simple_object_event_type( - scope, - args, - "EventTarget.addEventListener", - "Failed to execute 'addEventListener' on 'EventTarget': 1 argument required, but only 0 present.", - )?; - let options = webidl::event_listener_options(scope, args, 2, true); - let Some(signal) = event_listener_signal_from_options_value(scope, args.get(2)) else { - return Ok(webidl::ParseOutcome::Skip); - }; - let Some(listener) = simple_object_event_listener_parts(scope, args.get(1)) else { - return Ok(webidl::ParseOutcome::Skip); - }; - Ok(webidl::ParseOutcome::Parsed(SimpleObjectAddListenerCall { - event_type, - listener, - options, - signal, - })) -} - -fn simple_object_remove_listener_call<'s>( - scope: &mut v8::PinScope<'s, '_>, - args: &v8::FunctionCallbackArguments<'s>, - _index: i32, -) -> Result>, webidl::WebIdlError> { - let event_type = required_simple_object_event_type( - scope, - args, - "EventTarget.removeEventListener", - "Failed to execute 'removeEventListener' on 'EventTarget': 1 argument required, but only 0 present.", - )?; - let listener = args.get(1); - if listener.is_null_or_undefined() { - return Ok(webidl::ParseOutcome::Skip); - } - let options = webidl::event_listener_options(scope, args, 2, false); - Ok(webidl::ParseOutcome::Parsed( - SimpleObjectRemoveListenerCall { - event_type, - listener, - options, - }, - )) -} - pub(crate) fn simple_object_event_target_add_listener<'s>( scope: &mut v8::PinScope<'s, '_>, args: &v8::FunctionCallbackArguments<'s>, slot_name: &str, ) { - let Some(parsed) = webidl::parse_args::(scope, args) else { + let Some(call) = webidl::parse_args::(scope, args) else { return; }; - let webidl::ParseOutcome::Parsed(call) = parsed.call else { + let Some(listener) = call.listener else { return; }; + let listener = simple_object_event_listener_parts(scope, listener); let target = args.this(); simple_object_event_target_register_resolved_listener( scope, target, slot_name, call.event_type, - call.listener, - call.options, - call.signal, + listener, + call.options.options, + call.options.signal, ); } @@ -320,21 +231,7 @@ pub(crate) fn simple_object_event_target_register_webidl_listener<'s>( listener: webidl::WebIdlCallbackInterface, options: webidl::EventListenerOptions, ) { - let callback_value = listener.value(scope); - let callback = v8::Local::::try_from(callback_value) - .expect("converted EventListener callback must remain an object"); - let relevant_context = listener.relevant_context(scope); - let incumbent_context = listener.incumbent_context(scope); - let (relevant_context_anchor, incumbent_context_anchor, relevant_identity) = - simple_callback_context_anchors_for_contexts(scope, relevant_context, incumbent_context); - let listener = SimpleObjectResolvedEventListener { - original: callback.into(), - callback, - relevant_context_anchor, - incumbent_context_anchor, - relevant_identity, - is_callable: listener.callable_at_conversion(), - }; + let listener = simple_object_event_listener_parts(scope, listener); simple_object_event_target_register_resolved_listener( scope, target, slot_name, event_type, listener, options, None, ); @@ -416,19 +313,20 @@ pub(crate) fn simple_object_event_target_remove_listener<'s>( args: &v8::FunctionCallbackArguments<'s>, slot_name: &str, ) { - let Some(parsed) = webidl::parse_args::(scope, args) else { + let Some(call) = webidl::parse_args::(scope, args) else { return; }; - let webidl::ParseOutcome::Parsed(call) = parsed.call else { + let Some(listener) = call.listener else { return; }; + let listener = listener.value(scope); let target = args.this(); simple_object_event_remove_listener_value_for_type( scope, target, slot_name, &call.event_type, - call.listener, + listener, call.options.capture, ); } @@ -949,22 +847,23 @@ fn remove_simple_object_event_type_order<'s>( fn simple_object_event_listener_parts<'s>( scope: &mut v8::PinScope<'s, '_>, - value: v8::Local<'s, v8::Value>, -) -> Option> { - if value.is_null_or_undefined() { - return None; - } - let callback = v8::Local::::try_from(value).ok()?; + listener: webidl::WebIdlCallbackInterface, +) -> SimpleObjectResolvedEventListener<'s> { + let callback_value = listener.value(scope); + let callback = v8::Local::::try_from(callback_value) + .expect("converted EventListener callback must remain an object"); + let relevant_context = listener.relevant_context(scope); + let incumbent_context = listener.incumbent_context(scope); let (relevant_context_anchor, incumbent_context_anchor, relevant_identity) = - simple_callback_context_anchors(scope, callback); - Some(SimpleObjectResolvedEventListener { - original: value, + simple_callback_context_anchors_for_contexts(scope, relevant_context, incumbent_context); + SimpleObjectResolvedEventListener { + original: callback.into(), callback, relevant_context_anchor, incumbent_context_anchor, relevant_identity, - is_callable: callback.is_callable(), - }) + is_callable: listener.callable_at_conversion(), + } } fn simple_callback_context_anchors<'s>( diff --git a/moli-renderer-v8/src/context_bootstrap/message_ports/event_target.rs b/moli-renderer-v8/src/context_bootstrap/message_ports/event_target.rs index 834aa9e60e..9bedd04401 100644 --- a/moli-renderer-v8/src/context_bootstrap/message_ports/event_target.rs +++ b/moli-renderer-v8/src/context_bootstrap/message_ports/event_target.rs @@ -1,31 +1,13 @@ use super::*; -use crate::abort_signal_route::event_listener_signal_from_options_value; +use crate::event_listener_args::{AddEventListenerArgs, RemoveEventListenerArgs}; use crate::webidl; -#[derive(webidl::WebIdlArgs)] -#[webidl(prefix = "MessagePort.addEventListener")] -struct MessagePortAddEventListenerArgs { - #[webidl(required, name = "type")] - event_type: String, - #[webidl(required, converter = "callback_interface", nullable)] - listener: Option, -} - -#[derive(webidl::WebIdlArgs)] -#[webidl(prefix = "MessagePort.removeEventListener")] -struct MessagePortRemoveEventListenerArgs { - #[webidl(required, name = "type")] - event_type: String, - #[webidl(required, converter = "callback_interface", nullable)] - listener: Option, -} - pub(in crate::context_bootstrap) fn message_port_add_event_listener_callback<'s>( scope: &mut v8::PinScope<'s, '_>, args: v8::FunctionCallbackArguments<'s>, mut rv: v8::ReturnValue<'_, v8::Value>, ) { - let Some(parsed) = webidl::parse_args::(scope, &args) else { + let Some(parsed) = webidl::parse_args::(scope, &args) else { return; }; if !message_port_supports_event_type(&parsed.event_type) { @@ -36,10 +18,8 @@ pub(in crate::context_bootstrap) fn message_port_add_event_listener_callback<'s> rv.set_undefined(); return; }; - let options = webidl::event_listener_options(scope, &args, 2, true); - let Some(signal) = event_listener_signal_from_options_value(scope, args.get(2)) else { - return; - }; + let options = parsed.options.options; + let signal = parsed.options.signal; if signal.is_some_and(|signal| signal.is_aborted(scope)) { rv.set_undefined(); return; @@ -71,8 +51,7 @@ pub(in crate::context_bootstrap) fn message_port_remove_event_listener_callback< args: v8::FunctionCallbackArguments<'s>, mut rv: v8::ReturnValue<'_, v8::Value>, ) { - let Some(parsed) = webidl::parse_args::(scope, &args) - else { + let Some(parsed) = webidl::parse_args::(scope, &args) else { return; }; if !message_port_supports_event_type(&parsed.event_type) { @@ -83,7 +62,7 @@ pub(in crate::context_bootstrap) fn message_port_remove_event_listener_callback< rv.set_undefined(); return; }; - let capture = webidl::event_listener_options(scope, &args, 2, false).capture; + let capture = parsed.options.capture; remove_message_port_event_listener(scope, args.this(), &parsed.event_type, &listener, capture); rv.set_undefined(); } diff --git a/moli-renderer-v8/src/event_listener_args.rs b/moli-renderer-v8/src/event_listener_args.rs new file mode 100644 index 0000000000..ddfca8a7d2 --- /dev/null +++ b/moli-renderer-v8/src/event_listener_args.rs @@ -0,0 +1,44 @@ +//! The shared EventTarget argument boundary, before target-specific mutation. + +use crate::abort_signal_route::{ResolvedAbortSignal, event_listener_signal_from_options_value}; +use crate::webidl; + +#[derive(webidl::WebIdlArgs)] +#[webidl(prefix = "EventTarget.addEventListener")] +pub(crate) struct AddEventListenerArgs<'s> { + #[webidl(required, name = "type")] + pub(crate) event_type: String, + #[webidl(required, converter = "callback_interface", nullable)] + pub(crate) listener: Option, + #[webidl(with = add_event_listener_options)] + pub(crate) options: AddEventListenerOptions<'s>, +} + +#[derive(webidl::WebIdlArgs)] +#[webidl(prefix = "EventTarget.removeEventListener")] +pub(crate) struct RemoveEventListenerArgs { + #[webidl(required, name = "type")] + pub(crate) event_type: String, + #[webidl(required, converter = "callback_interface", nullable)] + pub(crate) listener: Option, + #[webidl(with = webidl::event_listener_options)] + pub(crate) options: webidl::EventListenerOptions, +} + +pub(crate) struct AddEventListenerOptions<'s> { + pub(crate) options: webidl::EventListenerOptions, + pub(crate) signal: Option>, +} + +fn add_event_listener_options<'s>( + scope: &mut v8::PinScope<'s, '_>, + args: &v8::FunctionCallbackArguments<'s>, + index: i32, +) -> Result, webidl::WebIdlError> { + // Inherited members come first, followed by this dictionary's members in + // lexical order. Finish conversion even when the callback is null. + let value = args.get(index); + let options = webidl::add_event_listener_options_value(scope, value)?; + let signal = event_listener_signal_from_options_value(scope, value)?; + Ok(AddEventListenerOptions { options, signal }) +} diff --git a/moli-renderer-v8/src/lib.rs b/moli-renderer-v8/src/lib.rs index f1b39d1d62..377a010fc9 100644 --- a/moli-renderer-v8/src/lib.rs +++ b/moli-renderer-v8/src/lib.rs @@ -53,6 +53,7 @@ mod document_script_scheduler; mod document_task_lane; mod dom_parser; mod dynamic_script_owner; +mod event_listener_args; mod exception_reporting; mod frame_owner_model; mod host; diff --git a/moli-renderer-v8/src/native_bridge/abort/signal.rs b/moli-renderer-v8/src/native_bridge/abort/signal.rs index 7f32c1fe00..d2d5a646ed 100644 --- a/moli-renderer-v8/src/native_bridge/abort/signal.rs +++ b/moli-renderer-v8/src/native_bridge/abort/signal.rs @@ -1,27 +1,10 @@ use super::AbortStore; use super::event::{invoke_abort_event_callbacks, local_object_in_scope}; use crate::context_bootstrap::abort_signal_events; +use crate::event_listener_args::{AddEventListenerArgs, RemoveEventListenerArgs}; use crate::util::{context_host_ptr_from_global_bridge, v8str}; use crate::webidl; -#[derive(webidl::WebIdlArgs)] -#[webidl(prefix = "AbortSignal.addEventListener")] -struct AbortSignalAddEventListenerArgs { - #[webidl(required)] - event_type: String, - #[webidl(required, converter = "callback_interface", nullable)] - listener: Option, -} - -#[derive(webidl::WebIdlArgs)] -#[webidl(prefix = "AbortSignal.removeEventListener")] -struct AbortSignalRemoveEventListenerArgs { - #[webidl(required)] - event_type: String, - #[webidl(required, converter = "callback_interface", nullable)] - listener: Option, -} - #[derive(webidl::WebIdlArgs)] #[webidl(prefix = "AbortSignal.dispatchEvent")] struct AbortSignalDispatchEventArgs<'s> { @@ -43,7 +26,7 @@ pub(crate) fn abort_signal_add_event_listener_callback<'s>( rv.set_undefined(); return; } - let Some(parsed) = webidl::parse_args::(scope, &args) else { + let Some(parsed) = webidl::parse_args::(scope, &args) else { rv.set_undefined(); return; }; @@ -51,7 +34,7 @@ pub(crate) fn abort_signal_add_event_listener_callback<'s>( rv.set_undefined(); return; }; - let options = webidl::event_listener_options(scope, &args, 2, true); + let options = parsed.options.options; unsafe { &mut *host_ptr }.register_abort_signal_event_listener( scope, signal, @@ -76,8 +59,7 @@ pub(crate) fn abort_signal_remove_event_listener_callback<'s>( rv.set_undefined(); return; } - let Some(parsed) = webidl::parse_args::(scope, &args) - else { + let Some(parsed) = webidl::parse_args::(scope, &args) else { rv.set_undefined(); return; }; @@ -85,7 +67,7 @@ pub(crate) fn abort_signal_remove_event_listener_callback<'s>( rv.set_undefined(); return; }; - let capture = webidl::event_listener_options(scope, &args, 2, true).capture; + let capture = parsed.options.capture; unsafe { &mut *host_ptr }.unregister_abort_signal_event_listener( scope, signal, diff --git a/moli-renderer-v8/src/script_vm/tests/browser_api/event_listener_options.rs b/moli-renderer-v8/src/script_vm/tests/browser_api/event_listener_options.rs new file mode 100644 index 0000000000..c51086d9ab --- /dev/null +++ b/moli-renderer-v8/src/script_vm/tests/browser_api/event_listener_options.rs @@ -0,0 +1,254 @@ +use super::service_worker_drain::drain_service_worker_test_turn; +use super::*; + +const OPTIONS_PROBE: &str = r#"function checkOptions(targets) { + const rows=[];let serial=0; + for(const [targetName,create] of targets) for(const method of ['addEventListener','removeEventListener']) + for(const callbackKind of ['function','null','primitive']) for(const fail of ['none','capture','once','passive','signal']) { + const [target,eventType,cleanup]=create(++serial); + const trace=[],marker={};let count=0,error=null; + const listener=()=>count++; + const callback=callbackKind==='function'?listener:callbackKind==='null'?null:42; + if(method==='removeEventListener')target.addEventListener(eventType,listener); + const type={toString(){trace.push('type');return eventType;}}; + const options=new Proxy({}, { + has(_,name){trace.push('has:'+name);return true;}, + get(_,name){trace.push(String(name));if(name===fail)throw marker;return name==='signal'?undefined:false;} + }); + try{target[method](type,callback,options);}catch(e){error=e===marker?'marker':e.name;} + target.dispatchEvent(new Event(eventType)); + target.removeEventListener(eventType,listener); + rows.push({target:targetName,method,callback:callbackKind,fail,trace,error,count}); + if(cleanup)cleanup(); + } + return rows; +} +"#; + +const PORT_PROBE: &str = r#"async function checkPortOptions() { + const targets=[['MessagePort',()=>{const channel=new MessageChannel();return [channel.port1,'message',()=>{channel.port1.close();channel.port2.close();},()=>channel.port2.postMessage('probe')];}]]; + const rows=[];let serial=0; + for(const [targetName,create] of targets) for(const method of ['addEventListener','removeEventListener']) + for(const callbackKind of ['function','null','primitive']) for(const fail of ['none','capture','once','passive','signal']) { + const [target,eventType,cleanup,send]=create(++serial); + const trace=[],marker={};let count=0,error=null; + const listener=()=>count++; + const callback=callbackKind==='function'?listener:callbackKind==='null'?null:42; + if(method==='removeEventListener')target.addEventListener(eventType,listener); + const type={toString(){trace.push('type');return eventType;}}; + const options=new Proxy({}, { + has(_,name){trace.push('has:'+name);return true;}, + get(_,name){trace.push(String(name));if(name===fail)throw marker;return name==='signal'?undefined:false;} + }); + try{target[method](type,callback,options);}catch(e){error=e===marker?'marker':e.name;} + await new Promise(resolve=>{target.onmessage=resolve;target.start();send();}); + target.removeEventListener(eventType,listener); + rows.push({target:targetName,method,callback:callbackKind,fail,trace,error,count}); + if(cleanup)cleanup(); + } + return rows; +} +"#; + +const PAGE_PROBE: &str = r#"(() => { + const frame=document.body.appendChild(document.createElement('iframe')); + const rows=checkOptions([ + ['Window',n=>[window,'options-'+n]], + ['child Window',n=>[frame.contentWindow,'options-'+n]], + ['Document',n=>[document,'options-'+n]], + ['Element',n=>[document.createElement('div'),'options-'+n]], + ['EventTarget',n=>[new EventTarget(),'options-'+n]], + ['FileReader',n=>[new FileReader(),'options-'+n]], + ['AbortSignal',()=>[new AbortController().signal,'abort']] + ]);frame.remove();return rows; +})()"#; + +const WORKER_TARGETS: &str = r#"checkOptions([ + ['WorkerGlobalScope',n=>[self,'options-'+n]], + ['EventTarget',n=>[new EventTarget(),'options-'+n]], + ['FileReader',n=>[new FileReader(),'options-'+n]], + ['AbortSignal',()=>[new AbortController().signal,'abort']] +])"#; + +const SIGNAL_PROBE: &str = r#"(() => { + const frame=document.body.appendChild(document.createElement('iframe')); + const child=frame.contentWindow; + const channel=new MessageChannel(); + const targets=[window,child,document,document.createElement('div'),new EventTarget(),new FileReader(),new AbortController().signal,channel.port1]; + const localSignal=new AbortController().signal,childSignal=new child.AbortController().signal; + const revoked=Proxy.revocable(localSignal,{});revoked.revoke(); + const signals=[['undefined',undefined],['null',null],['number',1],['object',{}],['forged',Object.create(AbortSignal.prototype)],['proxy',new Proxy(localSignal,{})],['revoked',revoked.proxy],['local',localSignal],['child',childSignal]]; + const rows=[]; + for(let index=0;index", + ); + let value = vm + .eval(&format!("{OPTIONS_PROBE}\nJSON.stringify({PAGE_PROBE})")) + .unwrap(); + assert_option_rows(&serde_json::from_str(&value).unwrap(), 210); +} + +#[test] +fn event_listener_options_validate_signal_even_for_null_callbacks() { + let mut vm = new_parsed_test_vm( + "https://event-listener-signal-conversion.test/", + "", + ); + let value = vm.eval(&format!("JSON.stringify({SIGNAL_PROBE})")).unwrap(); + let rows: serde_json::Value = serde_json::from_str(&value).unwrap(); + let rows = rows.as_array().unwrap(); + assert_eq!(rows.len(), 144); + for row in rows { + let add = row["method"] == "addEventListener"; + let valid = matches!( + row["kind"].as_str().unwrap(), + "undefined" | "local" | "child" + ); + let expected = if add && !valid { + serde_json::json!("TypeError") + } else { + serde_json::Value::Null + }; + assert_eq!(row["error"], expected, "{row}"); + assert_eq!( + row["realm"], + if expected.is_null() { + serde_json::Value::Null + } else { + serde_json::json!(true) + }, + "{row}" + ); + assert_eq!( + row["trace"], + if add { + serde_json::json!(["capture", "once", "passive", "signal"]) + } else { + serde_json::json!(["capture"]) + }, + "{row}" + ); + } +} + +#[tokio::test] +async fn worker_event_listener_options_share_conversion_and_exception_semantics() { + for shared in [false, true] { + let loader = ResourceRequestClient::new(&moli_fetch::FetchConfig::default()).unwrap(); + let (mut vm, browser_context_runtime) = + new_service_worker_page_test_vm_with_loader_and_browser_context_runtime( + "https://worker-listener-options.test/", + &loader, + ); + let worker_source = if shared { + format!( + "{OPTIONS_PROBE}\n{PORT_PROBE}\nonconnect=async e=>e.ports[0].postMessage({{options:{WORKER_TARGETS},ports:await checkPortOptions()}});" + ) + } else { + format!( + "{OPTIONS_PROBE}\n{PORT_PROBE}\n(async()=>postMessage({{options:{WORKER_TARGETS},ports:await checkPortOptions()}}))();" + ) + }; + let worker_source = serde_json::to_string(&worker_source).unwrap(); + let constructor = if shared { "SharedWorker" } else { "Worker" }; + let port = if shared { "worker.port" } else { "worker" }; + let cleanup = if shared { + "port.close()" + } else { + "worker.terminate()" + }; + vm.eval(&format!(r#" + const url=URL.createObjectURL(new Blob([{worker_source}],{{type:'text/javascript'}})); + const worker=new {constructor}(url),port={port}; + port.onmessage=e=>{{globalThis.optionsResult=e.data;{cleanup};URL.revokeObjectURL(url);}}; + worker.onerror=e=>{{globalThis.optionsResult=String(e.message);}}; + "#)).unwrap(); + tokio::time::timeout(std::time::Duration::from_secs(10), async { + while vm.eval("globalThis.optionsResult !== undefined").unwrap() != "true" { + browser_context_runtime.drain_shared_worker_service_lane(); + drain_service_worker_test_turn(&mut vm, &browser_context_runtime, &loader).await; + } + }) + .await + .expect("worker options probe should settle"); + let value = vm.eval("JSON.stringify(globalThis.optionsResult)").unwrap(); + let value: serde_json::Value = serde_json::from_str(&value).unwrap(); + assert_option_rows(&value["options"], 120); + assert_option_rows(&value["ports"], 30); + } +} + +#[tokio::test] +async fn message_port_options_errors_preserve_actual_message_listeners() { + let loader = ResourceRequestClient::new(&moli_fetch::FetchConfig::default()).unwrap(); + let (mut vm, browser_context_runtime) = + new_service_worker_page_test_vm_with_loader_and_browser_context_runtime( + "https://message-port-options.test/", + &loader, + ); + vm.eval(&format!("{PORT_PROBE}\ncheckPortOptions().then(value=>globalThis.portOptionsResult=value,error=>globalThis.portOptionsResult=String(error));")).unwrap(); + tokio::time::timeout(std::time::Duration::from_secs(10), async { + while vm + .eval("globalThis.portOptionsResult !== undefined") + .unwrap() + != "true" + { + drain_service_worker_test_turn(&mut vm, &browser_context_runtime, &loader).await; + } + }) + .await + .expect("message port options probe should settle"); + let value = vm + .eval("JSON.stringify(globalThis.portOptionsResult)") + .unwrap(); + assert_option_rows(&serde_json::from_str(&value).unwrap(), 30); +} diff --git a/moli-renderer-v8/src/script_vm/tests/browser_api/mod.rs b/moli-renderer-v8/src/script_vm/tests/browser_api/mod.rs index 564df3a294..2a1246becd 100644 --- a/moli-renderer-v8/src/script_vm/tests/browser_api/mod.rs +++ b/moli-renderer-v8/src/script_vm/tests/browser_api/mod.rs @@ -17,6 +17,7 @@ mod crypto_subtle_x25519; mod date_locale; mod details; mod event_handlers; +mod event_listener_options; mod events_selection_storage; mod fullscreen; mod gamepad; diff --git a/moli-renderer-v8/src/window_host.rs b/moli-renderer-v8/src/window_host.rs index 494c478788..512ed7aff6 100644 --- a/moli-renderer-v8/src/window_host.rs +++ b/moli-renderer-v8/src/window_host.rs @@ -41,7 +41,7 @@ use super::{ }, script_provenance::CompiledStringProvenance, util::{ - callback_arg_string, context_host_from_global_bridge, context_host_ptr_from_global_bridge, + context_host_from_global_bridge, context_host_ptr_from_global_bridge, context_host_ptr_from_window_object, define_non_enumerable_static_bool_property, get_private_value, object_bool_property, object_number_property, script_base_url_from_continuation_data, script_base_url_from_host_defined_options, @@ -49,6 +49,7 @@ use super::{ }, webidl, }; +use crate::event_listener_args::{AddEventListenerArgs, RemoveEventListenerArgs}; use crate::web_api_interfaces; use moli_webapi_declare::{WebApiFunctionTemplate, WebApiObject}; use std::time::{Duration, SystemTime, UNIX_EPOCH}; @@ -144,124 +145,6 @@ struct IdleDeadlinePrototypeDeclaration { time_remaining: (), } -#[derive(webidl::WebIdlArgs)] -#[webidl(prefix = "EventTarget.addEventListener")] -struct WindowAddEventListenerArgs<'s> { - #[webidl(with = window_add_event_listener_call)] - call: webidl::ParseOutcome>, -} - -#[derive(webidl::WebIdlArgs)] -#[webidl(prefix = "EventTarget.removeEventListener")] -struct WindowRemoveEventListenerArgs<'s> { - #[webidl(with = window_remove_event_listener_call)] - call: webidl::ParseOutcome>, -} - -struct WindowAddEventListenerCall<'s> { - event_type: String, - callback: v8::Local<'s, v8::Object>, - callback_relevant_context: v8::Local<'s, v8::Context>, - incumbent_context: v8::Local<'s, v8::Context>, - options: webidl::EventListenerOptions, - signal: Option>, -} - -struct WindowRemoveEventListenerCall<'s> { - event_type: String, - callback: v8::Local<'s, v8::Object>, - options: webidl::EventListenerOptions, -} - -fn window_add_event_listener_call<'s>( - scope: &mut v8::PinScope<'s, '_>, - args: &v8::FunctionCallbackArguments<'s>, - _index: i32, -) -> Result>, webidl::WebIdlError> { - let Some(event_type) = callback_arg_string(scope, args, 0) else { - return Ok(webidl::ParseOutcome::Skip); - }; - let options = webidl::event_listener_options(scope, args, 2, true); - let listener_arg = args.get(1); - let current_context = scope.get_current_context(); - let callback = if let Ok(function) = v8::Local::::try_from(listener_arg) { - let callback = v8::Local::::from(function); - let callback_relevant_context = callback - .get_creation_context(scope) - .unwrap_or(current_context); - Some((callback, callback_relevant_context)) - } else if listener_arg.is_object() && !listener_arg.is_null_or_undefined() { - let Ok(object) = v8::Local::::try_from(listener_arg) else { - return Ok(webidl::ParseOutcome::Skip); - }; - let callback_relevant_context = object - .get_creation_context(scope) - .unwrap_or(current_context); - Some((object, callback_relevant_context)) - } else { - None - }; - let Some((callback, callback_relevant_context)) = callback else { - return Ok(webidl::ParseOutcome::Skip); - }; - let incumbent_context = scope.get_incumbent_context().unwrap_or(current_context); - Ok(webidl::ParseOutcome::Parsed(WindowAddEventListenerCall { - event_type, - callback, - callback_relevant_context, - incumbent_context, - signal: signal_from_options_value(scope, args.get(2)), - options, - })) -} - -fn window_remove_event_listener_call<'s>( - scope: &mut v8::PinScope<'s, '_>, - args: &v8::FunctionCallbackArguments<'s>, - _index: i32, -) -> Result>, webidl::WebIdlError> { - let Some(event_type) = callback_arg_string(scope, args, 0) else { - return Ok(webidl::ParseOutcome::Skip); - }; - let options = webidl::event_listener_options(scope, args, 2, true); - let listener_arg = args.get(1); - let callback = if let Ok(function) = v8::Local::::try_from(listener_arg) { - Some(v8::Local::::from(function)) - } else if listener_arg.is_object() && !listener_arg.is_null_or_undefined() { - v8::Local::::try_from(listener_arg).ok() - } else { - None - }; - let Some(callback) = callback else { - return Ok(webidl::ParseOutcome::Skip); - }; - Ok(webidl::ParseOutcome::Parsed( - WindowRemoveEventListenerCall { - event_type, - callback, - options, - }, - )) -} - -fn signal_from_options_value<'s>( - scope: &mut v8::PinScope<'s, '_>, - value: v8::Local<'s, v8::Value>, -) -> Option> { - let Ok(options) = v8::Local::::try_from(value) else { - return None; - }; - options - .get(scope, v8str(scope, "signal").into()) - .and_then(|value| { - if value.is_null_or_undefined() { - None - } else { - v8::Local::::try_from(value).ok() - } - }) -} - fn capture_window_event_target_receiver<'s>( scope: &mut v8::PinScope<'s, '_>, receiver: v8::Local<'s, v8::Object>, @@ -316,16 +199,17 @@ pub(super) fn event_target_add_event_listener_callback<'s>( let Ok(window_receiver) = capture_window_event_target_receiver(scope, args.this(), host) else { return; }; - let parsed = webidl::parse_args::(scope, &args); - let Some(parsed) = parsed else { + let parsed = webidl::parse_args::(scope, &args); + let Some(call) = parsed else { return; }; - let webidl::ParseOutcome::Parsed(call) = parsed.call else { + let Some(listener) = call.listener else { return; }; - let capture = call.options.capture; - let once = call.options.once; - let signal = call.signal; + let options = call.options.options; + let capture = options.capture; + let once = options.once; + let signal = call.options.signal.map(|signal| signal.value()); if let Some(signal) = signal && host.abort_signal_aborted(scope, signal) { @@ -343,17 +227,20 @@ pub(super) fn event_target_add_event_listener_callback<'s>( throw_type_error(scope, "Illegal invocation"); return; }; - let passive = call - .options + let passive = options .passive .unwrap_or_else(|| default_passive_value(host, target, &call.event_type)); + let callback = v8::Local::::try_from(listener.value(scope)) + .expect("converted EventListener must remain an object"); + let callback_relevant_context = listener.relevant_context(scope); + let incumbent_context = listener.incumbent_context(scope); let Some(callback_id) = host.register_target_event_listener( scope, target, &call.event_type, - call.callback, - call.callback_relevant_context, - call.incumbent_context, + callback, + callback_relevant_context, + incumbent_context, capture, once, passive, @@ -434,10 +321,10 @@ pub(super) fn event_target_remove_event_listener_callback<'s>( let Ok(window_receiver) = capture_window_event_target_receiver(scope, args.this(), host) else { return; }; - let Some(parsed) = webidl::parse_args::(scope, &args) else { + let Some(call) = webidl::parse_args::(scope, &args) else { return; }; - let webidl::ParseOutcome::Parsed(call) = parsed.call else { + let Some(listener) = call.listener else { return; }; let capture = call.options.capture; @@ -456,7 +343,9 @@ pub(super) fn event_target_remove_event_listener_callback<'s>( throw_type_error(scope, "Illegal invocation"); return; }; - host.remove_registered_event_listener(scope, target, &call.event_type, call.callback, capture); + let callback = v8::Local::::try_from(listener.value(scope)) + .expect("converted EventListener must remain an object"); + host.remove_registered_event_listener(scope, target, &call.event_type, callback, capture); } pub(super) fn event_target_dispatch_event_callback<'s>( diff --git a/moli-renderer-v8/src/worker/abort/event_listener.rs b/moli-renderer-v8/src/worker/abort/event_listener.rs index 495e1c0291..fed0dfcbf8 100644 --- a/moli-renderer-v8/src/worker/abort/event_listener.rs +++ b/moli-renderer-v8/src/worker/abort/event_listener.rs @@ -9,6 +9,7 @@ use crate::context_bootstrap::{ EVENT_PASSIVE_SLOT, EVENT_STOP_IMMEDIATE_PROPAGATION_SLOT, construct_original_event, event_internal_bool_flag, set_event_internal_flag, }; +use crate::event_listener_args::{AddEventListenerArgs, RemoveEventListenerArgs}; use crate::exception_reporting::{CallbackExceptionLogLevel, invoke_callback}; use crate::util::v8str; use crate::webidl; @@ -49,24 +50,6 @@ struct PreparedWorkerAbortListener { passive: bool, } -#[derive(webidl::WebIdlArgs)] -#[webidl(prefix = "AbortSignal.addEventListener")] -struct WorkerAbortAddEventListenerArgs { - #[webidl(required)] - event_type: String, - #[webidl(required, converter = "callback_interface", nullable)] - listener: Option, -} - -#[derive(webidl::WebIdlArgs)] -#[webidl(prefix = "AbortSignal.removeEventListener")] -struct WorkerAbortRemoveEventListenerArgs { - #[webidl(required)] - event_type: String, - #[webidl(required, converter = "callback_interface", nullable)] - listener: Option, -} - #[derive(webidl::WebIdlArgs)] #[webidl(prefix = "AbortSignal.dispatchEvent")] struct WorkerAbortDispatchEventArgs<'s> { @@ -215,7 +198,7 @@ pub(crate) fn worker_abort_signal_add_event_listener_callback<'s>( rv.set_undefined(); return; }; - let Some(parsed) = webidl::parse_args::(scope, &args) else { + let Some(parsed) = webidl::parse_args::(scope, &args) else { rv.set_undefined(); return; }; @@ -223,7 +206,7 @@ pub(crate) fn worker_abort_signal_add_event_listener_callback<'s>( rv.set_undefined(); return; }; - let options = webidl::event_listener_options(scope, &args, 2, true); + let options = parsed.options.options; store.borrow_mut().register_event_listener( scope, signal_id, @@ -248,8 +231,7 @@ pub(crate) fn worker_abort_signal_remove_event_listener_callback<'s>( rv.set_undefined(); return; }; - let Some(parsed) = webidl::parse_args::(scope, &args) - else { + let Some(parsed) = webidl::parse_args::(scope, &args) else { rv.set_undefined(); return; }; @@ -257,7 +239,7 @@ pub(crate) fn worker_abort_signal_remove_event_listener_callback<'s>( rv.set_undefined(); return; }; - let capture = webidl::event_listener_options(scope, &args, 2, true).capture; + let capture = parsed.options.capture; store.borrow_mut().remove_event_listener( scope, signal_id, diff --git a/moli-webidl/src/helpers.rs b/moli-webidl/src/helpers.rs index 1aff94c31c..834ed28d29 100644 --- a/moli-webidl/src/helpers.rs +++ b/moli-webidl/src/helpers.rs @@ -1,4 +1,4 @@ -use crate::types::EventListenerOptionsMembers; +use crate::types::{AddEventListenerOptionsMembers, EventListenerOptionsMembers}; use crate::{ Context, DomString, EventListenerOptions, UnrestrictedDouble, WebIdlError, legacy_optional_member, parse_dictionary_object, @@ -170,65 +170,49 @@ pub fn optional_number_property<'s>( .map(Into::into) } -/// Parses the third argument shape used by event listener registration. +/// Converts `(AddEventListenerOptions or boolean)` through `passive`. /// -/// Boolean values use the legacy capture-only path. Object values are parsed as -/// `AddEventListenerOptions`. When `observe_passive` is true, the `passive` -/// member is read even if the resulting value is not otherwise needed, matching -/// sites that observe getter side effects. +/// The caller must convert the platform-specific `signal` member next, before +/// changing the listener list. Getter exceptions stop conversion immediately. +pub fn add_event_listener_options_value<'s>( + scope: &mut v8::PinScope<'s, '_>, + value: v8::Local<'s, v8::Value>, +) -> Result { + let Ok(object) = v8::Local::::try_from(value) else { + return Ok(EventListenerOptions { + capture: value.boolean_value(scope), + ..EventListenerOptions::default() + }); + }; + let parsed = parse_dictionary_object::(scope, object)?; + Ok(EventListenerOptions { + capture: parsed.capture, + once: parsed.once, + passive: parsed.passive, + }) +} + +/// Converts removeEventListener's `(EventListenerOptions or boolean)` argument. +/// Only `capture` belongs to this dictionary; no registration-only getters run. pub fn event_listener_options<'s>( scope: &mut v8::PinScope<'s, '_>, args: &v8::FunctionCallbackArguments<'s>, index: i32, - observe_passive: bool, -) -> EventListenerOptions { - if args.length() <= index { - return EventListenerOptions::default(); - } - event_listener_options_value(scope, args.get(index), observe_passive) +) -> Result { + event_listener_options_value(scope, args.get(index)) } pub fn event_listener_options_value<'s>( scope: &mut v8::PinScope<'s, '_>, value: v8::Local<'s, v8::Value>, - observe_passive: bool, -) -> EventListenerOptions { - if is_nullish(value) { - return EventListenerOptions::default(); - } - if !value.is_object() || value.is_boolean() { - return EventListenerOptions { - capture: value.boolean_value(scope), - once: false, - passive: None, - }; - } - let Ok(object) = v8::Local::::try_from(value) else { - return EventListenerOptions::default(); +) -> Result { + let capture = if let Ok(object) = v8::Local::::try_from(value) { + parse_dictionary_object::(scope, object)?.capture + } else { + value.boolean_value(scope) }; - if observe_passive { - let _ = property(scope, object, "passive"); - } - parse_dictionary_object::(scope, object) - .map(|parsed| EventListenerOptions { - capture: parsed.capture, - once: parsed.once, - passive: parsed.passive, - }) - .unwrap_or_default() -} - -pub fn event_listener_once_value<'s>( - scope: &mut v8::PinScope<'s, '_>, - value: v8::Local<'s, v8::Value>, -) -> bool { - event_listener_options_value(scope, value, false).once -} - -pub fn event_listener_once_option<'s>( - scope: &mut v8::PinScope<'s, '_>, - args: &v8::FunctionCallbackArguments<'s>, - index: i32, -) -> bool { - event_listener_options(scope, args, index, false).once + Ok(EventListenerOptions { + capture, + ..EventListenerOptions::default() + }) } diff --git a/moli-webidl/src/lib.rs b/moli-webidl/src/lib.rs index c41ccb40da..b3848b5fdd 100644 --- a/moli-webidl/src/lib.rs +++ b/moli-webidl/src/lib.rs @@ -49,11 +49,11 @@ pub use convert::{ }; pub use error::{Context, WebIdlError, WebIdlErrorKind}; pub use helpers::{ - dictionary_arg, dictionary_value, event_listener_once_option, event_listener_once_value, - event_listener_options, event_listener_options_value, is_nullish, optional_number_property, - optional_object_arg, optional_string_property, property, property_non_nullish, - property_non_undefined, property_result, symbol_property_result, throw_dom_exception, - throw_error, throw_index_size_error, throw_type_error, v8_string, + add_event_listener_options_value, dictionary_arg, dictionary_value, event_listener_options, + event_listener_options_value, is_nullish, optional_number_property, optional_object_arg, + optional_string_property, property, property_non_nullish, property_non_undefined, + property_result, symbol_property_result, throw_dom_exception, throw_error, + throw_index_size_error, throw_type_error, v8_string, }; pub use moli_webidl_callback::{ PreparedWebIdlCallbackFunction, PreparedWebIdlCallbackInterface, WebIdlCallbackFunction, diff --git a/moli-webidl/src/types.rs b/moli-webidl/src/types.rs index 7edf3e7cbf..c3f318e5b4 100644 --- a/moli-webidl/src/types.rs +++ b/moli-webidl/src/types.rs @@ -143,10 +143,17 @@ pub struct EventListenerOptions { } #[derive(WebIdlDictionary)] -#[webidl(prefix = "AddEventListenerOptions")] +#[webidl(prefix = "EventListenerOptions")] pub(crate) struct EventListenerOptionsMembers { #[webidl(default = false)] pub(crate) capture: bool, +} + +#[derive(WebIdlDictionary)] +#[webidl(prefix = "AddEventListenerOptions")] +pub(crate) struct AddEventListenerOptionsMembers { + #[webidl(default = false)] + pub(crate) capture: bool, #[webidl(default = false)] pub(crate) once: bool, pub(crate) passive: Option, diff --git a/moli-wpt-compat/fixtures/wpt/ported/fileapi/filereader-basic.html b/moli-wpt-compat/fixtures/wpt/ported/fileapi/filereader-basic.html index 81f59e0797..628e54c7a4 100644 --- a/moli-wpt-compat/fixtures/wpt/ported/fileapi/filereader-basic.html +++ b/moli-wpt-compat/fixtures/wpt/ported/fileapi/filereader-basic.html @@ -69,10 +69,20 @@ test(function () { reader.addEventListener(undefined, listener); reader.removeEventListener(undefined, listener); - reader.addEventListener("load"); + assert_throws_name( + "TypeError", + () => reader.addEventListener("load"), + "addEventListener callback is required" + ); + assert_throws_name( + "TypeError", + () => reader.removeEventListener("load"), + "removeEventListener callback is required" + ); reader.addEventListener("load", null); - reader.removeEventListener("load"); + reader.addEventListener("load", undefined); reader.removeEventListener("load", null); + reader.removeEventListener("load", undefined); }, "FileReader listener arguments follow WebIDL boundary conversion"); test(function () {