Skip to content

Commit

Permalink
[wasm] Fix handling of streaming cancellation
Browse files Browse the repository at this point in the history
When the stream gets cancelled, we receive a
`BytesConsumer::Result::kError` value in `OnStateChange`. In this case,
there is no network error though, so `consume_->GetError()` will return
an empty string or run into a DCHECK in debug builds.

This CL handles this better by checking the state of the consumer before
calling `GetError()`, and producing two different error messages
depending on whether the consumer is closed or errored (which are the
two defined final states).

R=ahaas@chromium.org

Bug: chromium:1448858, chromium:1449546
Change-Id: I2efae5454944c94dd4baa109a55b132b974295c9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4608425
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1157080}
  • Loading branch information
backes authored and Chromium LUCI CQ committed Jun 13, 2023
1 parent d255273 commit a542e0b
Showing 1 changed file with 16 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,9 @@ class FetchDataLoaderForWasmStreaming final : public FetchDataLoader,
void OnStateChange() override {
TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("devtools.timeline"),
"v8.wasm.compileConsume");
while (true) {
// Continue reading until we either finished, aborted, or no data is
// available any more (handled below).
while (streaming_) {
// |buffer| is owned by |consumer_|.
const char* buffer = nullptr;
size_t available = 0;
Expand All @@ -215,7 +217,7 @@ class FetchDataLoaderForWasmStreaming final : public FetchDataLoader,
return;
if (result == BytesConsumer::Result::kOk) {
// Ignore more bytes after an abort (streaming == nullptr).
if (available > 0 && streaming_) {
if (available > 0) {
if (code_cache_state_ == CodeCacheState::kBeforeFirstByte)
code_cache_state_ = MaybeConsumeCodeCache();

Expand All @@ -238,10 +240,6 @@ class FetchDataLoaderForWasmStreaming final : public FetchDataLoader,
case BytesConsumer::Result::kOk:
break;
case BytesConsumer::Result::kDone: {
// Ignore this event if we already aborted.
if (!streaming_) {
return;
}
TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("devtools.timeline"),
"v8.wasm.compileConsumeDone");
{
Expand All @@ -253,8 +251,18 @@ class FetchDataLoaderForWasmStreaming final : public FetchDataLoader,
return;
}
case BytesConsumer::Result::kError:
return AbortCompilation("Network error: " +
consumer_->GetError().Message());
switch (consumer_->GetPublicState()) {
case BytesConsumer::PublicState::kClosed:
AbortCompilation("Download cancelled");
break;
case BytesConsumer::PublicState::kErrored:
AbortCompilation("Network error: " +
consumer_->GetError().Message());
break;
default:
NOTREACHED();
}
break;
}
}
}
Expand Down

0 comments on commit a542e0b

Please sign in to comment.