Skip to content

Commit

Permalink
Streams: Mark promises as silent before rejecting them
Browse files Browse the repository at this point in the history
This marks the `ready` and `closed` promises as silent before rejecting
them. This is to prevent the inspector from breaking on exceptions whenever
a stream is released.

Bug: 1132506
Change-Id: If304579837e6434ff12abb8d1ffe73ef34e1220b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3002072
Reviewed-by: Adam Rice <ricea@chromium.org>
Commit-Queue: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#899837}
  • Loading branch information
jespertheend authored and Chromium LUCI CQ committed Jul 9, 2021
1 parent 4e89785 commit b279210
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 9 deletions.
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,7 @@ Jeroen Van den Berghe <vandenberghe.jeroen@gmail.com>
Jerry Lin <wahahab11@gmail.com>
Jerry Zhang <zhj8407@gmail.com>
Jesper Storm Bache <jsbache@gmail.com>
Jesper van den Ende <jespertheend@gmail.com>
Jesse Miller <jesse@jmiller.biz>
Jesus Sanchez-Palencia <jesus.sanchez-palencia.fernandez.fil@intel.com>
Jiadong Chen <chenjiadong@huawei.com>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ void ReadableStreamGenericReader::GenericRelease(
// 3. If reader.[[ownerReadableStream]].[[state]] is "readable", reject
// reader.[[closedPromise]] with a TypeError exception.
if (reader->owner_readable_stream_->state_ == ReadableStream::kReadable) {
reader->closed_promise_->MarkAsSilent(isolate);
reader->closed_promise_->Reject(
script_state,
v8::Exception::TypeError(V8String(
Expand All @@ -79,7 +80,7 @@ void ReadableStreamGenericReader::GenericRelease(
} else {
// 4. Otherwise, set reader.[[closedPromise]] to a promise rejected with a
// TypeError exception.
reader->closed_promise_ = StreamPromiseResolver::CreateRejected(
reader->closed_promise_ = StreamPromiseResolver::CreateRejectedAndSilent(
script_state, v8::Exception::TypeError(V8String(
isolate,
"This readable stream reader has been released and "
Expand Down Expand Up @@ -152,7 +153,7 @@ void ReadableStreamGenericReader::GenericInitialize(

// b. Set reader.[[closedPromise]] to a promise rejected with stream.
// [[storedError]].
reader->closed_promise_ = StreamPromiseResolver::CreateRejected(
reader->closed_promise_ = StreamPromiseResolver::CreateRejectedAndSilent(
script_state, stream->GetStoredError(isolate));

// c. Set reader.[[closedPromise]].[[PromiseIsHandled]] to true.
Expand Down
17 changes: 17 additions & 0 deletions third_party/blink/renderer/core/streams/stream_promise_resolver.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,15 @@ StreamPromiseResolver* StreamPromiseResolver::CreateRejected(
return promise;
}

StreamPromiseResolver* StreamPromiseResolver::CreateRejectedAndSilent(
ScriptState* script_state,
v8::Local<v8::Value> reason) {
auto* promise = MakeGarbageCollected<StreamPromiseResolver>(script_state);
promise->MarkAsSilent(script_state->GetIsolate());
promise->Reject(script_state, reason);
return promise;
}

StreamPromiseResolver::StreamPromiseResolver(ScriptState* script_state) {
v8::Local<v8::Promise::Resolver> resolver;
if (v8::Promise::Resolver::New(script_state->GetContext())
Expand Down Expand Up @@ -103,6 +112,14 @@ void StreamPromiseResolver::MarkAsHandled(v8::Isolate* isolate) {
promise->MarkAsHandled();
}

void StreamPromiseResolver::MarkAsSilent(v8::Isolate* isolate) {
v8::Local<v8::Promise> promise = V8Promise(isolate);
if (promise.IsEmpty()) {
return;
}
promise->MarkAsSilent();
}

v8::Promise::PromiseState StreamPromiseResolver::State(
v8::Isolate* isolate) const {
v8::Local<v8::Promise> promise = V8Promise(isolate);
Expand Down
10 changes: 10 additions & 0 deletions third_party/blink/renderer/core/streams/stream_promise_resolver.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ class CORE_EXPORT StreamPromiseResolver final
static StreamPromiseResolver* CreateRejected(ScriptState*,
v8::Local<v8::Value> reason);

// Similar to CreateRejected but marks the promise as silent before rejecting.
// https://crbug.com/1132506
static StreamPromiseResolver* CreateRejectedAndSilent(
ScriptState*,
v8::Local<v8::Value> reason);

// Creates an initialised promise.
explicit StreamPromiseResolver(ScriptState*);

Expand All @@ -70,6 +76,10 @@ class CORE_EXPORT StreamPromiseResolver final
// an unhandled rejection.
void MarkAsHandled(v8::Isolate*);

// Marks the promise as silent so that it doesn't pause the debugger when it
// rejects.
void MarkAsSilent(v8::Isolate*);

// Returns the state of the promise, one of pending, fulfilled or rejected.
v8::Promise::PromiseState State(v8::Isolate*) const;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ WritableStreamDefaultWriter::WritableStreamDefaultWriter(
case WritableStream::kErroring: {
// a. Set this.[[readyPromise]] to a promise rejected with
// stream.[[storedError]].
ready_promise_ = StreamPromiseResolver::CreateRejected(
ready_promise_ = StreamPromiseResolver::CreateRejectedAndSilent(
script_state, stream->GetStoredError(isolate));

// b. Set this.[[readyPromise]].[[PromiseIsHandled]] to true.
Expand Down Expand Up @@ -127,16 +127,16 @@ WritableStreamDefaultWriter::WritableStreamDefaultWriter(

// c. Set this.[[readyPromise]] to a promise rejected with
// storedError.
ready_promise_ =
StreamPromiseResolver::CreateRejected(script_state, stored_error);
ready_promise_ = StreamPromiseResolver::CreateRejectedAndSilent(
script_state, stored_error);

// d. Set this.[[readyPromise]].[[PromiseIsHandled]] to true.
ready_promise_->MarkAsHandled(isolate);

// e. Set this.[[closedPromise]] to a promise rejected with
// storedError.
closed_promise_ =
StreamPromiseResolver::CreateRejected(script_state, stored_error);
closed_promise_ = StreamPromiseResolver::CreateRejectedAndSilent(
script_state, stored_error);

// f. Set this.[[closedPromise]].[[PromiseIsHandled]] to true.
closed_promise_->MarkAsHandled(isolate);
Expand Down Expand Up @@ -284,12 +284,13 @@ void WritableStreamDefaultWriter::EnsureReadyPromiseRejected(
// 1. If writer.[[readyPromise]].[[PromiseState]] is "pending", reject
// writer.[[readyPromise]] with error.
if (!writer->ready_promise_->IsSettled()) {
writer->ready_promise_->MarkAsSilent(isolate);
writer->ready_promise_->Reject(script_state, error);
} else {
// 2. Otherwise, set writer.[[readyPromise]] to a promise rejected with
// error.
writer->ready_promise_ =
StreamPromiseResolver::CreateRejected(script_state, error);
StreamPromiseResolver::CreateRejectedAndSilent(script_state, error);
}

// 3. Set writer.[[readyPromise]].[[PromiseIsHandled]] to true.
Expand Down Expand Up @@ -517,12 +518,13 @@ void WritableStreamDefaultWriter::EnsureClosedPromiseRejected(
// 1. If writer.[[closedPromise]].[[PromiseState]] is "pending", reject
// writer.[[closedPromise]] with error.
if (!writer->closed_promise_->IsSettled()) {
writer->closed_promise_->MarkAsSilent(isolate);
writer->closed_promise_->Reject(script_state, error);
} else {
// 2. Otherwise, set writer.[[closedPromise]] to a promise rejected with
// error.
writer->closed_promise_ =
StreamPromiseResolver::CreateRejected(script_state, error);
StreamPromiseResolver::CreateRejectedAndSilent(script_state, error);
}

// 3. Set writer.[[closedPromise]].[[PromiseIsHandled]] to true.
Expand Down

0 comments on commit b279210

Please sign in to comment.