Browse Source

fix: wasm code generation in the renderer (#25777)

Shelley Vohr 4 years ago
parent
commit
042d25e926

+ 1 - 0
patches/chromium/.patches

@@ -100,3 +100,4 @@ worker_feat_add_hook_to_notify_script_ready.patch
 fix_properly_honor_printing_page_ranges.patch
 remove_deprecated_factory_parameter_for_worker_resource_loader.patch
 fix_use_electron_generated_resources.patch
+chore_expose_v8_initialization_isolate_callbacks.patch

+ 37 - 0
patches/chromium/chore_expose_v8_initialization_isolate_callbacks.patch

@@ -0,0 +1,37 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Shelley Vohr <[email protected]>
+Date: Mon, 5 Oct 2020 13:43:59 -0700
+Subject: chore: expose v8 initialization isolate callbacks
+
+This commit is necessary in order to ensure consistent behavior from
+v8 Isolate callbacks in contexts which Node.js does not control. If
+we're running with contextIsolation enabled, we should be falling back
+to Blink's logic. This will be upstreamed in some form.
+
+diff --git a/third_party/blink/renderer/bindings/core/v8/v8_initializer.cc b/third_party/blink/renderer/bindings/core/v8/v8_initializer.cc
+index 7c2ae147254dad017214d2535203a18719294768..3313d6aff572307777f02801b09fc3d29318050e 100644
+--- a/third_party/blink/renderer/bindings/core/v8/v8_initializer.cc
++++ b/third_party/blink/renderer/bindings/core/v8/v8_initializer.cc
+@@ -442,7 +442,7 @@ CodeGenerationCheckCallbackInMainThread(v8::Local<v8::Context> context,
+   return {true, std::move(stringified_source)};
+ }
+ 
+-static bool WasmCodeGenerationCheckCallbackInMainThread(
++bool V8Initializer::WasmCodeGenerationCheckCallbackInMainThread(
+     v8::Local<v8::Context> context,
+     v8::Local<v8::String> source) {
+   if (ExecutionContext* execution_context = ToExecutionContext(context)) {
+diff --git a/third_party/blink/renderer/bindings/core/v8/v8_initializer.h b/third_party/blink/renderer/bindings/core/v8/v8_initializer.h
+index e7cbc5db7d15aa0fcfb37ba261673b973827296a..6b93aa449a005e06862a99ea0c9b751ffff2d6ec 100644
+--- a/third_party/blink/renderer/bindings/core/v8/v8_initializer.h
++++ b/third_party/blink/renderer/bindings/core/v8/v8_initializer.h
+@@ -67,6 +67,9 @@ class CORE_EXPORT V8Initializer {
+                                          v8::Local<v8::Value>);
+   static void MessageHandlerInWorker(v8::Local<v8::Message>,
+                                      v8::Local<v8::Value>);
++  static bool WasmCodeGenerationCheckCallbackInMainThread(
++    v8::Local<v8::Context> context,
++    v8::Local<v8::String> source);
+ };
+ 
+ }  // namespace blink

+ 1 - 0
patches/node/.patches

@@ -29,3 +29,4 @@ crypto_update_certdata_to_nss_3_56.patch
 fix_-wincompatible-pointer-types-discards-qualifiers_error.patch
 fix_add_v8_enable_reverse_jsargs_defines_in_common_gypi.patch
 fix_allow_preventing_initializeinspector_in_env.patch
+chore_expose_v8_initialization_isolate_callbacks.patch

+ 89 - 0
patches/node/chore_expose_v8_initialization_isolate_callbacks.patch

@@ -0,0 +1,89 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Shelley Vohr <[email protected]>
+Date: Mon, 5 Oct 2020 16:05:45 -0700
+Subject: chore: expose v8 initialization isolate callbacks
+
+Exposes v8 initializer callbacks to Electron so that we can call them
+directly. We expand upon and adapt their behavior, so allows us to
+ensure that we stay in sync with Node.js default behavior.
+
+This will be upstreamed.
+
+diff --git a/src/api/environment.cc b/src/api/environment.cc
+index cf6115d04ba3c184937c5db85c9d7ebc78ed3db7..a8ee97729858b40179e6bce58cfbacf1a6980340 100644
+--- a/src/api/environment.cc
++++ b/src/api/environment.cc
+@@ -30,14 +30,16 @@ using v8::PropertyDescriptor;
+ using v8::String;
+ using v8::Value;
+ 
+-static bool AllowWasmCodeGenerationCallback(Local<Context> context,
++// static
++bool Environment::AllowWasmCodeGenerationCallback(Local<Context> context,
+                                             Local<String>) {
+   Local<Value> wasm_code_gen =
+       context->GetEmbedderData(ContextEmbedderIndex::kAllowWasmCodeGeneration);
+   return wasm_code_gen->IsUndefined() || wasm_code_gen->IsTrue();
+ }
+ 
+-static bool ShouldAbortOnUncaughtException(Isolate* isolate) {
++// static
++bool Environment::ShouldAbortOnUncaughtException(Isolate* isolate) {
+   DebugSealHandleScope scope(isolate);
+   Environment* env = Environment::GetCurrent(isolate);
+   return env != nullptr &&
+@@ -47,7 +49,8 @@ static bool ShouldAbortOnUncaughtException(Isolate* isolate) {
+          !env->inside_should_not_abort_on_uncaught_scope();
+ }
+ 
+-static MaybeLocal<Value> PrepareStackTraceCallback(Local<Context> context,
++// static
++MaybeLocal<Value> Environment::PrepareStackTraceCallback(Local<Context> context,
+                                       Local<Value> exception,
+                                       Local<Array> trace) {
+   Environment* env = Environment::GetCurrent(context);
+@@ -221,7 +224,7 @@ void SetIsolateErrorHandlers(v8::Isolate* isolate, const IsolateSettings& s) {
+ 
+   auto* abort_callback = s.should_abort_on_uncaught_exception_callback ?
+       s.should_abort_on_uncaught_exception_callback :
+-      ShouldAbortOnUncaughtException;
++      Environment::ShouldAbortOnUncaughtException;
+   isolate->SetAbortOnUncaughtExceptionCallback(abort_callback);
+ 
+   auto* fatal_error_cb = s.fatal_error_callback ?
+@@ -229,7 +232,7 @@ void SetIsolateErrorHandlers(v8::Isolate* isolate, const IsolateSettings& s) {
+   isolate->SetFatalErrorHandler(fatal_error_cb);
+ 
+   auto* prepare_stack_trace_cb = s.prepare_stack_trace_callback ?
+-      s.prepare_stack_trace_callback : PrepareStackTraceCallback;
++      s.prepare_stack_trace_callback : Environment::PrepareStackTraceCallback;
+   isolate->SetPrepareStackTraceCallback(prepare_stack_trace_cb);
+ }
+ 
+@@ -237,7 +240,7 @@ void SetIsolateMiscHandlers(v8::Isolate* isolate, const IsolateSettings& s) {
+   isolate->SetMicrotasksPolicy(s.policy);
+ 
+   auto* allow_wasm_codegen_cb = s.allow_wasm_code_generation_callback ?
+-    s.allow_wasm_code_generation_callback : AllowWasmCodeGenerationCallback;
++    s.allow_wasm_code_generation_callback : Environment::AllowWasmCodeGenerationCallback;
+   isolate->SetAllowWasmCodeGenerationCallback(allow_wasm_codegen_cb);
+ 
+   if ((s.flags & SHOULD_NOT_SET_PROMISE_REJECTION_CALLBACK) == 0) {
+diff --git a/src/env.h b/src/env.h
+index 4fe2eb3b7699efcab87c377743a955effbbfd9de..2bb196d199bd9b6c27d9de8ca7e5a9bffad0c788 100644
+--- a/src/env.h
++++ b/src/env.h
+@@ -904,6 +904,13 @@ class Environment : public MemoryRetainer {
+   void Exit(int code);
+   void ExitEnv();
+ 
++  static bool AllowWasmCodeGenerationCallback(v8::Local<v8::Context> context,
++                                       v8::Local<v8::String>);
++  static bool ShouldAbortOnUncaughtException(v8::Isolate* isolate);
++  static v8::MaybeLocal<v8::Value> PrepareStackTraceCallback(v8::Local<v8::Context> context,
++                                      v8::Local<v8::Value> exception,
++                                      v8::Local<v8::Array> trace);
++
+   // Register clean-up cb to be called on environment destruction.
+   inline void RegisterHandleCleanup(uv_handle_t* handle,
+                                     HandleCleanupCb cb,

+ 21 - 0
shell/common/node_bindings.cc

@@ -33,6 +33,7 @@
 #include "shell/common/gin_helper/microtasks_scope.h"
 #include "shell/common/mac/main_application_bundle.h"
 #include "shell/common/node_includes.h"
+#include "third_party/blink/renderer/bindings/core/v8/v8_initializer.h"  // nogncheck
 
 #if !defined(MAS_BUILD)
 #include "shell/common/crash_keys.h"
@@ -152,6 +153,22 @@ void V8FatalErrorCallback(const char* location, const char* message) {
   *zero = 0;
 }
 
+bool AllowWasmCodeGenerationCallback(v8::Local<v8::Context> context,
+                                     v8::Local<v8::String>) {
+  // If we're running with contextIsolation enabled in the renderer process,
+  // fall back to Blink's logic.
+  v8::Isolate* isolate = context->GetIsolate();
+  if (node::Environment::GetCurrent(isolate) == nullptr) {
+    if (gin_helper::Locker::IsBrowserProcess())
+      return false;
+    return blink::V8Initializer::WasmCodeGenerationCheckCallbackInMainThread(
+        context, v8::String::Empty(isolate));
+  }
+
+  return node::Environment::AllowWasmCodeGenerationCallback(
+      context, v8::String::Empty(isolate));
+}
+
 // Initialize Node.js cli options to pass to Node.js
 // See https://nodejs.org/api/cli.html#cli_options
 void SetNodeCliFlags() {
@@ -436,6 +453,10 @@ node::Environment* NodeBindings::CreateEnvironment(
     return false;
   };
 
+  // Use a custom callback here to allow us to leverage Blink's logic in the
+  // renderer process.
+  is.allow_wasm_code_generation_callback = AllowWasmCodeGenerationCallback;
+
   if (browser_env_ == BrowserEnvironment::BROWSER) {
     // Node.js requires that microtask checkpoints be explicitly invoked.
     is.policy = v8::MicrotasksPolicy::kExplicit;