script: Implement Compression API #39658
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
servo-highfive
added
the
S-awaiting-review
|
🔨 Triggering try run (#18242461313) for Linux (WPT) |
|
Test results for linux-wpt from try job (#18242461313): Flaky unexpected result (35)
Stable unexpected results that are known to be intermittent (25)
Stable unexpected results (2)
|
|
|
TimvdLippe
approved these changes
Oct 4, 2025
Contributor
TimvdLippe
left a comment
TimvdLippe
left a comment
There was a problem hiding this comment.
Did a quick review. Given that almost all WPT tests pass, I think it is pretty much complete. Only one remaining question regarding the bad chunks.
tests/wpt/meta/compression/compression-bad-chunks.tentative.any.js.ini
Outdated
Show resolved
Hide resolved
servo-highfive
added
S-awaiting-review
TimvdLippe
approved these changes
Oct 4, 2025
Contributor
TimvdLippe
left a comment
TimvdLippe
left a comment
There was a problem hiding this comment.
Last nit, but this still LGTM
servo-highfive
added
S-awaiting-review
Taym95
reviewed
Oct 5, 2025
| reflector_: Reflector, | ||
|
|
||
| /// <https://streams.spec.whatwg.org/#generictransformstream> | ||
| transform: Dom<TransformStream>, |
Member
There was a problem hiding this comment.
Happy to see the stream work give results and make it easier to implement other browser APIs.
Member
Author
There was a problem hiding this comment.
The stream work provides a great foundation for implementing this API. The TextEncoderStream and TextDecoderStream implementation also serve as good references for me. It turned out to be easier than I expected.
Taym95
reviewed
Oct 5, 2025
| // NOTE: In our implementation, the enum type of context already indicates the format. | ||
| let mut decompressor = ds.context.borrow_mut(); | ||
| let offset = decompressor.get_ref().len(); | ||
| let is_ended = decompressor |
Member
There was a problem hiding this comment.
you compute is_ended before calling try_finish, but you enqueue before checking !is_ended. That can emit bytes and then throw?!
Member
Author
There was a problem hiding this comment.
I also think it is weird, but the spec does emit bytes no matter whether the test passes or not.
https://compression.spec.whatwg.org/#decompress-flush-and-enqueue
p.s. I made a little change in the new commit. I flush and copy the buffer before doing the test. This guarantees the extra bytes won't go into the result. However, it does not resolve the weirdness here.
update: I reverted it, since this could turn a truncated stream (missing one zero byte at the end) into a valid stream.
| // Step 6.1 If this throws an exception e, | ||
| let realm = enter_realm(self); | ||
| let p = Promise::new_in_current_realm((&realm).into(), can_gc); | ||
| // return a promise rejected with e. |
Member
There was a problem hiding this comment.
can you use Promise::new_rejected here?
Member
Author
There was a problem hiding this comment.
Promise::new_rejected takes value: impl SafeToJSValConvertible, but e is a dom::bindings::error::Error, which currently does not implement this trait. If we use Promise::new_rejected, we still need extra few lines to do the conversion.
Member
Author
|
Rebased to resolve merge conflict. Also appended a fix for passing new WPT tests from upstream. |
TimTheBig
reviewed
Oct 24, 2025
Cargo.toml
Outdated
| encoding_rs = "0.8" | ||
| env_logger = "0.11" | ||
| euclid = "0.22" | ||
| flate2 = "1.1.2" |
There was a problem hiding this comment.
The specificity was lower in net
Suggested change
| flate2 = "1.1.2" | |
| flate2 = "1.1" |
Member
Author
There was a problem hiding this comment.
Thanks for pointing out. It's now pinned to 1.1.
Contributor
|
This PR still LGTM. @Taym95 do you have any further review feedback or does somebody else wants to take a look? If not, then I would opt for merging this PR. |
Member
|
Let's press the button! |
servo-highfive
added
the
S-awaiting-merge
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
servo-highfive
added
S-tests-failed
Contributor
|
Seems like the recent changes to |
Signed-off-by: Kingsley Yung <kingsley@kkoyung.dev>
Signed-off-by: Kingsley Yung <kingsley@kkoyung.dev>
https://compression.spec.whatwg.org/#decompressionstream Signed-off-by: Kingsley Yung <kingsley@kkoyung.dev>
Signed-off-by: Kingsley Yung <kingsley@kkoyung.dev>
This helps passing WPT tests from `compression/decompression-extra-input.any.js`. Signed-off-by: Kingsley Yung <kingsley@kkoyung.dev>
Signed-off-by: Kingsley Yung <kingsley@kkoyung.dev>
Signed-off-by: Kingsley Yung <kingsley@kkoyung.dev>
Signed-off-by: Kingsley Yung <kingsley@kkoyung.dev>
servo-highfive
removed
the
S-tests-failed
Member
Author
|
Rebased with 2 new commits to resolve the merge error |
servo-highfive
added
the
S-awaiting-merge
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
servo-highfive
added
S-tests-failed
Member
|
We will need to update the exposed interfaces test files to include the new DecompressionStream interface. Don't forget to use |
Signed-off-by: Kingsley Yung <kingsley@kkoyung.dev>
servo-highfive
removed
the
S-tests-failed
Signed-off-by: Kingsley Yung <kingsley@kkoyung.dev>
servo-highfive
added
the
S-awaiting-merge
servo-highfive
removed
the
S-awaiting-merge
Member
|
https://github.com/servo/servo/actions/runs/18833881485/job/53730445402 Re-queuing this, as all tests are failing. |
servo-highfive
added
the
S-awaiting-merge
servo-highfive
removed
the
S-awaiting-merge
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The patch implements Compression (https://compression.spec.whatwg.org/) with the compression and decompression provided by the
flate2crate (https://crates.io/crates/flate2).flate2supports several different backends, controlled through the crate's features. By default, it usesminiz_oxide(https://crates.io/crates/miniz_oxide).flate2provides three modulesread,writeandbufreadwhich work on instances of thestd::io::Read,std::io::Writeandstd::io::Bufreadtraits, respectively. Thewritemodule is chosen in the patch since it matches the streaming model in the specification.Testing: Enable WPT for Compression API, and introduce WPT expectation.