From 042d25e926a268a869775bc16d08046172443359 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Thu, 8 Oct 2020 12:26:01 -0700 Subject: [PATCH] fix: wasm code generation in the renderer (#25777) --- patches/chromium/.patches | 1 + ..._v8_initialization_isolate_callbacks.patch | 37 ++++++++ patches/node/.patches | 1 + ..._v8_initialization_isolate_callbacks.patch | 89 +++++++++++++++++++ shell/common/node_bindings.cc | 21 +++++ 5 files changed, 149 insertions(+) create mode 100644 patches/chromium/chore_expose_v8_initialization_isolate_callbacks.patch create mode 100644 patches/node/chore_expose_v8_initialization_isolate_callbacks.patch diff --git a/patches/chromium/.patches b/patches/chromium/.patches index a6bc476524bd..ec53ff659342 100644 --- a/patches/chromium/.patches +++ b/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 diff --git a/patches/chromium/chore_expose_v8_initialization_isolate_callbacks.patch b/patches/chromium/chore_expose_v8_initialization_isolate_callbacks.patch new file mode 100644 index 000000000000..e96460ef5112 --- /dev/null +++ b/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 +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 context, + return {true, std::move(stringified_source)}; + } + +-static bool WasmCodeGenerationCheckCallbackInMainThread( ++bool V8Initializer::WasmCodeGenerationCheckCallbackInMainThread( + v8::Local context, + v8::Local 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); + static void MessageHandlerInWorker(v8::Local, + v8::Local); ++ static bool WasmCodeGenerationCheckCallbackInMainThread( ++ v8::Local context, ++ v8::Local source); + }; + + } // namespace blink diff --git a/patches/node/.patches b/patches/node/.patches index 0d95f20af681..c664cf442c21 100644 --- a/patches/node/.patches +++ b/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 diff --git a/patches/node/chore_expose_v8_initialization_isolate_callbacks.patch b/patches/node/chore_expose_v8_initialization_isolate_callbacks.patch new file mode 100644 index 000000000000..d7d30811f8f6 --- /dev/null +++ b/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 +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, ++// static ++bool Environment::AllowWasmCodeGenerationCallback(Local context, + Local) { + Local 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 PrepareStackTraceCallback(Local context, ++// static ++MaybeLocal Environment::PrepareStackTraceCallback(Local context, + Local exception, + Local 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 context, ++ v8::Local); ++ static bool ShouldAbortOnUncaughtException(v8::Isolate* isolate); ++ static v8::MaybeLocal PrepareStackTraceCallback(v8::Local context, ++ v8::Local exception, ++ v8::Local trace); ++ + // Register clean-up cb to be called on environment destruction. + inline void RegisterHandleCleanup(uv_handle_t* handle, + HandleCleanupCb cb, diff --git a/shell/common/node_bindings.cc b/shell/common/node_bindings.cc index 0b4a77c48178..35250c611486 100644 --- a/shell/common/node_bindings.cc +++ b/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 context, + v8::Local) { + // 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;