Skip to content

Commit

Permalink
Merge ReadableByteStream into ReadableStream!
Browse files Browse the repository at this point in the history
## 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]]
  • Loading branch information
tyoshino committed Mar 28, 2016
1 parent 27c77cd commit e601d69
Show file tree
Hide file tree
Showing 11 changed files with 4,455 additions and 2,312 deletions.
3,314 changes: 2,037 additions & 1,277 deletions index.bs

Large diffs are not rendered by default.

38 changes: 34 additions & 4 deletions reference-implementation/lib/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ export function createArrayFromList(elements) {
return elements.slice();
}

export function ArrayBufferCopy(dest, destOffset, src, srcOffset, n) {
new Uint8Array(dest).set(new Uint8Array(src, srcOffset, n), destOffset);
}

export function CreateIterResultObject(value, done) {
assert(typeof done === 'boolean');
const obj = {};
Expand All @@ -44,6 +48,20 @@ export function CreateIterResultObject(value, done) {
return obj;
}

export function IsFiniteNonNegativeNumber(v) {
if (Number.isNaN(v)) {
return false;
}
if (v === +Infinity) {
return false;
}
if (v < 0) {
return false;
}

return true;
}

export function InvokeOrNoop(O, P, args) {
const method = O[P];
if (method === undefined) {
Expand Down Expand Up @@ -90,11 +108,13 @@ export function PromiseInvokeOrFallbackOrNoop(O, P1, args1, P2, args2) {
}
}

export function ValidateAndNormalizeQueuingStrategy(size, highWaterMark) {
if (size !== undefined && typeof size !== 'function') {
throw new TypeError('size property of a queuing strategy must be a function');
}
export function TransferArrayBuffer(buffer) {
// No-op. Just for marking places where detaching an ArrayBuffer is required.

return buffer;
}

export function ValidateAndNormalizeHighWaterMark(highWaterMark) {
highWaterMark = Number(highWaterMark);
if (Number.isNaN(highWaterMark)) {
throw new TypeError('highWaterMark property of a queuing strategy must be convertible to a non-NaN number');
Expand All @@ -103,5 +123,15 @@ export function ValidateAndNormalizeQueuingStrategy(size, highWaterMark) {
throw new RangeError('highWaterMark property of a queuing strategy must be nonnegative');
}

return highWaterMark;
}

export function ValidateAndNormalizeQueuingStrategy(size, highWaterMark) {
if (size !== undefined && typeof size !== 'function') {
throw new TypeError('size property of a queuing strategy must be a function');
}

highWaterMark = ValidateAndNormalizeHighWaterMark(highWaterMark);

return { size, highWaterMark };
}
3 changes: 2 additions & 1 deletion reference-implementation/lib/queue-with-sizes.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const assert = require('assert');
import { IsFiniteNonNegativeNumber } from './helpers';

export function DequeueValue(queue) {
assert(queue.length > 0, 'Spec-level failure: should never dequeue from an empty queue.');
Expand All @@ -8,7 +9,7 @@ export function DequeueValue(queue) {

export function EnqueueValueWithSize(queue, value, size) {
size = Number(size);
if (Number.isNaN(size) || size === +Infinity || size < 0) {
if (!IsFiniteNonNegativeNumber(size)) {
throw new RangeError('Size must be a finite, non-NaN, non-negative number.');
}

Expand Down
Loading

0 comments on commit e601d69

Please sign in to comment.