From 580fa57a296c6cc49019ce40464b7dce3c8bd0f7 Mon Sep 17 00:00:00 2001 From: Calvin Date: Wed, 7 May 2025 19:21:39 -0600 Subject: [PATCH] refactor: Node.js temporary "explicit" microtask policy scope pattern (#46973) refactor: Node.js explicit microtask scope pattern --- shell/common/node_bindings.cc | 30 +++++++++------------- shell/common/node_util.cc | 10 ++++++++ shell/common/node_util.h | 23 +++++++++++++++++ shell/renderer/electron_renderer_client.cc | 20 +++++---------- shell/renderer/web_worker_observer.cc | 21 ++++++--------- 5 files changed, 60 insertions(+), 44 deletions(-) diff --git a/shell/common/node_bindings.cc b/shell/common/node_bindings.cc index 7ccfcd6ea9..28597c3f66 100644 --- a/shell/common/node_bindings.cc +++ b/shell/common/node_bindings.cc @@ -908,28 +908,22 @@ void NodeBindings::UvRunOnce() { // Enter node context while dealing with uv events. v8::Context::Scope context_scope(env->context()); - // Node.js expects `kExplicit` microtasks policy and will run microtasks - // checkpoints after every call into JavaScript. Since we use a different - // policy in the renderer - switch to `kExplicit` and then drop back to the - // previous policy value. - v8::MicrotaskQueue* microtask_queue = env->context()->GetMicrotaskQueue(); - auto old_policy = microtask_queue->microtasks_policy(); - DCHECK_EQ(microtask_queue->GetMicrotasksScopeDepth(), 0); - microtask_queue->set_microtasks_policy(v8::MicrotasksPolicy::kExplicit); + { + util::ExplicitMicrotasksScope microtasks_scope( + env->context()->GetMicrotaskQueue()); - if (browser_env_ != BrowserEnvironment::kBrowser) - TRACE_EVENT_BEGIN0("devtools.timeline", "FunctionCall"); + if (browser_env_ != BrowserEnvironment::kBrowser) + TRACE_EVENT_BEGIN0("devtools.timeline", "FunctionCall"); - // Deal with uv events. - int r = uv_run(uv_loop_, UV_RUN_NOWAIT); + // Deal with uv events. + int r = uv_run(uv_loop_, UV_RUN_NOWAIT); - if (browser_env_ != BrowserEnvironment::kBrowser) - TRACE_EVENT_END0("devtools.timeline", "FunctionCall"); + if (browser_env_ != BrowserEnvironment::kBrowser) + TRACE_EVENT_END0("devtools.timeline", "FunctionCall"); - microtask_queue->set_microtasks_policy(old_policy); - - if (r == 0) - base::RunLoop().QuitWhenIdle(); // Quit from uv. + if (r == 0) + base::RunLoop().QuitWhenIdle(); // Quit from uv. + } // Tell the worker thread to continue polling. uv_sem_post(&embed_sem_); diff --git a/shell/common/node_util.cc b/shell/common/node_util.cc index 89eaf88942..f777796339 100644 --- a/shell/common/node_util.cc +++ b/shell/common/node_util.cc @@ -130,6 +130,16 @@ node::Environment* CreateEnvironment(v8::Isolate* isolate, return env; } +ExplicitMicrotasksScope::ExplicitMicrotasksScope(v8::MicrotaskQueue* queue) + : microtask_queue_(queue), original_policy_(queue->microtasks_policy()) { + DCHECK_EQ(microtask_queue_->GetMicrotasksScopeDepth(), 0); + microtask_queue_->set_microtasks_policy(v8::MicrotasksPolicy::kExplicit); +} + +ExplicitMicrotasksScope::~ExplicitMicrotasksScope() { + microtask_queue_->set_microtasks_policy(original_policy_); +} + } // namespace electron::util namespace electron::Buffer { diff --git a/shell/common/node_util.h b/shell/common/node_util.h index fdc92c0f6b..b159fcf657 100644 --- a/shell/common/node_util.h +++ b/shell/common/node_util.h @@ -10,6 +10,8 @@ #include #include "base/containers/span.h" +#include "base/memory/raw_ptr.h" +#include "v8-microtask-queue.h" #include "v8/include/v8-forward.h" namespace node { @@ -63,6 +65,27 @@ node::Environment* CreateEnvironment(v8::Isolate* isolate, node::EnvironmentFlags::Flags env_flags, std::string_view process_type = ""); +// A scope that temporarily changes the microtask policy to explicit. Use this +// anywhere that can trigger Node.js or uv_run(). +// +// Node.js expects `kExplicit` microtasks policy and will run microtasks +// checkpoints after every call into JavaScript. Since we use a different +// policy in the renderer, this scope temporarily changes the policy to +// `kExplicit` while the scope is active, then restores the original policy +// when it's destroyed. +class ExplicitMicrotasksScope { + public: + explicit ExplicitMicrotasksScope(v8::MicrotaskQueue* queue); + ~ExplicitMicrotasksScope(); + + ExplicitMicrotasksScope(const ExplicitMicrotasksScope&) = delete; + ExplicitMicrotasksScope& operator=(const ExplicitMicrotasksScope&) = delete; + + private: + base::raw_ptr microtask_queue_; + v8::MicrotasksPolicy original_policy_; +}; + } // namespace electron::util namespace electron::Buffer { diff --git a/shell/renderer/electron_renderer_client.cc b/shell/renderer/electron_renderer_client.cc index c18b81d49d..5757c2c76c 100644 --- a/shell/renderer/electron_renderer_client.cc +++ b/shell/renderer/electron_renderer_client.cc @@ -16,6 +16,7 @@ #include "shell/common/gin_helper/event_emitter_caller.h" #include "shell/common/node_bindings.h" #include "shell/common/node_includes.h" +#include "shell/common/node_util.h" #include "shell/common/options_switches.h" #include "shell/renderer/electron_render_frame_observer.h" #include "shell/renderer/web_worker_observer.h" @@ -178,19 +179,12 @@ void ElectronRendererClient::WillReleaseScriptContext( if (env == node_bindings_->uv_env()) node_bindings_->set_uv_env(nullptr); - // Destroying the node environment will also run the uv loop, - // Node.js expects `kExplicit` microtasks policy and will run microtasks - // checkpoints after every call into JavaScript. Since we use a different - // policy in the renderer - switch to `kExplicit` and then drop back to the - // previous policy value. - v8::MicrotaskQueue* microtask_queue = context->GetMicrotaskQueue(); - auto old_policy = microtask_queue->microtasks_policy(); - DCHECK_EQ(microtask_queue->GetMicrotasksScopeDepth(), 0); - microtask_queue->set_microtasks_policy(v8::MicrotasksPolicy::kExplicit); - - environments_.erase(iter); - - microtask_queue->set_microtasks_policy(old_policy); + // Destroying the node environment will also run the uv loop. + { + util::ExplicitMicrotasksScope microtasks_scope( + context->GetMicrotaskQueue()); + environments_.erase(iter); + } // ElectronBindings is tracking node environments. electron_bindings_->EnvironmentDestroyed(env); diff --git a/shell/renderer/web_worker_observer.cc b/shell/renderer/web_worker_observer.cc index d1953998cb..ccdaf7911a 100644 --- a/shell/renderer/web_worker_observer.cc +++ b/shell/renderer/web_worker_observer.cc @@ -14,6 +14,7 @@ #include "shell/common/gin_helper/event_emitter_caller.h" #include "shell/common/node_bindings.h" #include "shell/common/node_includes.h" +#include "shell/common/node_util.h" namespace electron { @@ -112,19 +113,13 @@ void WebWorkerObserver::ContextWillDestroy(v8::Local context) { gin_helper::EmitEvent(env->isolate(), env->process_object(), "exit"); } - // Destroying the node environment will also run the uv loop, - // Node.js expects `kExplicit` microtasks policy and will run microtasks - // checkpoints after every call into JavaScript. Since we use a different - // policy in the renderer - switch to `kExplicit` - v8::MicrotaskQueue* microtask_queue = context->GetMicrotaskQueue(); - auto old_policy = microtask_queue->microtasks_policy(); - DCHECK_EQ(microtask_queue->GetMicrotasksScopeDepth(), 0); - microtask_queue->set_microtasks_policy(v8::MicrotasksPolicy::kExplicit); - - base::EraseIf(environments_, - [env](auto const& item) { return item.get() == env; }); - - microtask_queue->set_microtasks_policy(old_policy); + // Destroying the node environment will also run the uv loop. + { + util::ExplicitMicrotasksScope microtasks_scope( + context->GetMicrotaskQueue()); + base::EraseIf(environments_, + [env](auto const& item) { return item.get() == env; }); + } // ElectronBindings is tracking node environments. electron_bindings_->EnvironmentDestroyed(env);