Re: [whatwg/streams] Allow other specifications to create readable byte streams (#1130)

@ricea commented on this pull request.



> -<p algorithm>To <dfn export for="ReadableStream">close</dfn> a {{ReadableStream}} |stream|, perform
-! [$ReadableStreamDefaultControllerClose$](|stream|.[=ReadableStream/[[controller]]=]).
+<p algorithm>A {{ReadableStream}} <dfn export for="ReadableStream" lt="need more data|needs
+more data">needs more data</dfn> if its [=ReadableStream/desired size to fill up to the high water
+mark=] is greater than zero.
+
+<div algorithm>
+ To <dfn export for="ReadableStream">close</dfn> a {{ReadableStream}} |stream|:
+
+ 1. If |stream|.[=ReadableStream/[[controller]]=] [=implements=] {{ReadableByteStreamController}},
+  1. Perform !
+     [$ReadableByteStreamControllerClose$](|stream|.[=ReadableStream/[[controller]]=]).
+  1. If |stream|.[=ReadableStream/[[controller]]=].[=ReadableByteStreamController/[[pendingPullIntos]]=]
+     is not [=list/is empty|empty=], perform !
+     [$ReadableByteStreamControllerRespond$](|stream|.[=ReadableStream/[[controller]]=], 0).
+     <p class="note">The caller has to make sure not to write into the

I feel some ambiguity here. How long before closing the stream is it okay to write into the view?

> +  1. Perform !
+     [$ReadableByteStreamControllerClose$](|stream|.[=ReadableStream/[[controller]]=]).
+  1. If |stream|.[=ReadableStream/[[controller]]=].[=ReadableByteStreamController/[[pendingPullIntos]]=]
+     is not [=list/is empty|empty=], perform !
+     [$ReadableByteStreamControllerRespond$](|stream|.[=ReadableStream/[[controller]]=], 0).
+     <p class="note">The caller has to make sure not to write into the
+     [=ReadableStream/current BYOB request view=] or transfer it to a different realm before
+     closing the stream.
+ 1. Otherwise, perform ! [$ReadableStreamDefaultControllerClose$](|stream|.[=ReadableStream/[[controller]]=]).
+</div>
+
+<div algorithm>
+ To <dfn export for="ReadableStream">error</dfn> a {{ReadableStream}} |stream| given a JavaScript
+ value |e|:
+
+ 1. If |stream|.[=ReadableStream/[[controller]]=] [=implements=] {{ReadableByteStreamController}},

All this switching on controller type is unfortunate. If we had more types we might add some polymorphism to avoid it.  But with only two types this is probably simpler.

> +<div algorithm>
+ To <dfn export for="ReadableStream">enqueue</dfn> the JavaScript value |chunk| into a
+ {{ReadableStream}} |stream|:
+
+ 1. If |stream|.[=ReadableStream/[[controller]]=] [=implements=]
+    {{ReadableStreamDefaultController}},
+  1. Perform ! [$ReadableStreamDefaultControllerEnqueue$](|stream|.[=ReadableStream/[[controller]]=],
+     |chunk|).
+ 1. Otherwise,
+  1. Assert: |stream|.[=ReadableStream/[[controller]]=] [=implements=]
+     {{ReadableByteStreamController}}.
+  1. Assert: |chunk| is an {{ArrayBufferView}}.
+  1. Let |byobRequest| be !
+     [$ReadableByteStreamControllerGetBYOBRequest$](|stream|.[=ReadableStream/[[controller]]=]).
+  1. If |byobRequest| is non-null, and |chunk|.\[[ViewedArrayBuffer]].\[[ArrayBufferData]] is the
+     same [=Data Block=] as |byobRequest|.\[[view]].\[[ViewedArrayBuffer]].\[[ArrayBufferData]],

There may be a problem here. If the calling specification needs to transfer `|byobRequest|.[[view]].[[ViewedArrayBuffer]]` for some reason, then it will be detached and not match.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/streams/pull/1130#pullrequestreview-706711704