From f15fa56e38e0189ae1bcc558c2048ce154c2266f Mon Sep 17 00:00:00 2001 From: Calvin Date: Sat, 19 Apr 2025 11:18:03 -0600 Subject: [PATCH] refactor: reduce & remove no-op MicrotasksScope calls (#46681) * fix: do not run microtasks in V8Serializer in browser process * Remove no-op MicrotasksScope in `shell/browser/api/electron_api_auto_updater.cc` This call was added in https://github.com/electron/electron/pull/40576 as an expansion of `gin_helper::EmitEvent`. Since this only runs in the browser process and `bool ignore_browser_checkpoint = true` this code is a no-op. Node should perform a microtask checkpoint if necessary in `node::MakeCallback`. * Remove no-op MicrotasksScope in `shell/common/api/electron_bindings.cc` This method is only called by the browser process. The containing function, `ElectronBindings::DidReceiveMemoryDump`, is only used in two places: * `ElectronBindings::GetProcessMemoryInfo` in the same file, which has a `CHECK` that it's running in the browser process at the top. * From `shell/browser/api/electron_api_web_contents.cc`, which is only run in the browser process. Added a DCHECK for clarity and validation. * Replace `gin_helper::MicrotasksScope` with `v8::MicrotasksScope` in `shell/renderer/` The browser check is unnecessary in the renderer. Since `gin_helper::MicrotasksScope` will always act exactly like `v8::MicrotasksScope`, it's clear to just use the v8 object directly. This also brings them in line with the many other uses of `v8::MicrotasksScope` in `shell/renderer/`. --- shell/browser/api/electron_api_auto_updater.cc | 4 ---- shell/common/api/electron_bindings.cc | 3 +-- shell/renderer/api/electron_api_spell_check_client.cc | 4 ++-- shell/renderer/electron_sandboxed_renderer_client.cc | 8 ++++---- 4 files changed, 7 insertions(+), 12 deletions(-) diff --git a/shell/browser/api/electron_api_auto_updater.cc b/shell/browser/api/electron_api_auto_updater.cc index 2a1feaa9d837..4111342ec1f9 100644 --- a/shell/browser/api/electron_api_auto_updater.cc +++ b/shell/browser/api/electron_api_auto_updater.cc @@ -46,10 +46,6 @@ void AutoUpdater::OnError(const std::string& message) { gin::StringToV8(isolate, message), }; - gin_helper::MicrotasksScope microtasks_scope{ - wrapper->GetCreationContextChecked(), true, - v8::MicrotasksScope::kRunMicrotasks}; - node::MakeCallback(isolate, wrapper, "emit", args.size(), args.data(), {0, 0}); } diff --git a/shell/common/api/electron_bindings.cc b/shell/common/api/electron_bindings.cc index a2bc082ed0e0..dc8ba3709ed9 100644 --- a/shell/common/api/electron_bindings.cc +++ b/shell/common/api/electron_bindings.cc @@ -235,12 +235,11 @@ void ElectronBindings::DidReceiveMemoryDump( base::ProcessId target_pid, bool success, std::unique_ptr global_dump) { + DCHECK(electron::IsBrowserProcess()); v8::Isolate* isolate = promise.isolate(); v8::HandleScope handle_scope(isolate); v8::Local local_context = v8::Local::New(isolate, context); - gin_helper::MicrotasksScope microtasks_scope{ - local_context, true, v8::MicrotasksScope::kRunMicrotasks}; v8::Context::Scope context_scope(local_context); if (!success) { diff --git a/shell/renderer/api/electron_api_spell_check_client.cc b/shell/renderer/api/electron_api_spell_check_client.cc index 46b8a03163a6..58ac2da5e028 100644 --- a/shell/renderer/api/electron_api_spell_check_client.cc +++ b/shell/renderer/api/electron_api_spell_check_client.cc @@ -215,8 +215,8 @@ void SpellCheckClient::SpellCheckWords(const SpellCheckScope& scope, DCHECK(!scope.spell_check_.IsEmpty()); auto context = isolate_->GetCurrentContext(); - gin_helper::MicrotasksScope microtasks_scope{ - context, false, v8::MicrotasksScope::kDoNotRunMicrotasks}; + v8::MicrotasksScope microtasks_scope( + context, v8::MicrotasksScope::kDoNotRunMicrotasks); v8::Local templ = gin_helper::CreateFunctionTemplate( isolate_, base::BindRepeating(&SpellCheckClient::OnSpellCheckDone, diff --git a/shell/renderer/electron_sandboxed_renderer_client.cc b/shell/renderer/electron_sandboxed_renderer_client.cc index 2c985c9a59c0..2f8511a3a70d 100644 --- a/shell/renderer/electron_sandboxed_renderer_client.cc +++ b/shell/renderer/electron_sandboxed_renderer_client.cc @@ -146,8 +146,8 @@ void ElectronSandboxedRendererClient::WillReleaseScriptContext( return; auto* isolate = context->GetIsolate(); - gin_helper::MicrotasksScope microtasks_scope{ - context, false, v8::MicrotasksScope::kDoNotRunMicrotasks}; + v8::MicrotasksScope microtasks_scope( + context, v8::MicrotasksScope::kDoNotRunMicrotasks); v8::HandleScope handle_scope(isolate); v8::Context::Scope context_scope(context); InvokeEmitProcessEvent(context, "exit"); @@ -164,8 +164,8 @@ void ElectronSandboxedRendererClient::EmitProcessEvent( v8::HandleScope handle_scope{isolate}; v8::Local context = GetContext(frame, isolate); - gin_helper::MicrotasksScope microtasks_scope{ - context, false, v8::MicrotasksScope::kDoNotRunMicrotasks}; + v8::MicrotasksScope microtasks_scope( + context, v8::MicrotasksScope::kDoNotRunMicrotasks); v8::Context::Scope context_scope(context); InvokeEmitProcessEvent(context, event_name);