Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: wasm code generation in the renderer #26063

Merged
merged 1 commit into from Oct 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions patches/chromium/.patches
Expand Up @@ -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
@@ -0,0 +1,37 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Shelley Vohr <shelley.vohr@gmail.com>
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 change: 1 addition & 0 deletions patches/node/.patches
Expand Up @@ -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
@@ -0,0 +1,89 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Shelley Vohr <shelley.vohr@gmail.com>
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,
16 changes: 13 additions & 3 deletions shell/common/node_bindings.cc
Expand Up @@ -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) \
Expand Down Expand Up @@ -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
Expand Down