Byte stream update, including reference implementation #361
Conversation
This file contains 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
…amController.enqueue()
…g storedError initialization
…to CancelReadableByteStreamController()
|
Made 8 more commits (both bug and readability fixes) |
…ing will be done on reader
… between ErrorReadableByteStream and CloseReadableByteStream
…reamReaderGeneric To make it consistent with ReadableStream
…erGeneric call order
|
Made some more commits mainly in order to make it parallel with ReadableStream. |
|
@tyoshino still would like to merge this if possible given how it is a big change/improvement from the current situation (in test/implementation coverage alone, if nothing else). To do that I guess we need to either update the spec to parallel the reference implementation, or remove most of the spec and say "please see reference implementation; still under development." |
|
@domenic okay. maybe latter and also start writing a patch to add the spec text. |
The spec texts will be updated by a separate patch.
|
Reduced the change on the spec side. Ready for review. |
…defined this method is gone
|
Looks good. Needs a rebase though. |
|
Merged as 482d14b. |
domenic
pushed a commit
that referenced
this pull request
Mar 25, 2016
## Background We originally designed ReadableByteStream, which included extended features to handle bytes, as a separate class from the ReadableStream, which handles a stream of general objects. While designing the details of ReadableByteStream (see #361), we noticed that we could simplify ReadableStream and ReadableByteStream by moving variables and logic for handling queuing into their controller class (see #379). This turned out to also clarify which part of the logic represents semantic requirements of readable streams, and which part of it is implementing helper logic for easier development of underlying sources. After the above refactoring, we also noticed that ReadableStream and ReadableByteStream share most of their code. So, we merged the ReadableByteStream class into ReadableStream. This has many benefits for developers who don't have to deal with two similar-but-different classes. Instead, the same class is used, with the behavior customized by the underlying source. ## Change summary The new ReadableStream class has two reader acquisition methods, getReader() and getBYOBReader(), with the latter working when BYOB reading is available. Availability of BYOB reading is determined by whether or not the underlying source passed to the ReadableStream had BYOB pulling functionality. This is indicated by a truthy `byob` property of the underlying source. The two readers are named the default reader and BYOB reader. The default reader's read() method takes no argument. The resulting promise of the read() method will be fulfilled with a newly-allocated chunk. The BYOB reader's read() method takes one argument; it must be an ArrayBuffer view. The ReadableStream will fill the passed ArrayBuffer view, and fulfill the returned promise with it. The ArrayBuffer view might be transferred several times, but the same backing memory is always written to. When the byob option is falsy, the underlying source is given a ReadableStreamDefaultController. This class provides methods to enqueue a new chunk and know the status of the queue. Its queuing strategy is configured by the parameters passed to the ReadableStream's constructor. The underlying source can subscribe to know when chunks are drained from the queue by implementing the "pull" method. Only the getReader() method will be functional on the ReadableStream. When the byob option is truthy, the underlying source is given a ReadableStreamBYOBController. In addition to the functionalities of the ReadableStreamDefaultController, this controller provides a getter named `byobRequest` which exposes the oldest outstanding BYOB reading request into which the underlying source can put bytes directly (see #423). Both the getReader() and the getBYOBReader() method will be functional on the ReadableStream. The ReadableStreamBYOBController can be configured to convert read requests from a default reader into BYOB read requests, by automatically allocating a buffer and exposing it via the byobRequest getter. This eases implementation of a reactive underlying source, as shown in one of the new examples. ## Changes included in this commit In addition to the major changes as described above, this commit includes bunch of design /logic/aesthetic changes as follows: ### Changes to existing observable features Although ReadableStream's internals were refactored immensely, its external behavior (when not providing a BYOB source) is almost identical to before, as verified by our extensive unit tests. However, we did make a few changes which are observable: - Changes to the semantics of the controller methods (see #424): - Make controller.close() and controller.enqueue() fail when the stream is not in the readable state - Make controller.enqueue() throw a predefined TypeError, not the stored error - (As a result of these changes, the tests test/pipe-to-options.js, test/readable-streams/general.js, and test/readable-stream-templated.js have been updated.) - Rename ReadableStreamController to ReadableStreamDefaultController - Rename ReadableStreamReader to ReadableStreamDefaultReader ### Changes to byte streams As explained above, byte streams were changed in fairly extensive ways to merge them into the base ReadableStream class. Here we call out a few notable changes from the previous specification text: - Remove auto release feature from the ReadableByteStream - Rename Byob to BYOB - Make the default highWaterMark of the byte source version to 0 - Port the functionality that the start method can delay pulling by returning a pending promise to the ReadableStreamBYOBController - Port the highWaterMark mechanism to ReadableByteStreamController - Rename ReadableByteStreamController to ReadableStreamBYOBController - Correctly update the [[disturbed]] slot in the byte handling logic - read(view) now checks view.byteLength before setting [[disturbed]]
tyoshino
added a commit
that referenced
this pull request
Mar 28, 2016
## Background We originally designed ReadableByteStream, which included extended features to handle bytes, as a separate class from the ReadableStream, which handles a stream of general objects. While designing the details of ReadableByteStream (see #361), we noticed that we could simplify ReadableStream and ReadableByteStream by moving variables and logic for handling queuing into their controller class (see #379). This turned out to also clarify which part of the logic represents semantic requirements of readable streams, and which part of it is implementing helper logic for easier development of underlying sources. After the above refactoring, we also noticed that ReadableStream and ReadableByteStream share most of their code. So, we merged the ReadableByteStream class into ReadableStream. This has many benefits for developers who don't have to deal with two similar-but-different classes. Instead, the same class is used, with the behavior customized by the underlying source. ## Change summary The new ReadableStream class has two reader acquisition methods, getReader() and getBYOBReader(), with the latter working when BYOB reading is available. Availability of BYOB reading is determined by whether or not the underlying source passed to the ReadableStream had BYOB pulling functionality. This is indicated by a truthy `byob` property of the underlying source. The two readers are named the default reader and BYOB reader. The default reader's read() method takes no argument. The resulting promise of the read() method will be fulfilled with a newly-allocated chunk. The BYOB reader's read() method takes one argument; it must be an ArrayBuffer view. The ReadableStream will fill the passed ArrayBuffer view, and fulfill the returned promise with it. The ArrayBuffer view might be transferred several times, but the same backing memory is always written to. When the byob option is falsy, the underlying source is given a ReadableStreamDefaultController. This class provides methods to enqueue a new chunk and know the status of the queue. Its queuing strategy is configured by the parameters passed to the ReadableStream's constructor. The underlying source can subscribe to know when chunks are drained from the queue by implementing the "pull" method. Only the getReader() method will be functional on the ReadableStream. When the byob option is truthy, the underlying source is given a ReadableStreamBYOBController. In addition to the functionalities of the ReadableStreamDefaultController, this controller provides a getter named `byobRequest` which exposes the oldest outstanding BYOB reading request into which the underlying source can put bytes directly (see #423). Both the getReader() and the getBYOBReader() method will be functional on the ReadableStream. The ReadableStreamBYOBController can be configured to convert read requests from a default reader into BYOB read requests, by automatically allocating a buffer and exposing it via the byobRequest getter. This eases implementation of a reactive underlying source, as shown in one of the new examples. ## Changes included in this commit In addition to the major changes as described above, this commit includes bunch of design /logic/aesthetic changes as follows: ### Changes to existing observable features Although ReadableStream's internals were refactored immensely, its external behavior (when not providing a BYOB source) is almost identical to before, as verified by our extensive unit tests. However, we did make a few changes which are observable: - Changes to the semantics of the controller methods (see #424): - Make controller.close() and controller.enqueue() fail when the stream is not in the readable state - Make controller.enqueue() throw a predefined TypeError, not the stored error - (As a result of these changes, the tests test/pipe-to-options.js, test/readable-streams/general.js, and test/readable-stream-templated.js have been updated.) - Rename ReadableStreamController to ReadableStreamDefaultController - Rename ReadableStreamReader to ReadableStreamDefaultReader ### Changes to byte streams As explained above, byte streams were changed in fairly extensive ways to merge them into the base ReadableStream class. Here we call out a few notable changes from the previous specification text: - Remove auto release feature from the ReadableByteStream - Rename Byob to BYOB - Make the default highWaterMark of the byte source version to 0 - Port the functionality that the start method can delay pulling by returning a pending promise to the ReadableStreamBYOBController - Port the highWaterMark mechanism to ReadableByteStreamController - Rename ReadableByteStreamController to ReadableStreamBYOBController - Correctly update the [[disturbed]] slot in the byte handling logic - read(view) now checks view.byteLength before setting [[disturbed]]
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Doing a PR of @tyoshino's changes so I can do line comments.