Re: [whatwg/streams] Reject pending reads when releasing reader (#1168)

@domenic commented on this pull request.

It'd be great if @nidhijaju could take a glance at this, from a perspective of "does this seem reasonable to implement in Chromium at some point" / "does the motivation and behavior make sense". For the latter https://github.com/web-platform-tests/wpt/pull/32072 might be helpful.

> @@ -2776,6 +2786,26 @@ The following abstract operations support the implementation and manipulation of
     |view|, |readIntoRequest|).
 </div>
 
+<div algorithm>
+ <dfn abstract-op lt="ReadableStreamBYOBReaderRelease"
+ id="readable-stream-byob-reader-release">ReadableStreamBYOBReaderRelease(|reader|)</dfn>

No hand-crafted id=""s for new abstract-ops

> + performs the following steps:
+
+ 1. Perform ! [$ReadableStreamReaderGenericRelease$](|reader|).
+ 1. Let |e| be a new {{TypeError}} exception.
+ 1. Perform ! [$ReadableStreamBYOBReaderErrorReadIntoRequests$](|reader|, |e|).
+</div>
+
+<div algorithm>
+ <dfn abstract-op lt="ReadableStreamBYOBReaderErrorReadIntoRequests">ReadableStreamBYOBReaderErrorReadIntoRequests(|reader|, |e|)</dfn>
+ performs the following steps:
+
+ 1. Let |readIntoRequests| be |reader|.[=ReadableStreamBYOBReader/[[readIntoRequests]]=].
+ 1. Set |reader|.[=ReadableStreamBYOBReader/[[readIntoRequests]]=] to a new empty [=list=].
+ 1. [=list/For each=] |readIntoRequest| of |readIntoRequests|,
+  1. Perform |readIntoRequest|'s [=read-into request/error steps=], given |e|.
+</div>

Alphabetize abstract ops within their section

> @@ -3156,7 +3207,15 @@ The following abstract operations support the implementation of the
   1. Set |firstPendingPullInto|'s [=pull-into descriptor/buffer=] to !
      [$TransferArrayBuffer$](|firstPendingPullInto|'s [=pull-into descriptor/buffer=]).
  1. Perform ! [$ReadableByteStreamControllerInvalidateBYOBRequest$](|controller|).
- 1. If ! [$ReadableStreamHasDefaultReader$](|stream|) is true
+ 1. If |controller|.[=ReadableByteStreamController/[[pendingPullIntos]]=] is not

Can we merge this with the previous "If controller.[[pendingPullIntos]] is not empty" block? I think it should be OK; the intermediate step should not do anything observable...

> @@ -3156,7 +3207,15 @@ The following abstract operations support the implementation of the
   1. Set |firstPendingPullInto|'s [=pull-into descriptor/buffer=] to !
      [$TransferArrayBuffer$](|firstPendingPullInto|'s [=pull-into descriptor/buffer=]).
  1. Perform ! [$ReadableByteStreamControllerInvalidateBYOBRequest$](|controller|).
- 1. If ! [$ReadableStreamHasDefaultReader$](|stream|) is true
+ 1. If |controller|.[=ReadableByteStreamController/[[pendingPullIntos]]=] is not
+    [=list/is empty|empty=],
+  1. Let |firstPendingPullInto| be
+     |controller|.[=ReadableByteStreamController/[[pendingPullIntos]]=][0].
+  1. If |firstPendingPullInto|'s [=pull-into descriptor/reader type=] is "`none`",
+   1. Perform ? [$ReadableByteStreamControllerEnqueueDetachedPullIntoToQueue$](|controller|,

Nit: I like to avoid nesting single-step blocks; this can just be `If X, Y` in one line.

> @@ -3435,6 +3539,8 @@ The following abstract operations support the implementation of the
  |firstDescriptor|)</dfn> performs the following steps:
 
  1. Assert: |firstDescriptor|'s [=pull-into descriptor/bytes filled=] is 0.
+ 1. If |firstDescriptor|'s [=pull-into descriptor/reader type=] is "`none`",
+  1. Perform ! [$ReadableByteStreamControllerShiftPendingPullInto$](|controller|).

Same single-nested-step nit here.

> @@ -3196,6 +3255,20 @@ The following abstract operations support the implementation of the
     |controller|.[=ReadableByteStreamController/[[queueTotalSize]]=] + |byteLength|.
 </div>
 
+<div algorithm>
+ <dfn abstract-op lt="ReadableByteStreamControllerEnqueueDetachedPullIntoToQueue">ReadableByteStreamControllerEnqueueDetachedPullIntoToQueue(|controller|,

Maybe "Enqueue ... to queue" is not necessary? Although hmm, I see it has a sibling following a similar naming scheme. Oh well.

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

Message ID: <whatwg/streams/pull/1168/review/846127110@github.com>