Skip to content

Commit

Permalink
fix: isolate callbacks in node_bindings (#23261)
Browse files Browse the repository at this point in the history
* fix: isolate callbacks in node_bindings

* clarify microtask policy comment
  • Loading branch information
codebytere authored Apr 27, 2020
1 parent e5fe81a commit 979c291
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 5 deletions.
2 changes: 1 addition & 1 deletion shell/app/node_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ int NodeMain(int argc, char* argv[]) {
exec_argc, exec_argv);
CHECK_NE(nullptr, env);

// TODO(codebytere): we shouldn't have to call this - upstream?
// This needs to be called before the inspector is initialized.
env->InitializeDiagnostics();

// This is needed in order to enable v8 host weakref hooks.
Expand Down
44 changes: 40 additions & 4 deletions shell/common/node_bindings.cc
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,23 @@ void SetNodeOptions(base::Environment* env) {
}
}

void HostCleanupFinalizationGroupCallback(
v8::Local<v8::Context> context,
v8::Local<v8::FinalizationGroup> group) {
node::Environment* env = node::Environment::GetCurrent(context);
if (env == nullptr) {
return;
}
env->RegisterFinalizationGroupForCleanup(group);
}

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();
}

} // namespace

namespace electron {
Expand Down Expand Up @@ -394,15 +411,34 @@ node::Environment* NodeBindings::CreateEnvironment(
}

if (browser_env_ == BrowserEnvironment::BROWSER) {
// SetAutorunMicrotasks is no longer called in node::CreateEnvironment
// so instead call it here to match expected node behavior
// This policy requires that microtask checkpoints be explicitly invoked.
// Node.js requires this.
context->GetIsolate()->SetMicrotasksPolicy(v8::MicrotasksPolicy::kExplicit);
} else {
// Node uses the deprecated SetAutorunMicrotasks(false) mode, we should
// switch to use the scoped policy to match blink's behavior.
// Match Blink's behavior by allowing microtasks invocation to be controlled
// by MicrotasksScope objects.
context->GetIsolate()->SetMicrotasksPolicy(v8::MicrotasksPolicy::kScoped);
}

// This needs to be called before the inspector is initialized.
env->InitializeDiagnostics();

// Ensure that WeakRefs work properly by specifying the callback to be called
// when FinalizationRegistries are ready to be cleaned up and require
// FinalizationGroup::Cleanup() to be called in a future task.
context->GetIsolate()->SetHostCleanupFinalizationGroupCallback(
HostCleanupFinalizationGroupCallback);

// Set the callback to invoke to check if wasm code generation should be
// allowed.
context->GetIsolate()->SetAllowWasmCodeGenerationCallback(
AllowWasmCodeGenerationCallback);

// Generate more detailed source positions to code objects. This results in
// better results when mapping profiling samples to script source.
v8::CpuProfiler::UseDetailedSourcePositionsForProfiling(
context->GetIsolate());

gin_helper::Dictionary process(context->GetIsolate(), env->process_object());
process.SetReadOnly("type", process_type);
process.Set("resourcesPath", resources_path);
Expand Down

0 comments on commit 979c291

Please sign in to comment.