Add io module for streams with handling streams in the associated document's realm#1135
Add io module for streams with handling streams in the associated document's realm#1135lutien wants to merge 4 commits into
Conversation
This adds the initial primitives for working with readable streams, although they are not yet connected up to any other parts of the specification.
|
|
||
| <dfn>Stream settings</dfn> is a [=struct=] with | ||
| an [=struct/item=] named <dfn for="stream settings">stream</dfn>, which is a [=readable stream=], | ||
| an [=struct/item=] named <dfn for="stream settings">navigable</dfn>, which is a <code>browsingContext.BrowsingContext</code> or null. |
There was a problem hiding this comment.
Should this be a navigable or directly be a realm (e.g. so that we support streams coming from workers)?
There was a problem hiding this comment.
Yeah, I was thinking about it too. Ok, I've updated the PR to use a realm.
| A <dfn lt="stream|IO stream" export>stream</dfn> is an abstract source of | ||
| data that can be read from or written to. | ||
|
|
||
| A [=BiDi Session=] has a <dfn>readable stream settings map</dfn> which is a [=/map=] between |
There was a problem hiding this comment.
As written this would seem to possibly keep navigables (or realms) alive as long as there's a reference to one of their streams. That feels likely to cause memory leaks.
There was a problem hiding this comment.
Ok, I've added the cleanup steps when the realms are gone.
|
|
||
| 1. If |promise| was rejected, then: | ||
|
|
||
| 1. [=Clean up after running script=] with |environment settings|. |
There was a problem hiding this comment.
I think this needs to be outside the reaction; if the promise isn't already fulfilled the steps end up being called in a future microtask checkpoint when we're no longer top of the execution stack.
|
|
||
| 1. [=Prepare to run script=] with |environment settings|. | ||
|
|
||
| 1. Let |reader| be the result of [=ReadableStream/getting a reader=] for |stream|. |
There was a problem hiding this comment.
This can fail, we need to handle the failure.
There was a problem hiding this comment.
I've added trying, is it enough, or do you want to send some specific error if it fails?
|
|
||
| 1. Let |reader| be the result of [=ReadableStream/getting a reader=] for |stream|. | ||
|
|
||
| 1. Let |read result| be the result of calling {{ReadableStreamDefaultReader/read()}} for |reader|. |
There was a problem hiding this comment.
I think you want to use https://streams.spec.whatwg.org/#readablestreamdefaultreader-read-a-chunk instead, which then affects the logic for the rest of this section.
There was a problem hiding this comment.
Ah, alright, I've updated it.
| property set to |handle id| if it's not null, or omitted otherwise, and | ||
| the <code>value</code> property set to |serialized|. | ||
|
|
||
| <dt>|value| is a [=platform object=] that implements {{ReadableStream}} |
There was a problem hiding this comment.
I'm not sure if we also want to support the case where you try to return a reader to the stream rather than the stream itself.
There was a problem hiding this comment.
Ok, yeah, I'm not sure either. I guess that's a good question for the spec meeting. Let's keep it open for now.
bed18e3 to
743f6dc
Compare
|
Hey, @MattiasBuelens, I've picked up the work on the streaming API from James. So when you have time, could you also review from your side? |
2 participants
An alternative approach to #1061 to handle streams in the associated document's realm + adding the serialization of ReadableStreams as RemoteValue to demonstrate how the whole flow would look.
Preview | Diff