Skip to content

Commit 042d25e

Browse files
authored
fix: wasm code generation in the renderer (electron#25777)
1 parent 2ca2a88 commit 042d25e

5 files changed

+149
-0
lines changed

patches/chromium/.patches

+1
Original file line numberDiff line numberDiff line change
@@ -100,3 +100,4 @@ worker_feat_add_hook_to_notify_script_ready.patch
100100
fix_properly_honor_printing_page_ranges.patch
101101
remove_deprecated_factory_parameter_for_worker_resource_loader.patch
102102
fix_use_electron_generated_resources.patch
103+
chore_expose_v8_initialization_isolate_callbacks.patch
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
2+
From: Shelley Vohr <[email protected]>
3+
Date: Mon, 5 Oct 2020 13:43:59 -0700
4+
Subject: chore: expose v8 initialization isolate callbacks
5+
6+
This commit is necessary in order to ensure consistent behavior from
7+
v8 Isolate callbacks in contexts which Node.js does not control. If
8+
we're running with contextIsolation enabled, we should be falling back
9+
to Blink's logic. This will be upstreamed in some form.
10+
11+
diff --git a/third_party/blink/renderer/bindings/core/v8/v8_initializer.cc b/third_party/blink/renderer/bindings/core/v8/v8_initializer.cc
12+
index 7c2ae147254dad017214d2535203a18719294768..3313d6aff572307777f02801b09fc3d29318050e 100644
13+
--- a/third_party/blink/renderer/bindings/core/v8/v8_initializer.cc
14+
+++ b/third_party/blink/renderer/bindings/core/v8/v8_initializer.cc
15+
@@ -442,7 +442,7 @@ CodeGenerationCheckCallbackInMainThread(v8::Local<v8::Context> context,
16+
return {true, std::move(stringified_source)};
17+
}
18+
19+
-static bool WasmCodeGenerationCheckCallbackInMainThread(
20+
+bool V8Initializer::WasmCodeGenerationCheckCallbackInMainThread(
21+
v8::Local<v8::Context> context,
22+
v8::Local<v8::String> source) {
23+
if (ExecutionContext* execution_context = ToExecutionContext(context)) {
24+
diff --git a/third_party/blink/renderer/bindings/core/v8/v8_initializer.h b/third_party/blink/renderer/bindings/core/v8/v8_initializer.h
25+
index e7cbc5db7d15aa0fcfb37ba261673b973827296a..6b93aa449a005e06862a99ea0c9b751ffff2d6ec 100644
26+
--- a/third_party/blink/renderer/bindings/core/v8/v8_initializer.h
27+
+++ b/third_party/blink/renderer/bindings/core/v8/v8_initializer.h
28+
@@ -67,6 +67,9 @@ class CORE_EXPORT V8Initializer {
29+
v8::Local<v8::Value>);
30+
static void MessageHandlerInWorker(v8::Local<v8::Message>,
31+
v8::Local<v8::Value>);
32+
+ static bool WasmCodeGenerationCheckCallbackInMainThread(
33+
+ v8::Local<v8::Context> context,
34+
+ v8::Local<v8::String> source);
35+
};
36+
37+
} // namespace blink

patches/node/.patches

+1
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,4 @@ crypto_update_certdata_to_nss_3_56.patch
2929
fix_-wincompatible-pointer-types-discards-qualifiers_error.patch
3030
fix_add_v8_enable_reverse_jsargs_defines_in_common_gypi.patch
3131
fix_allow_preventing_initializeinspector_in_env.patch
32+
chore_expose_v8_initialization_isolate_callbacks.patch
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
2+
From: Shelley Vohr <[email protected]>
3+
Date: Mon, 5 Oct 2020 16:05:45 -0700
4+
Subject: chore: expose v8 initialization isolate callbacks
5+
6+
Exposes v8 initializer callbacks to Electron so that we can call them
7+
directly. We expand upon and adapt their behavior, so allows us to
8+
ensure that we stay in sync with Node.js default behavior.
9+
10+
This will be upstreamed.
11+
12+
diff --git a/src/api/environment.cc b/src/api/environment.cc
13+
index cf6115d04ba3c184937c5db85c9d7ebc78ed3db7..a8ee97729858b40179e6bce58cfbacf1a6980340 100644
14+
--- a/src/api/environment.cc
15+
+++ b/src/api/environment.cc
16+
@@ -30,14 +30,16 @@ using v8::PropertyDescriptor;
17+
using v8::String;
18+
using v8::Value;
19+
20+
-static bool AllowWasmCodeGenerationCallback(Local<Context> context,
21+
+// static
22+
+bool Environment::AllowWasmCodeGenerationCallback(Local<Context> context,
23+
Local<String>) {
24+
Local<Value> wasm_code_gen =
25+
context->GetEmbedderData(ContextEmbedderIndex::kAllowWasmCodeGeneration);
26+
return wasm_code_gen->IsUndefined() || wasm_code_gen->IsTrue();
27+
}
28+
29+
-static bool ShouldAbortOnUncaughtException(Isolate* isolate) {
30+
+// static
31+
+bool Environment::ShouldAbortOnUncaughtException(Isolate* isolate) {
32+
DebugSealHandleScope scope(isolate);
33+
Environment* env = Environment::GetCurrent(isolate);
34+
return env != nullptr &&
35+
@@ -47,7 +49,8 @@ static bool ShouldAbortOnUncaughtException(Isolate* isolate) {
36+
!env->inside_should_not_abort_on_uncaught_scope();
37+
}
38+
39+
-static MaybeLocal<Value> PrepareStackTraceCallback(Local<Context> context,
40+
+// static
41+
+MaybeLocal<Value> Environment::PrepareStackTraceCallback(Local<Context> context,
42+
Local<Value> exception,
43+
Local<Array> trace) {
44+
Environment* env = Environment::GetCurrent(context);
45+
@@ -221,7 +224,7 @@ void SetIsolateErrorHandlers(v8::Isolate* isolate, const IsolateSettings& s) {
46+
47+
auto* abort_callback = s.should_abort_on_uncaught_exception_callback ?
48+
s.should_abort_on_uncaught_exception_callback :
49+
- ShouldAbortOnUncaughtException;
50+
+ Environment::ShouldAbortOnUncaughtException;
51+
isolate->SetAbortOnUncaughtExceptionCallback(abort_callback);
52+
53+
auto* fatal_error_cb = s.fatal_error_callback ?
54+
@@ -229,7 +232,7 @@ void SetIsolateErrorHandlers(v8::Isolate* isolate, const IsolateSettings& s) {
55+
isolate->SetFatalErrorHandler(fatal_error_cb);
56+
57+
auto* prepare_stack_trace_cb = s.prepare_stack_trace_callback ?
58+
- s.prepare_stack_trace_callback : PrepareStackTraceCallback;
59+
+ s.prepare_stack_trace_callback : Environment::PrepareStackTraceCallback;
60+
isolate->SetPrepareStackTraceCallback(prepare_stack_trace_cb);
61+
}
62+
63+
@@ -237,7 +240,7 @@ void SetIsolateMiscHandlers(v8::Isolate* isolate, const IsolateSettings& s) {
64+
isolate->SetMicrotasksPolicy(s.policy);
65+
66+
auto* allow_wasm_codegen_cb = s.allow_wasm_code_generation_callback ?
67+
- s.allow_wasm_code_generation_callback : AllowWasmCodeGenerationCallback;
68+
+ s.allow_wasm_code_generation_callback : Environment::AllowWasmCodeGenerationCallback;
69+
isolate->SetAllowWasmCodeGenerationCallback(allow_wasm_codegen_cb);
70+
71+
if ((s.flags & SHOULD_NOT_SET_PROMISE_REJECTION_CALLBACK) == 0) {
72+
diff --git a/src/env.h b/src/env.h
73+
index 4fe2eb3b7699efcab87c377743a955effbbfd9de..2bb196d199bd9b6c27d9de8ca7e5a9bffad0c788 100644
74+
--- a/src/env.h
75+
+++ b/src/env.h
76+
@@ -904,6 +904,13 @@ class Environment : public MemoryRetainer {
77+
void Exit(int code);
78+
void ExitEnv();
79+
80+
+ static bool AllowWasmCodeGenerationCallback(v8::Local<v8::Context> context,
81+
+ v8::Local<v8::String>);
82+
+ static bool ShouldAbortOnUncaughtException(v8::Isolate* isolate);
83+
+ static v8::MaybeLocal<v8::Value> PrepareStackTraceCallback(v8::Local<v8::Context> context,
84+
+ v8::Local<v8::Value> exception,
85+
+ v8::Local<v8::Array> trace);
86+
+
87+
// Register clean-up cb to be called on environment destruction.
88+
inline void RegisterHandleCleanup(uv_handle_t* handle,
89+
HandleCleanupCb cb,

shell/common/node_bindings.cc

+21
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
#include "shell/common/gin_helper/microtasks_scope.h"
3434
#include "shell/common/mac/main_application_bundle.h"
3535
#include "shell/common/node_includes.h"
36+
#include "third_party/blink/renderer/bindings/core/v8/v8_initializer.h" // nogncheck
3637

3738
#if !defined(MAS_BUILD)
3839
#include "shell/common/crash_keys.h"
@@ -152,6 +153,22 @@ void V8FatalErrorCallback(const char* location, const char* message) {
152153
*zero = 0;
153154
}
154155

156+
bool AllowWasmCodeGenerationCallback(v8::Local<v8::Context> context,
157+
v8::Local<v8::String>) {
158+
// If we're running with contextIsolation enabled in the renderer process,
159+
// fall back to Blink's logic.
160+
v8::Isolate* isolate = context->GetIsolate();
161+
if (node::Environment::GetCurrent(isolate) == nullptr) {
162+
if (gin_helper::Locker::IsBrowserProcess())
163+
return false;
164+
return blink::V8Initializer::WasmCodeGenerationCheckCallbackInMainThread(
165+
context, v8::String::Empty(isolate));
166+
}
167+
168+
return node::Environment::AllowWasmCodeGenerationCallback(
169+
context, v8::String::Empty(isolate));
170+
}
171+
155172
// Initialize Node.js cli options to pass to Node.js
156173
// See https://nodejs.org/api/cli.html#cli_options
157174
void SetNodeCliFlags() {
@@ -436,6 +453,10 @@ node::Environment* NodeBindings::CreateEnvironment(
436453
return false;
437454
};
438455

456+
// Use a custom callback here to allow us to leverage Blink's logic in the
457+
// renderer process.
458+
is.allow_wasm_code_generation_callback = AllowWasmCodeGenerationCallback;
459+
439460
if (browser_env_ == BrowserEnvironment::BROWSER) {
440461
// Node.js requires that microtask checkpoints be explicitly invoked.
441462
is.policy = v8::MicrotasksPolicy::kExplicit;

0 commit comments

Comments
 (0)