From a542e0b91fd5eeb32e35803a94499202c9bd27ff Mon Sep 17 00:00:00 2001 From: Clemens Backes Date: Tue, 13 Jun 2023 18:38:22 +0000 Subject: [PATCH] [wasm] Fix handling of streaming cancellation 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 Commit-Queue: Clemens Backes Cr-Commit-Position: refs/heads/main@{#1157080} --- .../core/v8/v8_wasm_response_extensions.cc | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/third_party/blink/renderer/bindings/core/v8/v8_wasm_response_extensions.cc b/third_party/blink/renderer/bindings/core/v8/v8_wasm_response_extensions.cc index 166384f2d32c0f..35a1c38a261b2c 100644 --- a/third_party/blink/renderer/bindings/core/v8/v8_wasm_response_extensions.cc +++ b/third_party/blink/renderer/bindings/core/v8/v8_wasm_response_extensions.cc @@ -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; @@ -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(); @@ -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"); { @@ -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; } } }