Browse Source

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

Shelley Vohr 4 years ago
parent
commit
8a052c5537

+ 1 - 0
patches/chromium/.patches

@@ -104,3 +104,4 @@ cherry-pick-8629cd7f8af3.patch
 avoid_use-after-free.patch
 don_t_create_providers_if_context_is_lost.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 21504ce01403d20067c8439c0c61ee0d71de84a5..13855078e4e9531304d30ec46cd2bb79798623ad 100644
+--- a/third_party/blink/renderer/bindings/core/v8/v8_initializer.cc
++++ b/third_party/blink/renderer/bindings/core/v8/v8_initializer.cc
+@@ -452,7 +452,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

@@ -49,3 +49,4 @@ lib_use_non-symbols_in_isurlinstance_check.patch
 fix_enable_tls_renegotiation.patch
 crypto_update_certdata_to_nss_3_56.patch
 n-api_src_provide_asynchronous_cleanup_hooks.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 7b370579d365204b2bd40a25e740bbc83726c376..be58db8d2bebd1e1e5a0e6eb35c09b459d5d56e6 100644
+--- a/src/api/environment.cc
++++ b/src/api/environment.cc
+@@ -25,14 +25,16 @@ using v8::Private;
+ 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 &&
+@@ -41,7 +43,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);
+@@ -195,7 +198,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 ?
+@@ -203,7 +206,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);
+ }
+ 
+@@ -211,7 +214,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);
+ 
+   auto* promise_reject_cb = s.promise_reject_callback ?
+diff --git a/src/env.h b/src/env.h
+index e269c47ae3814b42fdd2792360c1acb1995e98d2..e251eb1bb478b61b6b5a679f311c00d4310ff5ce 100644
+--- a/src/env.h
++++ b/src/env.h
+@@ -908,6 +908,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,

+ 13 - 3
shell/common/node_bindings.cc

@@ -32,6 +32,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
 
 #define ELECTRON_BUILTIN_MODULES(V)      \
   V(electron_browser_app)                \
@@ -225,9 +226,18 @@ void SetNodeOptions(base::Environment* env) {
 
 bool AllowWasmCodeGenerationCallback(v8::Local<v8::Context> context,
                                      v8::Local<v8::String>) {
-  v8::Local<v8::Value> wasm_code_gen = context->GetEmbedderData(
-      node::ContextEmbedderIndex::kAllowWasmCodeGeneration);
-  return wasm_code_gen->IsUndefined() || wasm_code_gen->IsTrue();
+  // 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));
 }
 
 }  // namespace