From ad7ad10fca31278519398fb27cd3107cd2ecacf5 Mon Sep 17 00:00:00 2001 From: Shannon Booth Date: Sun, 22 Dec 2024 18:06:35 +1300 Subject: [PATCH 1/4] LibWeb/Streams: Fix spec link for RBS controller release steps --- Libraries/LibWeb/Streams/ReadableByteStreamController.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Libraries/LibWeb/Streams/ReadableByteStreamController.cpp b/Libraries/LibWeb/Streams/ReadableByteStreamController.cpp index 886bd529137e..7506430b4843 100644 --- a/Libraries/LibWeb/Streams/ReadableByteStreamController.cpp +++ b/Libraries/LibWeb/Streams/ReadableByteStreamController.cpp @@ -172,7 +172,7 @@ void ReadableByteStreamController::pull_steps(GC::Ref read_request) readable_byte_stream_controller_call_pull_if_needed(*this); } -// https://streams.spec.whatwg.org/#rbs-controller-private-pull +// https://streams.spec.whatwg.org/#abstract-opdef-readablebytestreamcontroller-releasesteps void ReadableByteStreamController::release_steps() { // 1. If this.[[pendingPullIntos]] is not empty, From 5af835edaee4beb61437aa349945f6cf6fbd4052 Mon Sep 17 00:00:00 2001 From: Shannon Booth Date: Mon, 23 Dec 2024 16:19:17 +1300 Subject: [PATCH 2/4] LibWeb/WebIDL: Implement 'write' operation for ArrayBufferView --- Libraries/LibWeb/WebIDL/Buffers.cpp | 20 ++++++++++++++++++++ Libraries/LibWeb/WebIDL/Buffers.h | 1 + 2 files changed, 21 insertions(+) diff --git a/Libraries/LibWeb/WebIDL/Buffers.cpp b/Libraries/LibWeb/WebIDL/Buffers.cpp index 8529496a0a08..94477be5638e 100644 --- a/Libraries/LibWeb/WebIDL/Buffers.cpp +++ b/Libraries/LibWeb/WebIDL/Buffers.cpp @@ -90,6 +90,26 @@ u32 ArrayBufferView::byte_offset() const [](auto& view) -> u32 { return static_cast(view->byte_offset()); }); } +// https://webidl.spec.whatwg.org/#arraybufferview-write +void ArrayBufferView::write(ReadonlyBytes bytes, u32 starting_offset) +{ + // 1. Let jsView be the result of converting view to a JavaScript value. + // 2. Assert: bytes’s length ≤ jsView.[[ByteLength]] − startingOffset. + VERIFY(bytes.size() <= byte_length() - starting_offset); + + // 3. Assert: if view is not a DataView, then bytes’s length modulo the element size of view’s type is 0. + if (!m_bufferable_object.has>()) { + auto element_size = m_bufferable_object.get>()->element_size(); + VERIFY(bytes.size() % element_size == 0); + } + + // 4. Let arrayBuffer be the result of converting jsView.[[ViewedArrayBuffer]] to an IDL value of type ArrayBuffer. + auto array_buffer = viewed_array_buffer(); + + // 5. Write bytes into arrayBuffer with startingOffset set to jsView.[[ByteOffset]] + startingOffset. + array_buffer->buffer().overwrite(byte_offset() + starting_offset, bytes.data(), bytes.size()); +} + BufferSource::~BufferSource() = default; } diff --git a/Libraries/LibWeb/WebIDL/Buffers.h b/Libraries/LibWeb/WebIDL/Buffers.h index 2381e286205a..10df7ea44f19 100644 --- a/Libraries/LibWeb/WebIDL/Buffers.h +++ b/Libraries/LibWeb/WebIDL/Buffers.h @@ -69,6 +69,7 @@ class ArrayBufferView : public BufferableObjectBase { using BufferableObjectBase::is_typed_array_base; u32 byte_offset() const; + void write(ReadonlyBytes, u32 starting_offset = 0); }; // https://webidl.spec.whatwg.org/#BufferSource From 2c5af124f8db8a3c4e76b3fb42284a8b6d5fdf82 Mon Sep 17 00:00:00 2001 From: Shannon Booth Date: Mon, 23 Dec 2024 16:20:02 +1300 Subject: [PATCH 3/4] LibWeb/Streams: Fully implement BYOB operations for ReadableStream As far as I can tell there is no change in WPT from this implementation. But these steps should be more effecient than using the non BYOB steps as we can make direct use of the provided buffer to the byte stream. --- Libraries/LibWeb/Streams/ReadableStream.cpp | 69 ++++++++++++++++----- Libraries/LibWeb/Streams/ReadableStream.h | 2 + 2 files changed, 54 insertions(+), 17 deletions(-) diff --git a/Libraries/LibWeb/Streams/ReadableStream.cpp b/Libraries/LibWeb/Streams/ReadableStream.cpp index a0283b88579d..742277848c69 100644 --- a/Libraries/LibWeb/Streams/ReadableStream.cpp +++ b/Libraries/LibWeb/Streams/ReadableStream.cpp @@ -15,9 +15,12 @@ #include #include #include +#include #include #include #include +#include +#include #include namespace Web::Streams { @@ -280,8 +283,10 @@ WebIDL::ExceptionOr ReadableStream::pull_from_bytes(ByteBuffer bytes) // 3. Let desiredSize be available. auto desired_size = available; - // FIXME: 4. If stream’s current BYOB request view is non-null, then set desiredSize to stream’s current BYOB request - // view's byte length. + // 4. If stream’s current BYOB request view is non-null, then set desiredSize to stream’s current BYOB request + // view's byte length. + if (auto byob_view = current_byob_request_view()) + desired_size = byob_view->byte_length(); // 5. Let pullSize be the smaller value of available and desiredSize. auto pull_size = min(available, desired_size); @@ -293,11 +298,16 @@ WebIDL::ExceptionOr ReadableStream::pull_from_bytes(ByteBuffer bytes) if (pull_size != available) bytes = MUST(bytes.slice(pull_size, available - pull_size)); - // FIXME: 8. If stream’s current BYOB request view is non-null, then: - // 1. Write pulled into stream’s current BYOB request view. - // 2. Perform ? ReadableByteStreamControllerRespond(stream.[[controller]], pullSize). + // 8. If stream’s current BYOB request view is non-null, then: + if (auto byob_view = current_byob_request_view()) { + // 1. Write pulled into stream’s current BYOB request view. + byob_view->write(pulled); + + // 2. Perform ? ReadableByteStreamControllerRespond(stream.[[controller]], pullSize). + TRY(readable_byte_stream_controller_respond(controller, pull_size)); + } // 9. Otherwise, - { + else { // 1. Set view to the result of creating a Uint8Array from pulled in stream’s relevant Realm. auto array_buffer = JS::ArrayBuffer::create(realm, move(pulled)); auto view = JS::Uint8Array::create(realm, array_buffer->byte_length(), *array_buffer); @@ -309,6 +319,23 @@ WebIDL::ExceptionOr ReadableStream::pull_from_bytes(ByteBuffer bytes) return {}; } +// https://streams.spec.whatwg.org/#readablestream-current-byob-request-view +GC::Ptr ReadableStream::current_byob_request_view() +{ + // 1. Assert: stream.[[controller]] implements ReadableByteStreamController. + VERIFY(m_controller->has>()); + + // 2. Let byobRequest be ! ReadableByteStreamControllerGetBYOBRequest(stream.[[controller]]). + auto byob_request = readable_byte_stream_controller_get_byob_request(m_controller->get>()); + + // 3. If byobRequest is null, then return null. + if (!byob_request) + return {}; + + // 4. Return byobRequest.[[view]]. + return byob_request->view(); +} + // https://streams.spec.whatwg.org/#readablestream-enqueue WebIDL::ExceptionOr ReadableStream::enqueue(JS::Value chunk) { @@ -317,7 +344,7 @@ WebIDL::ExceptionOr ReadableStream::enqueue(JS::Value chunk) // 1. If stream.[[controller]] implements ReadableStreamDefaultController, if (m_controller->has>()) { // 1. Perform ! ReadableStreamDefaultControllerEnqueue(stream.[[controller]], chunk). - return readable_stream_default_controller_enqueue(m_controller->get>(), chunk); + MUST(readable_stream_default_controller_enqueue(m_controller->get>(), chunk)); } // 2. Otherwise, else { @@ -325,23 +352,31 @@ WebIDL::ExceptionOr ReadableStream::enqueue(JS::Value chunk) VERIFY(m_controller->has>()); auto readable_byte_controller = m_controller->get>(); - // FIXME: 2. Assert: chunk is an ArrayBufferView. + // 2. Assert: chunk is an ArrayBufferView. + VERIFY(chunk.is_object()); + auto chunk_view = heap().allocate(chunk.as_object()); // 3. Let byobView be the current BYOB request view for stream. - // FIXME: This is not what the spec means by 'current BYOB request view' - auto byob_view = readable_byte_controller->raw_byob_request(); + auto byob_view = current_byob_request_view(); // 4. If byobView is non-null, and chunk.[[ViewedArrayBuffer]] is byobView.[[ViewedArrayBuffer]], then: - if (byob_view) { - // FIXME: 1. Assert: chunk.[[ByteOffset]] is byobView.[[ByteOffset]]. - // FIXME: 2. Assert: chunk.[[ByteLength]] ≤ byobView.[[ByteLength]]. - // FIXME: 3. Perform ? ReadableByteStreamControllerRespond(stream.[[controller]], chunk.[[ByteLength]]). - TODO(); - } + if (byob_view && chunk_view->viewed_array_buffer() == byob_view->viewed_array_buffer()) { + // 1. Assert: chunk.[[ByteOffset]] is byobView.[[ByteOffset]]. + VERIFY(chunk_view->byte_offset() == byob_view->byte_offset()); + + // 2. Assert: chunk.[[ByteLength]] ≤ byobView.[[ByteLength]]. + VERIFY(chunk_view->byte_length() <= byob_view->byte_length()); + // 3. Perform ? ReadableByteStreamControllerRespond(stream.[[controller]], chunk.[[ByteLength]]). + TRY(readable_byte_stream_controller_respond(readable_byte_controller, chunk_view->byte_length())); + } // 5. Otherwise, perform ? ReadableByteStreamControllerEnqueue(stream.[[controller]], chunk). - return readable_byte_stream_controller_enqueue(readable_byte_controller, chunk); + else { + TRY(readable_byte_stream_controller_enqueue(readable_byte_controller, chunk)); + } } + + return {}; } // https://streams.spec.whatwg.org/#readablestream-set-up-with-byte-reading-support diff --git a/Libraries/LibWeb/Streams/ReadableStream.h b/Libraries/LibWeb/Streams/ReadableStream.h index 3959c82bbe5b..5cfc1e66dced 100644 --- a/Libraries/LibWeb/Streams/ReadableStream.h +++ b/Libraries/LibWeb/Streams/ReadableStream.h @@ -111,6 +111,8 @@ class ReadableStream final : public Bindings::PlatformObject { void set_up_with_byte_reading_support(GC::Ptr = {}, GC::Ptr = {}, double high_water_mark = 0); GC::Ref piped_through(GC::Ref, bool prevent_close = false, bool prevent_abort = false, bool prevent_cancel = false, JS::Value signal = JS::js_undefined()); + GC::Ptr current_byob_request_view(); + private: explicit ReadableStream(JS::Realm&); From ec4434920b0b5537b15cd3c494a261aae560221f Mon Sep 17 00:00:00 2001 From: Shannon Booth Date: Thu, 26 Dec 2024 12:02:07 +1300 Subject: [PATCH 4/4] LibWeb/Streams: Actually implement the piped through steps This mistakenly implemented the 'piped to' operation on ReadableStream. No functional difference as the caller was doing the extra work already of 'piped through' vs 'piped to'. --- Libraries/LibWeb/Fetch/Fetching/Fetching.cpp | 4 +--- Libraries/LibWeb/Streams/ReadableStream.cpp | 17 ++++++++++++----- Libraries/LibWeb/Streams/ReadableStream.h | 2 +- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/Libraries/LibWeb/Fetch/Fetching/Fetching.cpp b/Libraries/LibWeb/Fetch/Fetching/Fetching.cpp index 40fac6b051ee..2a3bb9cc24f9 100644 --- a/Libraries/LibWeb/Fetch/Fetching/Fetching.cpp +++ b/Libraries/LibWeb/Fetch/Fetching/Fetching.cpp @@ -744,9 +744,7 @@ void fetch_response_handover(JS::Realm& realm, Infrastructure::FetchParams const transform_stream->set_up(identity_transform_algorithm, flush_algorithm); // 4. Set internalResponse’s body’s stream to the result of internalResponse’s body’s stream piped through transformStream. - auto promise = internal_response->body()->stream()->piped_through(transform_stream->writable()); - WebIDL::mark_promise_as_handled(*promise); - internal_response->body()->set_stream(transform_stream->readable()); + internal_response->body()->set_stream(internal_response->body()->stream()->piped_through(transform_stream)); } // 8. If fetchParams’s process response consume body is non-null, then: diff --git a/Libraries/LibWeb/Streams/ReadableStream.cpp b/Libraries/LibWeb/Streams/ReadableStream.cpp index 742277848c69..91e1373e0844 100644 --- a/Libraries/LibWeb/Streams/ReadableStream.cpp +++ b/Libraries/LibWeb/Streams/ReadableStream.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -426,19 +427,25 @@ void ReadableStream::set_up_with_byte_reading_support(GC::Ptr pul } // https://streams.spec.whatwg.org/#readablestream-pipe-through -GC::Ref ReadableStream::piped_through(GC::Ref writable, bool prevent_close, bool prevent_abort, bool prevent_cancel, JS::Value signal) +GC::Ref ReadableStream::piped_through(GC::Ref transform, bool prevent_close, bool prevent_abort, bool prevent_cancel, JS::Value signal) { // 1. Assert: ! IsReadableStreamLocked(readable) is false. VERIFY(!is_readable_stream_locked(*this)); - // 2. Assert: ! IsWritableStreamLocked(writable) is false. - VERIFY(!is_writable_stream_locked(writable)); + // 2. Assert: ! IsWritableStreamLocked(transform.[[writable]]) is false. + VERIFY(!is_writable_stream_locked(transform->writable())); // 3. Let signalArg be signal if signal was given, or undefined otherwise. // NOTE: Done by default arguments. - // 4. Return ! ReadableStreamPipeTo(readable, writable, preventClose, preventAbort, preventCancel, signalArg). - return readable_stream_pipe_to(*this, writable, prevent_close, prevent_abort, prevent_cancel, signal); + // 4. Let promise be ! ReadableStreamPipeTo(readable, transform.[[writable]], preventClose, preventAbort, preventCancel, signalArg). + auto promise = readable_stream_pipe_to(*this, transform->writable(), prevent_close, prevent_abort, prevent_cancel, signal); + + // 5. Set promise.[[PromiseIsHandled]] to true. + WebIDL::mark_promise_as_handled(*promise); + + // 6. Return transform.[[readable]]. + return transform->readable(); } } diff --git a/Libraries/LibWeb/Streams/ReadableStream.h b/Libraries/LibWeb/Streams/ReadableStream.h index 5cfc1e66dced..af7e9f4fd8ca 100644 --- a/Libraries/LibWeb/Streams/ReadableStream.h +++ b/Libraries/LibWeb/Streams/ReadableStream.h @@ -109,7 +109,7 @@ class ReadableStream final : public Bindings::PlatformObject { WebIDL::ExceptionOr pull_from_bytes(ByteBuffer); WebIDL::ExceptionOr enqueue(JS::Value chunk); void set_up_with_byte_reading_support(GC::Ptr = {}, GC::Ptr = {}, double high_water_mark = 0); - GC::Ref piped_through(GC::Ref, bool prevent_close = false, bool prevent_abort = false, bool prevent_cancel = false, JS::Value signal = JS::js_undefined()); + GC::Ref piped_through(GC::Ref, bool prevent_close = false, bool prevent_abort = false, bool prevent_cancel = false, JS::Value signal = JS::js_undefined()); GC::Ptr current_byob_request_view();