servo
servo copied to clipboard
Rewrite readable stream implementation without spidermonkey builtin object
The next spidermonkey upgrade will bring in https://bugzilla.mozilla.org/show_bug.cgi?id=1795914, which removes the JS implementation. We will need to port https://bugzilla.mozilla.org/show_bug.cgi?id=1730589 and subsequent changes from https://searchfox.org/mozilla-central/source/dom/streams/ReadableStream.cpp before we can upgrade again.
Relevant base code in servo is https://github.com/servo/servo/blob/master/components/script/dom/readablestream.rs
Depends on https://github.com/servo/servo/pull/29079
I'm working on this. It looks like it might be easier than I anticipated, since we aren't actually implementing the full WHATWG Streams API defined in https://streams.spec.whatwg.org/.
I've pushed my (very rough) in-progress branch to https://github.com/servo/servo/compare/master...jdm:servo:no-streams?expand=1 . I was using tests/wpt/web-platform-tests/xhr/overridemimetype-blob.html as the initial testcase; right now this subtest times out because the promises aren't queued correctly.
Note to self: consume_stream in script_runtime.rs will also need to be removed/replaced.
The latest commit allows the majority of the tests in XHR to pass. The idlharness tests crash in the JS engine because the memory safety story for readable streams is much less clear than it used to be. My goal once that crash is fixed is to clean up the code and then document the state of the world so it becomes easier to reason about how to make it less complex. It's... difficult to follow at the moment.
I also started working on prototyping streams, but I left it due to time constrains at that time and I do not plan to comeback working on it anytime soon. It does not compile, but some initial work has been laid down (mostly webidls, which for some reason need to be splitted over multiple files or else codegen does not work).
We will need to decide if we turn off readable streams in mozjs as part of this work. That would definitely make the test results more honest.
There is --enable-js-streams.
Yep. I think I'd like to get the tests all passing with js streams enabled, then remove that flag and see what the impact is on the test coverage.
We decided to not block SM upgrade on this and port JS streams to SM ESR 115 in https://github.com/servo/mozjs/pull/408. We should still do this sometimes in the future, preferably before next ESR.
The title of this issue is more straight forward so I'll leave the progress here. So we decided to create another branch, readablestream-re-implementation, to work on this.
Here's a rough overview of the progress. I'll add more details as it proceeds:
- [x]
ReadableStreamWebIDL codegen: #32605- [ ]
Constructor: https://github.com/servo/servo/pull/32634 - [ ]
ReadableStreamMethodsimplementation- [ ]
Locked - [ ]
Cancel - [ ]
GetReader
- [ ]
- [ ]
- [x]
ReadableStreamDefaultControllerWebIDL codegen- [ ]
SetUpReadableStreamDefaultControllerFromUnderlyingSource - [ ]
ReadableStreamDefaultControllerMethodsimplementation: todo- [ ]
GetDesiredSize - [ ]
Close - [ ]
Enqueue - [ ]
Error
- [ ]
- [ ]
- [x]
ReadableBytesStreamControllerWebIDL codegen- [ ]
SetUpReadableByteStreamControllerFromUnderlyingSource - [ ]
ReadableBytesStreamControllerMethodsimplementation: todo- [ ]
GetByobRequest - [ ]
GetDesiredSize - [ ]
Close - [ ]
Enqueue - [ ]
Error
- [ ]
- [ ]
- [x]
ReadableStreamDefaultReaderWebIDL codegen: #32605 - [x]
ReadableStreamBYOBReaderWebIDL codegen: #32605 - [x]
QueuingStrategy: #32572 - [x]
UnderlyingSource: #32572
For anyone who is interested in contributing, here's what you could do:
- For "todo" item, please open PR against my fork, or wait for that interface is available in servo's branch.
- For "draft complete" or items that has a opened PR, please help me review and see if the implementation is correct.
I don't think all the "todo" items can be done separately, I would say let's complete the WebIDL skeleton first, then we will have to look at everything again and come-up with a plan for the implementation.
@gterzian I think you are right. I just opened https://github.com/servo/servo/pull/32602 but I think it will be super difficult to review.
The only missing WebIDL is ReadableStream left. I guess it's better to close that draft to fill the skeleton first.