Skip to content

Commit

Permalink
ReadableStream: Resolve BYOB reads immediately on cancel
Browse files Browse the repository at this point in the history
This CL is based on spec changes made in whatwg/streams#1129.

Summary/context of changes:
If you cancel a stream while a BYOB read is pending, the stream
should simply not give back the buffer. This means that cancel()
can immediately resolve all pending BYOB reads with the classic
{ done: true, value: undefined }, without having to wait for the
underlying byte source. Also, when the stream is cancelled, any
pending BYOB request is now immediately invalidated, so the
underlying byte source doesn't erroneously think that it still
needs to provide a response.

Bug: 1223565
Change-Id: I17d1d5cfaee80c02c51bc682447b381ade4363bf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3216778
Commit-Queue: Nidhi Jaju <nidhijaju@chromium.org>
Reviewed-by: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/main@{#930987}
  • Loading branch information
nidhijaju authored and Chromium LUCI CQ committed Oct 13, 2021
1 parent 49ad2e4 commit e8c4592
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1442,13 +1442,8 @@ v8::Local<v8::Promise> ReadableByteStreamController::CancelSteps(
ScriptState* script_state,
v8::Local<v8::Value> reason) {
// https://streams.spec.whatwg.org/#rbs-controller-private-cancel
// 1. If this.[[pendingPullIntos]] is not empty,
if (!pending_pull_intos_.IsEmpty()) {
// a. Let firstDescriptor be this.[[pendingPullIntos]][0].
PullIntoDescriptor* first_descriptor = pending_pull_intos_[0];
// b. Set firstDescriptor’s bytes filled to 0.
first_descriptor->bytes_filled = 0;
}
// 1. Perform ! ReadableByteStreamControllerClearPendingPullIntos(this).
ClearPendingPullIntos(this);
// 2. Perform ! ResetQueue(this).
ResetQueue(this);
// 3. Let result be the result of performing this.[[cancelAlgorithm]], passing
Expand Down
22 changes: 20 additions & 2 deletions third_party/blink/renderer/core/streams/readable_stream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1732,7 +1732,25 @@ v8::Local<v8::Promise> ReadableStream::Cancel(ScriptState* script_state,
// 4. Perform ! ReadableStreamClose(stream).
Close(script_state, stream);

// 5. Let sourceCancelPromise be ! stream.[[readableStreamController]].
// 5. Let reader be stream.[[reader]].
ReadableStreamGenericReader* reader = stream->reader_;

// 6. If reader is not undefined and reader implements
// ReadableStreamBYOBReader,
if (reader && reader->IsBYOBReader()) {
// a. For each readIntoRequest of reader.[[readIntoRequests]],
ReadableStreamBYOBReader* byob_reader =
To<ReadableStreamBYOBReader>(reader);
for (ReadableStreamBYOBReader::ReadIntoRequest* request :
byob_reader->read_into_requests_) {
// i. Perform readIntoRequest's close steps, given undefined.
request->CloseSteps(script_state, nullptr);
}
// b. Set reader.[[readIntoRequests]] to an empty list.
byob_reader->read_into_requests_.clear();
}

// 7. Let sourceCancelPromise be ! stream.[[readableStreamController]].
// [[CancelSteps]](reason).
v8::Local<v8::Promise> source_cancel_promise =
stream->readable_stream_controller_->CancelSteps(script_state, reason);
Expand All @@ -1747,7 +1765,7 @@ v8::Local<v8::Promise> ReadableStream::Cancel(ScriptState* script_state,
void CallWithLocal(v8::Local<v8::Value>) override {}
};

// 6. Return the result of transforming sourceCancelPromise with a
// 8. Return the result of transforming sourceCancelPromise with a
// fulfillment handler that returns undefined.
return StreamThenPromise(
script_state->GetContext(), source_cancel_promise,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,11 @@ void ReadableStreamBYOBReader::ReadIntoRequest::CloseSteps(
DOMArrayBufferView* chunk) const {
resolver_->Resolve(script_state,
ReadableStream::CreateReadResult(
script_state, ToV8(chunk, script_state), true, true));
script_state,
chunk ? ToV8(chunk, script_state)
: static_cast<v8::Local<v8::Value>>(
v8::Undefined(script_state->GetIsolate())),
true, true));
}

void ReadableStreamBYOBReader::ReadIntoRequest::ErrorSteps(
Expand Down
4 changes: 0 additions & 4 deletions third_party/blink/web_tests/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -3140,10 +3140,8 @@ crbug.com/626703 [ Mac ] external/wpt/streams/readable-byte-streams/non-transfer
crbug.com/626703 [ Win7 ] external/wpt/streams/readable-byte-streams/non-transferable-buffers.any.serviceworker.html [ Crash ]
crbug.com/626703 [ Win10.20h2 ] external/wpt/streams/readable-byte-streams/non-transferable-buffers.any.serviceworker.html [ Crash Timeout ]
crbug.com/626703 [ Mac11.0 ] external/wpt/css/css-counter-styles/counter-style-name-syntax.html [ Failure Timeout ]
crbug.com/626703 external/wpt/streams/readable-byte-streams/general.any.sharedworker.html [ Timeout ]
crbug.com/626703 external/wpt/css/css-sizing/fit-content-length-percentage-015.html [ Failure ]
crbug.com/626703 external/wpt/css/css-sizing/fit-content-length-percentage-014.html [ Failure ]
crbug.com/626703 external/wpt/streams/readable-byte-streams/general.any.html [ Timeout ]
crbug.com/626703 external/wpt/css/css-sizing/fit-content-length-percentage-001.html [ Failure ]
crbug.com/626703 external/wpt/css/css-sizing/fit-content-length-percentage-010.html [ Failure ]
crbug.com/626703 external/wpt/css/css-sizing/fit-content-length-percentage-016.html [ Failure ]
Expand All @@ -3154,7 +3152,6 @@ crbug.com/626703 external/wpt/css/css-grid/grid-model/grid-areas-overflowing-gri
crbug.com/626703 external/wpt/css/css-sizing/fit-content-length-percentage-002.html [ Failure ]
crbug.com/626703 [ Mac10.14 ] external/wpt/pointerevents/pointerevent_pointercapture_in_frame.html?pen [ Timeout ]
crbug.com/626703 [ Mac11.0 ] external/wpt/pointerevents/pointerevent_pointercapture_in_frame.html?pen [ Timeout ]
crbug.com/626703 external/wpt/streams/readable-byte-streams/general.any.worker.html [ Timeout ]
crbug.com/626703 external/wpt/html/cross-origin-opener-policy/iframe-popup-same-origin-to-unsafe-none.https.html [ Skip Timeout ]
crbug.com/626703 external/wpt/css/css-sizing/fit-content-length-percentage-006.html [ Failure ]
crbug.com/626703 external/wpt/css/css-sizing/fit-content-length-percentage-007.html [ Failure ]
Expand All @@ -3171,7 +3168,6 @@ crbug.com/626703 [ Mac ] external/wpt/streams/readable-byte-streams/non-transfer
crbug.com/626703 [ Win7 ] external/wpt/streams/readable-byte-streams/non-transferable-buffers.any.sharedworker.html [ Crash ]
crbug.com/626703 [ Win10.20h2 ] external/wpt/streams/readable-byte-streams/non-transferable-buffers.any.sharedworker.html [ Crash Timeout ]
crbug.com/626703 external/wpt/css/css-sizing/fit-content-length-percentage-008.html [ Failure ]
crbug.com/626703 external/wpt/streams/readable-byte-streams/general.any.serviceworker.html [ Timeout ]
crbug.com/626703 external/wpt/css/css-sizing/fit-content-length-percentage-011.html [ Failure ]
crbug.com/626703 external/wpt/css/css-sizing/fit-content-length-percentage-013.html [ Failure ]
crbug.com/626703 external/wpt/streams/readable-byte-streams/non-transferable-buffers.any.worker.html [ Crash ]
Expand Down

0 comments on commit e8c4592

Please sign in to comment.