From d15274d1e4c6d3e28dcb2c5d51539dcdf3e2cfed Mon Sep 17 00:00:00 2001 From: ldm0 Date: Tue, 6 Oct 2026 05:10:26 +0800 Subject: [PATCH] refactor(streams): derive dictionary arguments in conversion stages Derive strategy and codec/iterator options while retaining highWaterMark presence and operation-specific defaults in the stream algorithm. Convert and validate TransformStream writable strategy before its readable strategy, then read transformer members. Derive QueuingStrategyInit with required unrestricted highWaterMark and cover conversion/error order across realms. Validation on the combined tree: cargo fmt --all; cargo clippy --workspace --all-targets --all-features -- -D warnings; cargo nextest run --no-fail-fast (19639 passed, 16 skipped; CARGO_BUILD_JOBS=1). --- .../src/context_bootstrap/stream_adapter.rs | 7 +- .../stream_adapter/callbacks.rs | 60 +++--- .../src/context_bootstrap/streams.rs | 7 +- .../context_bootstrap/streams/constructors.rs | 174 +++++++++--------- .../src/context_bootstrap/streams/readable.rs | 14 +- .../src/script_vm/tests/streams.rs | 52 ++++++ 6 files changed, 165 insertions(+), 149 deletions(-) diff --git a/moli-renderer-v8/src/context_bootstrap/stream_adapter.rs b/moli-renderer-v8/src/context_bootstrap/stream_adapter.rs index ae9e5f134a..5928dc9776 100644 --- a/moli-renderer-v8/src/context_bootstrap/stream_adapter.rs +++ b/moli-renderer-v8/src/context_bootstrap/stream_adapter.rs @@ -54,13 +54,12 @@ mod utils; mod writable; pub(in crate::context_bootstrap) use callbacks::{ - StreamQueuingStrategy, StreamWebIdlCallbackCarrier, WebIdlReadableStreamSource, + QueuingStrategyMembers, StreamWebIdlCallbackCarrier, WebIdlReadableStreamSource, WebIdlTransformStreamTransformer, WebIdlWritableStreamSink, callback_carrier_value, invoke_stored_stream_algorithm, invoke_stored_stream_promise_algorithm, invoke_stored_stream_size_algorithm, invoke_stream_webidl_callback, - parse_readable_stream_source_object, parse_stream_strategy_arg, - parse_transform_stream_transformer_object, parse_writable_stream_sink_object, - stored_stream_algorithm_is_webidl, + parse_readable_stream_source_object, parse_transform_stream_transformer_object, + parse_writable_stream_sink_object, stored_stream_algorithm_is_webidl, }; pub(super) use construction::{ initialize_transform_stream_object, initialize_webidl_readable_stream_object, diff --git a/moli-renderer-v8/src/context_bootstrap/stream_adapter/callbacks.rs b/moli-renderer-v8/src/context_bootstrap/stream_adapter/callbacks.rs index 613449c469..2287cb2717 100644 --- a/moli-renderer-v8/src/context_bootstrap/stream_adapter/callbacks.rs +++ b/moli-renderer-v8/src/context_bootstrap/stream_adapter/callbacks.rs @@ -89,7 +89,7 @@ struct TransformerMembers<'s> { /// `highWaterMark` before `size`. #[derive(Default, webidl::WebIdlDictionary)] #[webidl(prefix = "QueuingStrategy")] -struct QueuingStrategyMembers { +pub(in crate::context_bootstrap) struct QueuingStrategyMembers { #[webidl(name = "highWaterMark", converter = "unrestricted_double")] high_water_mark: Option, #[webidl(converter = "callback_function")] @@ -127,16 +127,6 @@ pub(in crate::context_bootstrap) struct StreamQueuingStrategy { pub(in crate::context_bootstrap) size: Option, } -impl StreamQueuingStrategy { - pub(in crate::context_bootstrap) const fn without_size(high_water_mark: f64) -> Self { - Self { - high_water_mark, - high_water_mark_provided: true, - size: None, - } - } -} - #[derive(Clone, Copy)] pub(in crate::context_bootstrap) struct StreamWebIdlCallbackCarrier<'s> { object: v8::Local<'s, v8::Object>, @@ -238,35 +228,27 @@ pub(in crate::context_bootstrap) fn parse_transform_stream_transformer_object<'s }) } -pub(in crate::context_bootstrap) fn parse_stream_strategy_arg<'s>( - scope: &mut v8::PinScope<'s, '_>, - args: &v8::FunctionCallbackArguments<'s>, - index: i32, - prefix: &'static str, - default_high_water_mark: f64, -) -> Result { - let context = webidl::Context::argument(prefix, (index + 1) as usize); - let Some(object) = webidl::dictionary_arg(args, index, context)? else { - return Ok(StreamQueuingStrategy { - high_water_mark: default_high_water_mark, - high_water_mark_provided: false, - size: None, - }); - }; - let members = webidl::parse_dictionary_object::(scope, object)?; - let high_water_mark = members.high_water_mark.unwrap_or(default_high_water_mark); - if moli_streams::numeric::validate_high_water_mark(high_water_mark).is_err() { - crate::util::throw_range_error( - scope, - "A queuing strategy's highWaterMark must be a nonnegative, non-NaN number", - ); - return Err(webidl::WebIdlError::pending_exception(context)); +impl QueuingStrategyMembers { + pub(in crate::context_bootstrap) fn resolve( + self, + scope: &mut v8::PinScope<'_, '_>, + context: webidl::Context, + default_high_water_mark: f64, + ) -> Result { + let high_water_mark = self.high_water_mark.unwrap_or(default_high_water_mark); + if moli_streams::numeric::validate_high_water_mark(high_water_mark).is_err() { + crate::util::throw_range_error( + scope, + "A queuing strategy's highWaterMark must be a nonnegative, non-NaN number", + ); + return Err(webidl::WebIdlError::pending_exception(context)); + } + Ok(StreamQueuingStrategy { + high_water_mark, + high_water_mark_provided: self.high_water_mark.is_some(), + size: self.size, + }) } - Ok(StreamQueuingStrategy { - high_water_mark, - high_water_mark_provided: members.high_water_mark.is_some(), - size: members.size, - }) } /// Invokes one page-supplied callback through Web IDL while leaving exception, diff --git a/moli-renderer-v8/src/context_bootstrap/streams.rs b/moli-renderer-v8/src/context_bootstrap/streams.rs index db33a54988..0797a86b8e 100644 --- a/moli-renderer-v8/src/context_bootstrap/streams.rs +++ b/moli-renderer-v8/src/context_bootstrap/streams.rs @@ -1,11 +1,10 @@ use super::specs::{ConstructorKind, ConstructorSpec}; use super::stream_adapter::{ - StreamQueuingStrategy, cancel_readable_stream, close_stream, enqueue_chunk, + QueuingStrategyMembers, cancel_readable_stream, close_stream, enqueue_chunk, initialize_transform_stream_object, initialize_webidl_readable_stream_object, initialize_webidl_transform_stream_object, initialize_webidl_writable_stream_object, - parse_readable_stream_source_object, parse_stream_strategy_arg, - parse_transform_stream_transformer_object, parse_writable_stream_sink_object, - readable_stream_byob_request_respond_callback, + parse_readable_stream_source_object, parse_transform_stream_transformer_object, + parse_writable_stream_sink_object, readable_stream_byob_request_respond_callback, readable_stream_byob_request_respond_with_new_view_callback, readable_stream_byob_request_view_getter, readable_stream_is_byte_stream, readable_stream_locked, rejected_promise_value, set_resolved_promise, stream_slot_object, diff --git a/moli-renderer-v8/src/context_bootstrap/streams/constructors.rs b/moli-renderer-v8/src/context_bootstrap/streams/constructors.rs index d45614156e..00ea081ea5 100644 --- a/moli-renderer-v8/src/context_bootstrap/streams/constructors.rs +++ b/moli-renderer-v8/src/context_bootstrap/streams/constructors.rs @@ -68,8 +68,8 @@ struct ByteLengthQueuingStrategyPrototypeDeclaration { struct ReadableStreamConstructorArgs<'s> { #[webidl(index = 0)] source: Option>, - #[webidl(index = 1, with = parse_readable_stream_strategy_arg)] - strategy: StreamQueuingStrategy, + #[webidl(index = 1, dictionary)] + strategy: QueuingStrategyMembers, } #[derive(webidl::WebIdlArgs)] @@ -77,8 +77,8 @@ struct ReadableStreamConstructorArgs<'s> { struct WritableStreamConstructorArgs<'s> { #[webidl(index = 0)] sink: Option>, - #[webidl(index = 1, with = parse_writable_stream_strategy_arg)] - strategy: StreamQueuingStrategy, + #[webidl(index = 1, dictionary)] + strategy: QueuingStrategyMembers, } #[derive(webidl::WebIdlArgs)] @@ -86,10 +86,17 @@ struct WritableStreamConstructorArgs<'s> { struct TransformStreamConstructorArgs<'s> { #[webidl(index = 0)] transformer: Option>, - #[webidl(index = 1, with = parse_transform_stream_writable_strategy_arg)] - writable_strategy: StreamQueuingStrategy, - #[webidl(index = 2, with = parse_transform_stream_readable_strategy_arg)] - readable_strategy: StreamQueuingStrategy, + #[webidl(index = 1, dictionary)] + writable_strategy: QueuingStrategyMembers, +} + +// Writable strategy conversion and validation must finish before reading the +// readable strategy. Keep this second conversion stage ahead of transformer reads. +#[derive(webidl::WebIdlArgs)] +#[webidl(prefix = "TransformStream")] +struct TransformStreamReadableStrategyArgs { + #[webidl(index = 2, dictionary)] + readable_strategy: QueuingStrategyMembers, } #[derive(webidl::WebIdlArgs)] @@ -97,22 +104,29 @@ struct TransformStreamConstructorArgs<'s> { struct TextDecoderStreamConstructorArgs { #[webidl(default = "utf-8")] label: String, - #[webidl(index = 1, with = parse_text_decoder_stream_options_arg)] + #[webidl(index = 1, dictionary)] options: TextDecoderStreamOptions, } #[derive(webidl::WebIdlArgs)] #[webidl(prefix = "CountQueuingStrategy")] struct CountQueuingStrategyConstructorArgs { - #[webidl(required, with = parse_queuing_strategy_init_arg)] - init: StreamQueuingStrategy, + #[webidl(required, dictionary)] + init: QueuingStrategyInit, } #[derive(webidl::WebIdlArgs)] #[webidl(prefix = "ByteLengthQueuingStrategy")] struct ByteLengthQueuingStrategyConstructorArgs { - #[webidl(required, with = parse_queuing_strategy_init_arg)] - init: StreamQueuingStrategy, + #[webidl(required, dictionary)] + init: QueuingStrategyInit, +} + +#[derive(webidl::WebIdlDictionary)] +#[webidl(prefix = "QueuingStrategyInit")] +struct QueuingStrategyInit { + #[webidl(required, converter = "unrestricted_double")] + high_water_mark: f64, } #[derive(Default, webidl::WebIdlDictionary)] @@ -139,6 +153,17 @@ pub(in crate::context_bootstrap) fn readable_stream_constructor_callback<'s>( let Some(parsed) = webidl::parse_args::>(scope, &args) else { return; }; + let strategy = + match parsed + .strategy + .resolve(scope, webidl::Context::argument("ReadableStream", 2), 1.0) + { + Ok(strategy) => strategy, + Err(error) => { + webidl::throw_error(scope, &error); + return; + } + }; let source = match parsed .source .map(|source| parse_readable_stream_source_object(scope, source)) @@ -151,7 +176,7 @@ pub(in crate::context_bootstrap) fn readable_stream_constructor_callback<'s>( } }; let byte_stream = source.as_ref().is_some_and(|source| source.byte_stream); - if byte_stream && parsed.strategy.size.is_some() { + if byte_stream && strategy.size.is_some() { throw_range_error( scope, "A byte ReadableStream cannot use a queuing strategy size function", @@ -167,10 +192,10 @@ pub(in crate::context_bootstrap) fn readable_stream_constructor_callback<'s>( throw_type_error(scope, "autoAllocateChunkSize must be greater than zero"); return; } - let high_water_mark = if byte_stream && !parsed.strategy.high_water_mark_provided { + let high_water_mark = if byte_stream && !strategy.high_water_mark_provided { 0.0 } else { - parsed.strategy.high_water_mark + strategy.high_water_mark }; initialize_webidl_readable_stream_object( scope, @@ -178,7 +203,7 @@ pub(in crate::context_bootstrap) fn readable_stream_constructor_callback<'s>( source, byte_stream, high_water_mark, - parsed.strategy.size, + strategy.size, ); rv.set(args.this().into()); } @@ -198,6 +223,17 @@ pub(in crate::context_bootstrap) fn writable_stream_constructor_callback<'s>( let Some(parsed) = webidl::parse_args::>(scope, &args) else { return; }; + let strategy = + match parsed + .strategy + .resolve(scope, webidl::Context::argument("WritableStream", 2), 1.0) + { + Ok(strategy) => strategy, + Err(error) => { + webidl::throw_error(scope, &error); + return; + } + }; let sink = match parsed .sink .map(|sink| parse_writable_stream_sink_object(scope, sink)) @@ -213,8 +249,8 @@ pub(in crate::context_bootstrap) fn writable_stream_constructor_callback<'s>( scope, args.this(), sink, - parsed.strategy.high_water_mark, - parsed.strategy.size, + strategy.high_water_mark, + strategy.size, ); rv.set(args.this().into()); } @@ -235,6 +271,32 @@ pub(in crate::context_bootstrap) fn transform_stream_constructor_callback<'s>( else { return; }; + let writable_strategy = match parsed.writable_strategy.resolve( + scope, + webidl::Context::argument("TransformStream", 2), + 1.0, + ) { + Ok(strategy) => strategy, + Err(error) => { + webidl::throw_error(scope, &error); + return; + } + }; + let Some(readable) = webidl::parse_args::(scope, &args) + else { + return; + }; + let readable_strategy = match readable.readable_strategy.resolve( + scope, + webidl::Context::argument("TransformStream", 3), + 0.0, + ) { + Ok(strategy) => strategy, + Err(error) => { + webidl::throw_error(scope, &error); + return; + } + }; let transformer = match parsed .transformer .map(|transformer| parse_transform_stream_transformer_object(scope, transformer)) @@ -251,10 +313,10 @@ pub(in crate::context_bootstrap) fn transform_stream_constructor_callback<'s>( args.this(), transformer, None, - parsed.writable_strategy.high_water_mark, - parsed.writable_strategy.size, - parsed.readable_strategy.high_water_mark, - parsed.readable_strategy.size, + writable_strategy.high_water_mark, + writable_strategy.size, + readable_strategy.high_water_mark, + readable_strategy.size, ); rv.set(args.this().into()); } @@ -567,69 +629,3 @@ fn byte_length_queuing_strategy_size_callback<'s>( }; rv.set(byte_length); } - -fn parse_readable_stream_strategy_arg<'s>( - scope: &mut v8::PinScope<'s, '_>, - args: &v8::FunctionCallbackArguments<'s>, - index: i32, -) -> Result { - parse_stream_strategy_arg(scope, args, index, "ReadableStream", 1.0) -} - -fn parse_writable_stream_strategy_arg<'s>( - scope: &mut v8::PinScope<'s, '_>, - args: &v8::FunctionCallbackArguments<'s>, - index: i32, -) -> Result { - parse_stream_strategy_arg(scope, args, index, "WritableStream", 1.0) -} - -fn parse_transform_stream_writable_strategy_arg<'s>( - scope: &mut v8::PinScope<'s, '_>, - args: &v8::FunctionCallbackArguments<'s>, - index: i32, -) -> Result { - parse_stream_strategy_arg(scope, args, index, "TransformStream", 1.0) -} - -fn parse_transform_stream_readable_strategy_arg<'s>( - scope: &mut v8::PinScope<'s, '_>, - args: &v8::FunctionCallbackArguments<'s>, - index: i32, -) -> Result { - parse_stream_strategy_arg(scope, args, index, "TransformStream", 0.0) -} - -fn parse_queuing_strategy_init_arg<'s>( - scope: &mut v8::PinScope<'s, '_>, - args: &v8::FunctionCallbackArguments<'s>, - index: i32, -) -> Result { - let context = webidl::Context::argument("QueuingStrategyInit", (index + 1) as usize); - let object = webidl::dictionary_arg(args, index, context)? - .ok_or_else(|| webidl::WebIdlError::missing_required(context))?; - let high_water_mark = object - .get(scope, v8str(scope, "highWaterMark").into()) - .ok_or_else(|| webidl::WebIdlError::pending_exception(context))?; - if high_water_mark.is_undefined() { - return Err(webidl::WebIdlError::custom_message( - "QueuingStrategyInit.highWaterMark is required", - )); - } - let high_water_mark = high_water_mark.number_value(scope).ok_or_else(|| { - webidl::WebIdlError::custom_message("QueuingStrategyInit.highWaterMark must be a number") - })?; - Ok(StreamQueuingStrategy::without_size(high_water_mark)) -} - -fn parse_text_decoder_stream_options_arg<'s>( - scope: &mut v8::PinScope<'s, '_>, - args: &v8::FunctionCallbackArguments<'s>, - index: i32, -) -> Result { - let context = webidl::Context::argument("TextDecoderStream", (index + 1) as usize); - webidl::dictionary_arg(args, index, context)? - .map(|object| webidl::parse_dictionary_object(scope, object)) - .transpose() - .map(|options| options.unwrap_or_default()) -} diff --git a/moli-renderer-v8/src/context_bootstrap/streams/readable.rs b/moli-renderer-v8/src/context_bootstrap/streams/readable.rs index 866edf2f8d..ce60bf1731 100644 --- a/moli-renderer-v8/src/context_bootstrap/streams/readable.rs +++ b/moli-renderer-v8/src/context_bootstrap/streams/readable.rs @@ -58,7 +58,7 @@ struct StreamPipeOptions<'s> { #[derive(webidl::WebIdlArgs)] #[webidl(prefix = "ReadableStream.values")] struct ReadableStreamValuesArgs { - #[webidl(with = readable_stream_iterator_options_arg)] + #[webidl(dictionary)] options: ReadableStreamIteratorOptions, } @@ -609,18 +609,6 @@ fn new_readable_stream_async_iterator_object<'s>( Some(v8::Local::new(scope, &iterator)) } -fn readable_stream_iterator_options_arg<'s>( - scope: &mut v8::PinScope<'s, '_>, - args: &v8::FunctionCallbackArguments<'s>, - index: i32, -) -> Result { - let context = webidl::Context::argument("ReadableStream.values", (index + 1) as usize); - webidl::dictionary_arg(args, index, context)? - .map(|object| webidl::parse_dictionary_object(scope, object)) - .transpose() - .map(|options| options.unwrap_or_default()) -} - pub(in crate::context_bootstrap) fn readable_stream_locked_getter<'s>( scope: &mut v8::PinScope<'s, '_>, args: v8::FunctionCallbackArguments<'s>, diff --git a/moli-renderer-v8/src/script_vm/tests/streams.rs b/moli-renderer-v8/src/script_vm/tests/streams.rs index a7bd734360..854ca6ecd2 100644 --- a/moli-renderer-v8/src/script_vm/tests/streams.rs +++ b/moli-renderer-v8/src/script_vm/tests/streams.rs @@ -1,5 +1,57 @@ use super::*; +#[test] +fn stream_dictionary_arguments_preserve_required_members_and_staged_strategy_validation() { + let mut vm = new_parsed_test_vm( + "https://stream-dictionary-args.test/", + "", + ); + let result = vm.eval(r#" + (() => { + const assert = (ok, message) => { if (!ok) throw new Error(message); }; + const child = document.getElementById('child').contentWindow; + const marker = {}; + for (const realm of [window, child]) { + for (const C of [realm.CountQueuingStrategy, realm.ByteLengthQueuingStrategy]) { + for (const args of [[], [undefined], [null], [{}], [{highWaterMark: undefined}]]) { + let error; + try { new C(...args); } catch (caught) {error = caught;} + assert(error instanceof realm.TypeError, 'required highWaterMark in callee realm'); + } + for (const highWaterMark of [NaN, Infinity, -1, 0]) { + assert(Object.is(new C({highWaterMark}).highWaterMark, highWaterMark), 'unrestricted highWaterMark'); + } + let reads = 0; + const strategy = new C({ + get highWaterMark() {reads++; return {valueOf() {reads++; return 2;}};}, + get size() {throw 'size must not be read';} + }); + assert(strategy.highWaterMark === 2 && reads === 2, 'one member read and one conversion'); + let error; + try {new C({highWaterMark: {valueOf() {throw marker;}}});} catch (caught) {error = caught;} + assert(error === marker, 'numeric exception identity'); + } + const reads = []; + let error; + try { + new realm.TransformStream({ + get start() {reads.push('transformer');} + }, { + get highWaterMark() {reads.push('writable.highWaterMark'); return -1;}, + get size() {reads.push('writable.size');} + }, { + get highWaterMark() {reads.push('readable.highWaterMark'); return 0;} + }); + } catch (caught) {error = caught;} + assert(error instanceof realm.RangeError, 'strategy validation realm'); + assert(reads.join() === 'writable.highWaterMark,writable.size', 'stop before readable strategy and transformer: ' + reads); + } + return 'ok'; + })() + "#).unwrap(); + assert_eq!(result, "ok"); +} + mod compression; mod intrinsic_promises; mod transform_finish;