[v8][wasm] Avoid repeated streaming abortion

We see crashes that could be triggered e.g. by concurrent abortion of
Wasm streaming by the client and via the network, or concurrent abortion
and finishing the download.

We do not have a reproducer yet, but not calling {Abort} or {Finish}
multiple times might fix the issue and is a good convention anyway.

To make the order of events easier to follow, this also converts a scope
object to a function call to convert the exception to a streaming abort
message.

R=​haraken@chromium.org

(cherry picked from commit c8536db)

Bug: 1399790, 1400066, 1406746
Change-Id: I90eaa51b59976b0d92893a706d626a9826937393
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4249278
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1105507}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4270991
Auto-Submit: Daniel Yip <danielyip@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Daniel Yip <danielyip@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5481@{#1208}
Cr-Branched-From: 130f3e4-refs/heads/main@{#1084008}

third_party/blink/renderer/bindings/core/v8/v8_wasm_response_extensions.cc

@@ -237,13 +237,18 @@ 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");
           {
             ScriptState::Scope scope(script_state_);
             streaming_->Finish(HasValidCodeCache());
           }
           client_->DidFetchDataLoadedCustomFormat();
+          streaming_.reset();
           return;
         }
         case BytesConsumer::Result::kError: {
@@ -270,6 +275,10 @@ class FetchDataLoaderForWasmStreaming final : public FetchDataLoader,
   }
 
   void AbortFromClient() {
+    // Ignore a repeated abort request, or abort after successfully finishing.
+    if (!streaming_) {
+      return;
+    }
     auto* exception =
         MakeGarbageCollected<DOMException>(DOMExceptionCode::kAbortError);
     ScriptState::Scope scope(script_state_);
@@ -286,13 +295,18 @@ class FetchDataLoaderForWasmStreaming final : public FetchDataLoader,
           ToV8Traits<DOMException>::ToV8(script_state_, exception)
               .ToLocalChecked();
       streaming_->Abort(v8_exception);
+      streaming_.reset();
     }
   }
 
  private:
   // TODO(ahaas): replace with spec-ed error types, once spec clarifies
   // what they are.
   void AbortCompilation() {
+    // Ignore a repeated abort request, or abort after successfully finishing.
+    if (!streaming_) {
+      return;
+    }
     if (script_state_->ContextIsValid()) {
       ScriptState::Scope scope(script_state_);
       streaming_->Abort(V8ThrowException::CreateTypeError(
@@ -304,6 +318,7 @@ class FetchDataLoaderForWasmStreaming final : public FetchDataLoader,
       // rejected.
       streaming_->Abort(v8::Local<v8::Value>());
     }
+    streaming_.reset();
   }
 
   CodeCacheState MaybeConsumeCodeCache() {
@@ -441,37 +456,18 @@ class WasmDataLoaderClient final
   Member<FetchDataLoaderForWasmStreaming> loader_;
 };
 
-// ExceptionToAbortStreamingScope converts a possible exception to an abort
-// message for WasmStreaming instead of throwing the exception.
-//
-// All exceptions which happen in the setup of WebAssembly streaming compilation
-// have to be passed as an abort message to V8 so that V8 can reject the promise
-// associated to the streaming compilation.
-class ExceptionToAbortStreamingScope {
-  STACK_ALLOCATED();
-
- public:
-  ExceptionToAbortStreamingScope(std::shared_ptr<v8::WasmStreaming> streaming,
-                                 ExceptionState& exception_state)
-      : streaming_(streaming), exception_state_(exception_state) {}
-
-  ExceptionToAbortStreamingScope(const ExceptionToAbortStreamingScope&) =
-      delete;
-  ExceptionToAbortStreamingScope& operator=(
-      const ExceptionToAbortStreamingScope&) = delete;
-
-  ~ExceptionToAbortStreamingScope() {
-    if (!exception_state_.HadException())
-      return;
-
-    streaming_->Abort(exception_state_.GetException());
-    exception_state_.ClearException();
-  }
-
- private:
-  std::shared_ptr<v8::WasmStreaming> streaming_;
-  ExceptionState& exception_state_;
-};
+// Convert an exception to an abort message for WasmStreaming. This rejects the
+// promise instead of actually throwing the exception.
+// No further methods should be called on the WasmStreaming object afterwards,
+// hence we receive the shared_ptr by reference and clear it.
+void PropagateExceptionToWasmStreaming(
+    ExceptionState& exception_state,
+    std::shared_ptr<v8::WasmStreaming>& streaming) {
+  DCHECK(exception_state.HadException());
+  streaming->Abort(exception_state.GetException());
+  streaming.reset();
+  exception_state.ClearException();
+}
 
 scoped_refptr<base::SingleThreadTaskRunner> GetContextTaskRunner(
     ExecutionContext& execution_context) {
@@ -513,7 +509,6 @@ void StreamFromResponseCallback(
                                  "WebAssembly", "compile");
   std::shared_ptr<v8::WasmStreaming> streaming =
       v8::WasmStreaming::Unpack(args.GetIsolate(), args.Data());
-  ExceptionToAbortStreamingScope exception_scope(streaming, exception_state);
 
   ScriptState* script_state = ScriptState::ForCurrentRealm(args);
   if (!script_state->ContextIsValid()) {
@@ -551,13 +546,15 @@ void StreamFromResponseCallback(
     exception_state.ThrowTypeError(
         "An argument must be provided, which must be a "
         "Response or Promise<Response> object");
+    PropagateExceptionToWasmStreaming(exception_state, streaming);
     return;
   }
 
   if (!response->ok()) {
     base::UmaHistogramEnumeration("V8.WasmStreamingInputType",
                                   WasmStreamingInputType::kResponseNotOK);
     exception_state.ThrowTypeError("HTTP status code is not ok");
+    PropagateExceptionToWasmStreaming(exception_state, streaming);
     return;
   }
 
@@ -569,6 +566,7 @@ void StreamFromResponseCallback(
                                   WasmStreamingInputType::kWrongMimeType);
     exception_state.ThrowTypeError(
         "Incorrect response MIME type. Expected 'application/wasm'.");
+    PropagateExceptionToWasmStreaming(exception_state, streaming);
     return;
   }
 
@@ -577,6 +575,7 @@ void StreamFromResponseCallback(
                                   WasmStreamingInputType::kReponseLocked);
     exception_state.ThrowTypeError(
         "Cannot compile WebAssembly.Module from an already read Response");
+    PropagateExceptionToWasmStreaming(exception_state, streaming);
     return;
   }
 
@@ -586,6 +585,7 @@ void StreamFromResponseCallback(
     // Since the status is 2xx (ok), this must be status 204 (No Content),
     // status 205 (Reset Content) or a malformed status 200 (OK).
     exception_state.ThrowWasmCompileError("Empty WebAssembly module");
+    PropagateExceptionToWasmStreaming(exception_state, streaming);
     return;
   }
 
@@ -626,9 +626,11 @@ void StreamFromResponseCallback(
         });
   }
 
+  DCHECK(!exception_state.HadException());
   FetchDataLoaderForWasmStreaming* loader =
       MakeGarbageCollected<FetchDataLoaderForWasmStreaming>(
-          url, streaming, script_state, cache_handler, code_caching_callback);
+          url, std::move(streaming), script_state, cache_handler,
+          code_caching_callback);
   response->BodyBuffer()->StartLoading(
       loader, MakeGarbageCollected<WasmDataLoaderClient>(loader),
       exception_state);