[wasm][streaming] Change the signature of the Abort API function to MaybeLocal

When streaming compilation for WebAssembly gets aborted, we reject the
promise associated with the compilation. However, in some circumstances,
e.g. when streaming comilation gets aborted because the browser tab gets
refreshed, then we want to omit rejecting the promise. In an older CL
(https://crrev.com/c/876103) we omit rejecting the promise when the
exception value is null. With this CL the exception value is a MaybeLocal
so that we document properly that the value can be null. In addition, I
added documentation to say that in that case we do not reject the promise.

R=adamk@chromium.org

Bug: chromium:803838
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
Change-Id: I6a093e61c8ec63f7ae385a7f77ae6178e7b34a06
Reviewed-on: https://chromium-review.googlesource.com/897647Reviewed-by: Adam Klein <adamk@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51211}

include/v8.h

@@ -4143,7 +4143,10 @@ class V8_EXPORT WasmModuleObjectBuilderStreaming final {
   // The buffer passed into OnBytesReceived is owned by the caller.
   void OnBytesReceived(const uint8_t*, size_t size);
   void Finish();
-  void Abort(Local<Value> exception);
+  // Abort streaming compilation. If {exception} has a value, then the promise
+  // associated with streaming compilation is rejected with that value. If
+  // {exception} does not have value, the promise does not get rejected.
+  void Abort(MaybeLocal<Value> exception);
   Local<Promise> GetPromise();
 
   ~WasmModuleObjectBuilderStreaming();

src/api.cc

@@ -7686,24 +7686,23 @@ void WasmModuleObjectBuilderStreaming::Finish() {
       {wire_bytes.get(), wire_bytes.get() + total_size_}, false);
 }
 
-void WasmModuleObjectBuilderStreaming::Abort(Local<Value> exception) {
+void WasmModuleObjectBuilderStreaming::Abort(MaybeLocal<Value> exception) {
   Local<Promise> promise = GetPromise();
   // The promise has already been resolved, e.g. because of a compilation
   // error.
   if (promise->State() != v8::Promise::kPending) return;
   if (i::FLAG_wasm_stream_compilation) streaming_decoder_->Abort();
 
-  // If there is no exception, then we do not reject the promise. The reason is
-  // that 'no exception' indicates that we are in a ScriptForbiddenScope, which
-  // means that it is not allowed to reject the promise at the moment, or
-  // execute any other JavaScript code.
+  // If no exception value is provided, we do not reject the promise. This can
+  // happen when streaming compilation gets aborted when no script execution is
+  // allowed anymore, e.g. when a browser tab gets refreshed.
   if (exception.IsEmpty()) return;
 
   Local<Promise::Resolver> resolver = promise.As<Promise::Resolver>();
   i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate_);
   i::HandleScope scope(i_isolate);
   Local<Context> context = Utils::ToLocal(handle(i_isolate->context()));
-  auto maybe = resolver->Reject(context, exception);
+  auto maybe = resolver->Reject(context, exception.ToLocalChecked());
   CHECK_IMPLIES(!maybe.FromMaybe(false), i_isolate->has_scheduled_exception());
 }
 

test/cctest/test-api.cc

@@ -26918,3 +26918,21 @@ TEST(PersistentValueMap) {
           .ToLocalChecked();
   map.Set("key", value);
 }
+
+TEST(WasmStreamingAbort) {
+  LocalContext env;
+  v8::Isolate* isolate = env->GetIsolate();
+  v8::HandleScope scope(isolate);
+  v8::WasmModuleObjectBuilderStreaming streaming(isolate);
+  streaming.Abort(v8::Object::New(isolate));
+  CHECK_EQ(streaming.GetPromise()->State(), v8::Promise::kRejected);
+}
+
+TEST(WasmStreamingAbortNoReject) {
+  LocalContext env;
+  v8::Isolate* isolate = env->GetIsolate();
+  v8::HandleScope scope(isolate);
+  v8::WasmModuleObjectBuilderStreaming streaming(isolate);
+  streaming.Abort({});
+  CHECK_EQ(streaming.GetPromise()->State(), v8::Promise::kPending);
+}