Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LibWeb: Add and use an ad-hoc ReadableStreamDefaultReader::read_all_chunks AO #24179

Merged
merged 4 commits into from May 1, 2024

Conversation

trflynn89
Copy link
Member

Ref #24132 (comment)

These patches were done as part of making fetch use streams to read response bodies. That ultimately has some issues to be worked out, but in the meantime, this affords some cleanup that aligns us back closer to the spec.

The ReadableStreamPipeTo AO requires reading all chunks from a stream.
There actually isn't an AO defined to do that, so the "read all bytes"
implementation was changed to provide each chunk in a vector in commit
12cfa08.

This change makes reading all bytes a bit more uncomfortable in normal
use cases, as we now have to manually join the vector we receive. This
can also cause churn with huge allocations.

So instead, let's just provide an ad-hoc callback to receive each chunk
as they arrive.
The callback is already specified to take the bytes by value. No need to
copy the bytes here.
@github-actions github-actions bot added the 👀 pr-needs-review PR needs review from a maintainer or community member label May 1, 2024
@awesomekling awesomekling merged commit 46b8a3a into SerenityOS:master May 1, 2024
12 checks passed
@github-actions github-actions bot removed the 👀 pr-needs-review PR needs review from a maintainer or community member label May 1, 2024
@trflynn89 trflynn89 deleted the stream_read_chunks branch May 1, 2024 22:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants