From 084c6ef549742e15375c711a9aabc20a59d970e2 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Fri, 25 Jul 2025 09:06:57 -0500 Subject: [PATCH] refactor: prefer `GetCreationContextChecked(v8::Isolate*)` over `GetCreationContextChecked()` (#47878) * refactor: pass an isolate when calling GetCreationContextChecked() in V8FunctionInvoker * refactor: pass an isolate when calling GetCreationContextChecked() in RendererClientBase * refactor: pass an isolate when calling GetCreationContextChecked() in ScriptExecutionCallback::Completed() * refactor: pass an isolate when calling GetCreationContextChecked() in ScriptExecutionCallback::CopyResultToCallingContextAndFinalize() * refactor: pass an isolate when calling GetCreationContextChecked() in electron::GetRenderFrame() * refactor: pass an isolate when calling GetCreationContextChecked() in gin_helper::internal::CallMethodWithArgs() * refactor: pass an isolate when calling GetCreationContextChecked() in OverrideGlobalPropertyFromIsolatedWorld() * refactor: pass an isolate when calling GetCreationContextChecked() in OverrideGlobalValueFromIsolatedWorld() * refactor: pass an isolate when calling GetCreationContextChecked() in ProxyFunctionWrapper() * refactor: pass an isolate when calling GetCreationContextChecked() in PassValueToOtherContextInner() * fixup! refactor: pass an isolate when calling GetCreationContextChecked() in electron::GetRenderFrame() --- shell/common/gin_helper/callback.h | 6 +-- .../common/gin_helper/event_emitter_caller.cc | 2 +- .../api/electron_api_context_bridge.cc | 41 +++++++++++-------- shell/renderer/api/electron_api_web_frame.cc | 19 +++++---- shell/renderer/renderer_client_base.cc | 11 ++--- 5 files changed, 44 insertions(+), 35 deletions(-) diff --git a/shell/common/gin_helper/callback.h b/shell/common/gin_helper/callback.h index b8e0094baa6f..b16c431f34e4 100644 --- a/shell/common/gin_helper/callback.h +++ b/shell/common/gin_helper/callback.h @@ -49,7 +49,7 @@ struct V8FunctionInvoker(ArgTypes...)> { if (!function.IsAlive()) return v8::Null(isolate); v8::Local holder = function.NewHandle(isolate); - v8::Local context = holder->GetCreationContextChecked(); + v8::Local context = holder->GetCreationContextChecked(isolate); v8::MicrotasksScope microtasks_scope(context, v8::MicrotasksScope::kRunMicrotasks); v8::Context::Scope context_scope(context); @@ -74,7 +74,7 @@ struct V8FunctionInvoker { if (!function.IsAlive()) return; v8::Local holder = function.NewHandle(isolate); - v8::Local context = holder->GetCreationContextChecked(); + v8::Local context = holder->GetCreationContextChecked(isolate); v8::MicrotasksScope microtasks_scope(context, v8::MicrotasksScope::kRunMicrotasks); v8::Context::Scope context_scope(context); @@ -98,7 +98,7 @@ struct V8FunctionInvoker { if (!function.IsAlive()) return ret; v8::Local holder = function.NewHandle(isolate); - v8::Local context = holder->GetCreationContextChecked(); + v8::Local context = holder->GetCreationContextChecked(isolate); v8::MicrotasksScope microtasks_scope(context, v8::MicrotasksScope::kRunMicrotasks); v8::Context::Scope context_scope(context); diff --git a/shell/common/gin_helper/event_emitter_caller.cc b/shell/common/gin_helper/event_emitter_caller.cc index eb55adfc271c..d21a3fb77a6f 100644 --- a/shell/common/gin_helper/event_emitter_caller.cc +++ b/shell/common/gin_helper/event_emitter_caller.cc @@ -23,7 +23,7 @@ v8::Local CallMethodWithArgs( node::async_context{0, 0}}; // Perform microtask checkpoint after running JavaScript. - v8::MicrotasksScope microtasks_scope(obj->GetCreationContextChecked(), + v8::MicrotasksScope microtasks_scope(obj->GetCreationContextChecked(isolate), v8::MicrotasksScope::kRunMicrotasks); // node::MakeCallback will also run pending tasks in Node.js. diff --git a/shell/renderer/api/electron_api_context_bridge.cc b/shell/renderer/api/electron_api_context_bridge.cc index 27184f28639b..030836ee9320 100644 --- a/shell/renderer/api/electron_api_context_bridge.cc +++ b/shell/renderer/api/electron_api_context_bridge.cc @@ -38,7 +38,8 @@ BASE_FEATURE(kContextBridgeMutability, namespace electron { -content::RenderFrame* GetRenderFrame(v8::Local value); +content::RenderFrame* GetRenderFrame(v8::Isolate* const isolate, + v8::Local value); namespace api { @@ -157,13 +158,15 @@ v8::MaybeLocal PassValueToOtherContextInner( bool support_dynamic_properties, int recursion_depth, BridgeErrorTarget error_target) { + v8::Isolate* const source_isolate = source_context->GetIsolate(); + TRACE_EVENT0("electron", "ContextBridge::PassValueToOtherContextInner"); if (recursion_depth >= kMaxRecursion) { v8::Context::Scope error_scope(error_target == BridgeErrorTarget::kSource ? source_context : destination_context); - source_context->GetIsolate()->ThrowException(v8::Exception::TypeError( - gin::StringToV8(source_context->GetIsolate(), + source_isolate->ThrowException(v8::Exception::TypeError( + gin::StringToV8(source_isolate, "Electron contextBridge recursion depth exceeded. " "Nested objects " "deeper than 1000 are not supported."))); @@ -205,8 +208,8 @@ v8::MaybeLocal PassValueToOtherContextInner( // creation context of the original method. If it's not we proceed // with the proxy logic if (maybe_original_fn.ToLocal(&proxy_func) && proxy_func->IsFunction() && - proxy_func.As()->GetCreationContextChecked() == - destination_context) { + proxy_func.As()->GetCreationContextChecked( + source_isolate) == destination_context) { return v8::MaybeLocal(proxy_func); } @@ -243,8 +246,8 @@ v8::MaybeLocal PassValueToOtherContextInner( v8::Local proxied_promise_handle = proxied_promise->GetHandle(); - v8::Global global_then_source_context( - source_context->GetIsolate(), source_context); + v8::Global global_then_source_context(source_isolate, + source_context); v8::Global global_then_destination_context( destination_context->GetIsolate(), destination_context); global_then_source_context.SetWeak(); @@ -290,8 +293,8 @@ v8::MaybeLocal PassValueToOtherContextInner( std::move(global_then_source_context), std::move(global_then_destination_context)); - v8::Global global_catch_source_context( - source_context->GetIsolate(), source_context); + v8::Global global_catch_source_context(source_isolate, + source_context); v8::Global global_catch_destination_context( destination_context->GetIsolate(), destination_context); global_catch_source_context.SetWeak(); @@ -356,8 +359,7 @@ v8::MaybeLocal PassValueToOtherContextInner( // crosses the bridge we fallback to the v8::Message approach if we can't // pull "message" for some reason v8::MaybeLocal maybe_message = value.As()->Get( - source_context, - gin::ConvertToV8(source_context->GetIsolate(), "message")); + source_context, gin::ConvertToV8(source_isolate, "message")); v8::Local message; if (maybe_message.ToLocal(&message) && message->IsString()) { return v8::MaybeLocal( @@ -496,7 +498,7 @@ void ProxyFunctionWrapper(const v8::FunctionCallbackInfo& info) { v8::Local func = func_value.As(); v8::Local func_owning_context = - func->GetCreationContextChecked(); + func->GetCreationContextChecked(args.isolate()); { v8::Context::Scope func_owning_context_scope(func_owning_context); @@ -760,7 +762,7 @@ v8::MaybeLocal GetTargetContext(v8::Isolate* isolate, blink::ExecutionContext* execution_context = blink::ExecutionContext::From(source_context); if (execution_context->IsWindow()) { - auto* render_frame = GetRenderFrame(source_context->Global()); + auto* render_frame = GetRenderFrame(isolate, source_context->Global()); CHECK(render_frame); auto* frame = render_frame->GetWebFrame(); CHECK(frame); @@ -812,13 +814,14 @@ gin_helper::Dictionary TraceKeyPath(const gin_helper::Dictionary& start, } void OverrideGlobalValueFromIsolatedWorld( + v8::Isolate* isolate, const std::vector& key_path, v8::Local value, bool support_dynamic_properties) { if (key_path.empty()) return; - auto* render_frame = GetRenderFrame(value); + auto* render_frame = GetRenderFrame(isolate, value); CHECK(render_frame); auto* frame = render_frame->GetWebFrame(); CHECK(frame); @@ -831,7 +834,8 @@ void OverrideGlobalValueFromIsolatedWorld( { v8::Context::Scope main_context_scope(main_context); - v8::Local source_context = value->GetCreationContextChecked(); + v8::Local source_context = + value->GetCreationContextChecked(isolate); v8::MaybeLocal maybe_proxy = PassValueToOtherContext( source_context, main_context, value, source_context->Global(), support_dynamic_properties, BridgeErrorTarget::kSource); @@ -843,6 +847,7 @@ void OverrideGlobalValueFromIsolatedWorld( } bool OverrideGlobalPropertyFromIsolatedWorld( + v8::Isolate* const isolate, const std::vector& key_path, v8::Local getter, v8::Local setter, @@ -850,7 +855,7 @@ bool OverrideGlobalPropertyFromIsolatedWorld( if (key_path.empty()) return false; - auto* render_frame = GetRenderFrame(getter); + auto* render_frame = GetRenderFrame(isolate, getter); CHECK(render_frame); auto* frame = render_frame->GetWebFrame(); CHECK(frame); @@ -869,7 +874,7 @@ bool OverrideGlobalPropertyFromIsolatedWorld( v8::Local setter_proxy; if (!getter->IsNullOrUndefined()) { v8::Local source_context = - getter->GetCreationContextChecked(); + getter->GetCreationContextChecked(isolate); v8::MaybeLocal maybe_getter_proxy = PassValueToOtherContext( source_context, main_context, getter, source_context->Global(), false, BridgeErrorTarget::kSource); @@ -878,7 +883,7 @@ bool OverrideGlobalPropertyFromIsolatedWorld( } if (!setter->IsNullOrUndefined() && setter->IsObject()) { v8::Local source_context = - getter->GetCreationContextChecked(); + getter->GetCreationContextChecked(isolate); v8::MaybeLocal maybe_setter_proxy = PassValueToOtherContext( source_context, main_context, setter, source_context->Global(), false, BridgeErrorTarget::kSource); diff --git a/shell/renderer/api/electron_api_web_frame.cc b/shell/renderer/api/electron_api_web_frame.cc index 575e8fccfc21..476c4fa88738 100644 --- a/shell/renderer/api/electron_api_web_frame.cc +++ b/shell/renderer/api/electron_api_web_frame.cc @@ -90,8 +90,9 @@ struct Converter { namespace electron { -content::RenderFrame* GetRenderFrame(v8::Local value) { - v8::Local context = value->GetCreationContextChecked(); +content::RenderFrame* GetRenderFrame(v8::Isolate* const isolate, + v8::Local value) { + v8::Local context = value->GetCreationContextChecked(isolate); if (context.IsEmpty()) return nullptr; blink::WebLocalFrame* frame = blink::WebLocalFrame::FrameForContext(context); @@ -146,7 +147,7 @@ class ScriptExecutionCallback { ScriptExecutionCallback& operator=(const ScriptExecutionCallback&) = delete; void CopyResultToCallingContextAndFinalize( - v8::Isolate* isolate, + v8::Isolate* const isolate, const v8::Local& result) { v8::MaybeLocal maybe_result; bool success = true; @@ -155,7 +156,7 @@ class ScriptExecutionCallback { { v8::TryCatch try_catch(isolate); v8::Local source_context = - result->GetCreationContextChecked(); + result->GetCreationContextChecked(isolate); maybe_result = PassValueToOtherContext( source_context, promise_.GetContext(), result, source_context->Global(), false, BridgeErrorTarget::kSource); @@ -201,7 +202,7 @@ class ScriptExecutionCallback { bool should_clone_value = !(value->IsObject() && promise_.GetContext() == - value.As()->GetCreationContextChecked()) && + value.As()->GetCreationContextChecked(isolate)) && value->IsObject(); if (should_clone_value) { CopyResultToCallingContextAndFinalize(isolate, @@ -839,7 +840,8 @@ class WebFrameRenderer final // Get the WebLocalFrame before (possibly) executing any user-space JS while // getting the |params|. We track the status of the RenderFrame via an // observer in case it is deleted during user code execution. - content::RenderFrame* render_frame = GetRenderFrame(content_window); + content::RenderFrame* render_frame = + GetRenderFrame(isolate, content_window); if (!render_frame) return v8::Null(isolate); @@ -965,8 +967,9 @@ void Initialize(v8::Local exports, v8::Isolate* const isolate = v8::Isolate::GetCurrent(); gin_helper::Dictionary dict{isolate, exports}; - dict.Set("mainFrame", WebFrameRenderer::Create( - isolate, electron::GetRenderFrame(exports))); + dict.Set("mainFrame", + WebFrameRenderer::Create( + isolate, electron::GetRenderFrame(isolate, exports))); } } // namespace diff --git a/shell/renderer/renderer_client_base.cc b/shell/renderer/renderer_client_base.cc index cc971f7b65a5..d82e4f125bfb 100644 --- a/shell/renderer/renderer_client_base.cc +++ b/shell/renderer/renderer_client_base.cc @@ -102,7 +102,8 @@ namespace electron { -content::RenderFrame* GetRenderFrame(v8::Local value); +content::RenderFrame* GetRenderFrame(v8::Isolate* const isolate, + v8::Local value); namespace { @@ -613,16 +614,16 @@ void RendererClientBase::AllowGuestViewElementDefinition( v8::Local context, v8::Local register_cb) { v8::HandleScope handle_scope(isolate); - v8::Context::Scope context_scope(context->GetCreationContextChecked()); + v8::Context::Scope context_scope(context->GetCreationContextChecked(isolate)); blink::WebCustomElement::EmbedderNamesAllowedScope embedder_names_scope; - content::RenderFrame* render_frame = GetRenderFrame(context); + content::RenderFrame* render_frame = GetRenderFrame(isolate, context); if (!render_frame) return; render_frame->GetWebFrame()->RequestExecuteV8Function( - context->GetCreationContextChecked(), register_cb, v8::Null(isolate), 0, - nullptr, base::NullCallback()); + context->GetCreationContextChecked(isolate), register_cb, + v8::Null(isolate), 0, nullptr, base::NullCallback()); } } // namespace electron