Browse Source

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

* fix: wasm code generation in the renderer

* Fixup patches

* update patches

* update patches

Co-authored-by: Shelley Vohr <[email protected]>
Co-authored-by: Electron Bot <[email protected]>
Co-authored-by: Cheng Zhao <[email protected]>
trop[bot] 4 years ago
parent
commit
becfe25528

+ 1 - 0
patches/chromium/.patches

@@ -99,3 +99,4 @@ skip_atk_toolchain_check.patch
 worker_feat_add_hook_to_notify_script_ready.patch
 cherry-pick-2f5b8357dca2.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 85ac9d5d95856470707d320d382743673cd1451a..398095d8890cee3f8838170a86ccb48010ff74a0 100644
+--- a/third_party/blink/renderer/bindings/core/v8/v8_initializer.cc
++++ b/third_party/blink/renderer/bindings/core/v8/v8_initializer.cc
+@@ -438,7 +438,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

@@ -46,3 +46,4 @@ fix_enable_tls_renegotiation.patch
 crypto_update_certdata_to_nss_3_56.patch
 n-api_src_provide_asynchronous_cleanup_hooks.patch
 fix_add_v8_enable_reverse_jsargs_defines_in_common_gypi.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 28851b8a8f8bdd6dec0f54c62f79fd75a3be08ed..e42416b4807fcc9d35a93399b916968b45ed0c7a 100644
+--- a/src/api/environment.cc
++++ b/src/api/environment.cc
+@@ -26,14 +26,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 &&
+@@ -42,7 +44,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);
+@@ -216,7 +219,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 ?
+@@ -224,7 +227,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);
+ }
+ 
+@@ -232,7 +235,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_SET_PROMISE_REJECTION_CALLBACK) {
+diff --git a/src/env.h b/src/env.h
+index 9420bdf3f71e2df1011ddd7e583071f5c99beac8..a5c72b6c145feedd624f7b6e407617ab295ad3bb 100644
+--- a/src/env.h
++++ b/src/env.h
+@@ -918,6 +918,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"
@@ -151,6 +152,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() {
@@ -441,6 +458,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;