refactor: avoid deprecated v8::Context::GetIsolate() calls pt 3 context get isolate pt 3 (#47910)

* refactor: add a v8::Isolate* arg to RendererClientBase::IsWebViewFrame()

Needed for creating gin dictionaries

refactor: add a v8::Isolate* arg to ShouldLoadPreload()

Needed for calling IsWebViewFrame()

Co-authored-by: Charles Kerr <charles@charleskerr.com>

* refactor: add a v8::Isolate* arg to electron::util::CompileAndCall()

Co-authored-by: Charles Kerr <charles@charleskerr.com>

* refactor: add a v8::Isolate* arg to OnCreatePreloadableV8Context()

Co-authored-by: Charles Kerr <charles@charleskerr.com>

* refactor: add a v8::Isolate* arg to InvokeEmitProcessEvent()

Co-authored-by: Charles Kerr <charles@charleskerr.com>

* refactor: add a v8::Isolate* arg to ServiceWorkerData's constructor

Co-authored-by: Charles Kerr <charles@charleskerr.com>

* refactor: add a v8::Isolate* arg to RendererClientBase::SetupMainWorldOverrides()

Co-authored-by: Charles Kerr <charles@charleskerr.com>

* refactor: add a v8::Isolate* arg to RendererClientBase::WilLReleaseScriptContext()

Co-authored-by: Charles Kerr <charles@charleskerr.com>

* docs: update docs to avoid v8::Context::GetIsolate()

Co-authored-by: Charles Kerr <charles@charleskerr.com>

* refactor: add a v8::Isolate* arg to ElectronSandboxedRendererClient::InitializeBindings()

Co-authored-by: Charles Kerr <charles@charleskerr.com>

* refactor: avoid v8::Context::GetIsolate() call in PromiseBase::SettleScope::~SettleScope()

Co-authored-by: Charles Kerr <charles@charleskerr.com>

---------

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Charles Kerr <charles@charleskerr.com>

docs/development/creating-api.md

@@ -117,7 +117,7 @@ void Initialize(v8::Local<v8::Object> exports,
                 v8::Local<v8::Value> unused,
                 v8::Local<v8::Context> context,
                 void* priv) {
-  v8::Isolate* isolate = context->GetIsolate();
+  v8::Isolate* const isolate = v8::Isolate::GetCurrent();
   gin_helper::Dictionary dict(isolate, exports);
   dict.Set("apiName", electron::api::ApiName::Create(isolate));
 }

shell/common/gin_helper/promise.cc

@@ -14,15 +14,15 @@
 namespace gin_helper {
 
 PromiseBase::SettleScope::SettleScope(const PromiseBase& base)
-    : handle_scope_{base.isolate()},
+    : isolate_{base.isolate()},
+      handle_scope_{isolate_},
       context_{base.GetContext()},
       microtasks_scope_(context_, v8::MicrotasksScope::kRunMicrotasks),
       context_scope_{context_} {}
 
 PromiseBase::SettleScope::~SettleScope() {
-  if (electron::IsBrowserProcess()) {
+  if (electron::IsBrowserProcess())
-    context_->GetMicrotaskQueue()->PerformCheckpoint(context_->GetIsolate());
+    context_->GetMicrotaskQueue()->PerformCheckpoint(isolate_);
-  }
 }
 
 PromiseBase::PromiseBase(v8::Isolate* isolate)

shell/common/gin_helper/promise.h

@@ -59,6 +59,7 @@ class PromiseBase {
     explicit SettleScope(const PromiseBase& base);
     ~SettleScope();
 
+    const raw_ptr<v8::Isolate> isolate_;
     v8::HandleScope handle_scope_;
     v8::Local<v8::Context> context_;
     v8::MicrotasksScope microtasks_scope_;

shell/common/node_bindings.cc

@@ -1007,8 +1007,9 @@ void OnNodePreload(node::Environment* env,
       env->isolate(), {node::FIXED_ONE_BYTE_STRING(env->isolate(), "process"),
                        node::FIXED_ONE_BYTE_STRING(env->isolate(), "require")});
   v8::LocalVector<v8::Value> bundle_args(env->isolate(), {process, require});
-  electron::util::CompileAndCall(env->context(), "electron/js2c/node_init",
+  electron::util::CompileAndCall(env->isolate(), env->context(),
-                                 &bundle_params, &bundle_args);
+                                 "electron/js2c/node_init", &bundle_params,
+                                 &bundle_args);
 }
 
 }  // namespace electron

shell/common/node_util.cc

@@ -21,12 +21,12 @@
 namespace electron::util {
 
 v8::MaybeLocal<v8::Value> CompileAndCall(
+    v8::Isolate* const isolate,
     v8::Local<v8::Context> context,
     const char* id,
     v8::LocalVector<v8::String>* parameters,
     v8::LocalVector<v8::Value>* arguments) {
-  v8::Isolate* isolate = context->GetIsolate();
+  v8::TryCatch try_catch{isolate};
-  v8::TryCatch try_catch(isolate);
 
   thread_local node::builtins::BuiltinLoader builtin_loader;
   v8::MaybeLocal<v8::Function> compiled = builtin_loader.LookupAndCompile(

shell/common/node_util.h

@@ -51,6 +51,7 @@ void EmitDeprecationWarning(std::string_view warning_msg,
 // JS code run with this method can assume that their top-level
 // declarations won't affect the global scope.
 v8::MaybeLocal<v8::Value> CompileAndCall(
+    v8::Isolate* isolate,
     v8::Local<v8::Context> context,
     const char* id,
     v8::LocalVector<v8::String>* parameters,

shell/renderer/electron_render_frame_observer.cc

@@ -122,9 +122,9 @@ void ElectronRenderFrameObserver::DidInstallConditionalFeatures(
   v8::MicrotasksScope microtasks_scope(
       context, v8::MicrotasksScope::kDoNotRunMicrotasks);
 
+  v8::Isolate* const isolate = v8::Isolate::GetCurrent();
   if (ShouldNotifyClient(world_id))
-    renderer_client_->DidCreateScriptContext(v8::Isolate::GetCurrent(), context,
+    renderer_client_->DidCreateScriptContext(isolate, context, render_frame_);
-                                             render_frame_);
 
   auto prefs = render_frame_->GetBlinkPreferences();
   bool use_context_isolation = prefs.context_isolation;
@@ -141,8 +141,9 @@ void ElectronRenderFrameObserver::DidInstallConditionalFeatures(
 
   if (should_create_isolated_context) {
     CreateIsolatedWorldContext();
-    if (!renderer_client_->IsWebViewFrame(context, render_frame_))
+    if (!renderer_client_->IsWebViewFrame(isolate, context, render_frame_))
-      renderer_client_->SetupMainWorldOverrides(context, render_frame_);
+      renderer_client_->SetupMainWorldOverrides(isolate, context,
+                                                render_frame_);
   }
 }
 
@@ -150,7 +151,8 @@ void ElectronRenderFrameObserver::WillReleaseScriptContext(
     v8::Local<v8::Context> context,
     int world_id) {
   if (ShouldNotifyClient(world_id))
-    renderer_client_->WillReleaseScriptContext(context, render_frame_);
+    renderer_client_->WillReleaseScriptContext(context->GetIsolate(), context,
+                                               render_frame_);
 }
 
 void ElectronRenderFrameObserver::OnDestruct() {

shell/renderer/electron_renderer_client.cc

@@ -98,7 +98,7 @@ void ElectronRendererClient::DidCreateScriptContext(
 
   // Only load Node.js if we are a main frame or a devtools extension
   // unless Node.js support has been explicitly enabled for subframes.
-  if (!ShouldLoadPreload(renderer_context, render_frame))
+  if (!ShouldLoadPreload(isolate, renderer_context, render_frame))
     return;
 
   injected_frames_.insert(render_frame);
@@ -178,6 +178,7 @@ void ElectronRendererClient::DidCreateScriptContext(
 }
 
 void ElectronRendererClient::WillReleaseScriptContext(
+    v8::Isolate* const isolate,
     v8::Local<v8::Context> context,
     content::RenderFrame* render_frame) {
   if (injected_frames_.erase(render_frame) == 0)
@@ -189,7 +190,7 @@ void ElectronRendererClient::WillReleaseScriptContext(
   if (iter == environments_.end())
     return;
 
-  gin_helper::EmitEvent(env->isolate(), env->process_object(), "exit");
+  gin_helper::EmitEvent(isolate, env->process_object(), "exit");
 
   // The main frame may be replaced.
   if (env == node_bindings_->uv_env())

shell/renderer/electron_renderer_client.h

@@ -32,7 +32,8 @@ class ElectronRendererClient : public RendererClientBase {
   void DidCreateScriptContext(v8::Isolate* isolate,
                               v8::Local<v8::Context> context,
                               content::RenderFrame* render_frame) override;
-  void WillReleaseScriptContext(v8::Local<v8::Context> context,
+  void WillReleaseScriptContext(v8::Isolate* isolate,
+                                v8::Local<v8::Context> context,
                                 content::RenderFrame* render_frame) override;
 
  private:

shell/renderer/electron_sandboxed_renderer_client.cc

@@ -36,9 +36,9 @@ constinit thread_local ServiceWorkerData* service_worker_data = nullptr;
 
 constexpr std::string_view kEmitProcessEventKey = "emit-process-event";
 
-void InvokeEmitProcessEvent(v8::Local<v8::Context> context,
+void InvokeEmitProcessEvent(v8::Isolate* const isolate,
+                            v8::Local<v8::Context> context,
                             const std::string& event_name) {
-  auto* isolate = context->GetIsolate();
   // set by sandboxed_renderer/init.js
   auto binding_key = gin::ConvertToV8(isolate, kEmitProcessEventKey)
                          ->ToString(context)
@@ -69,9 +69,9 @@ ElectronSandboxedRendererClient::~ElectronSandboxedRendererClient() = default;
 
 void ElectronSandboxedRendererClient::InitializeBindings(
     v8::Local<v8::Object> binding,
+    v8::Isolate* const isolate,
     v8::Local<v8::Context> context,
     content::RenderFrame* render_frame) {
-  auto* isolate = context->GetIsolate();
   gin_helper::Dictionary b(isolate, binding);
   b.SetMethod("get", preload_utils::GetBinding);
   b.SetMethod("createPreloadScript", preload_utils::CreatePreloadScript);
@@ -114,7 +114,7 @@ void ElectronSandboxedRendererClient::DidCreateScriptContext(
   // Only allow preload for the main frame or
   // For devtools we still want to run the preload_bundle script
   // Or when nodeSupport is explicitly enabled in sub frames
-  if (!ShouldLoadPreload(context, render_frame))
+  if (!ShouldLoadPreload(isolate, context, render_frame))
     return;
 
   injected_frames_.insert(render_frame);
@@ -122,7 +122,7 @@ void ElectronSandboxedRendererClient::DidCreateScriptContext(
   // Wrap the bundle into a function that receives the binding object as
   // argument.
   auto binding = v8::Object::New(isolate);
-  InitializeBindings(binding, context, render_frame);
+  InitializeBindings(binding, isolate, context, render_frame);
 
   v8::LocalVector<v8::String> sandbox_preload_bundle_params(
       isolate, {node::FIXED_ONE_BYTE_STRING(isolate, "binding")});
@@ -130,26 +130,26 @@ void ElectronSandboxedRendererClient::DidCreateScriptContext(
   v8::LocalVector<v8::Value> sandbox_preload_bundle_args(isolate, {binding});
 
   util::CompileAndCall(
-      isolate->GetCurrentContext(), "electron/js2c/sandbox_bundle",
+      isolate, isolate->GetCurrentContext(), "electron/js2c/sandbox_bundle",
       &sandbox_preload_bundle_params, &sandbox_preload_bundle_args);
 
-  v8::HandleScope handle_scope(isolate);
+  v8::HandleScope handle_scope{isolate};
-  v8::Context::Scope context_scope(context);
+  v8::Context::Scope context_scope{context};
-  InvokeEmitProcessEvent(context, "loaded");
+  InvokeEmitProcessEvent(isolate, context, "loaded");
 }
 
 void ElectronSandboxedRendererClient::WillReleaseScriptContext(
+    v8::Isolate* const isolate,
     v8::Local<v8::Context> context,
     content::RenderFrame* render_frame) {
   if (injected_frames_.erase(render_frame) == 0)
     return;
 
-  auto* isolate = context->GetIsolate();
   v8::MicrotasksScope microtasks_scope(
       context, v8::MicrotasksScope::kDoNotRunMicrotasks);
-  v8::HandleScope handle_scope(isolate);
+  v8::HandleScope handle_scope{isolate};
-  v8::Context::Scope context_scope(context);
+  v8::Context::Scope context_scope{context};
-  InvokeEmitProcessEvent(context, "exit");
+  InvokeEmitProcessEvent(isolate, context, "exit");
 }
 
 void ElectronSandboxedRendererClient::EmitProcessEvent(
@@ -163,11 +163,11 @@ void ElectronSandboxedRendererClient::EmitProcessEvent(
   v8::HandleScope handle_scope{isolate};
 
   v8::Local<v8::Context> context = GetContext(frame, isolate);
-  v8::MicrotasksScope microtasks_scope(
+  v8::MicrotasksScope microtasks_scope{
-      context, v8::MicrotasksScope::kDoNotRunMicrotasks);
+      context, v8::MicrotasksScope::kDoNotRunMicrotasks};
-  v8::Context::Scope context_scope(context);
+  v8::Context::Scope context_scope{context};
 
-  InvokeEmitProcessEvent(context, event_name);
+  InvokeEmitProcessEvent(isolate, context, event_name);
 }
 
 void ElectronSandboxedRendererClient::WillEvaluateServiceWorkerOnWorkerThread(
@@ -183,12 +183,14 @@ void ElectronSandboxedRendererClient::WillEvaluateServiceWorkerOnWorkerThread(
 
   auto* command_line = base::CommandLine::ForCurrentProcess();
   if (command_line->HasSwitch(switches::kServiceWorkerPreload)) {
+    v8::Isolate* const v8_isolate = v8_context->GetIsolate();
+
     if (!service_worker_data) {
-      service_worker_data = new ServiceWorkerData(
+      service_worker_data = new ServiceWorkerData{
-          context_proxy, service_worker_version_id, v8_context);
+          context_proxy, service_worker_version_id, v8_isolate, v8_context};
     }
 
-    preload_realm::OnCreatePreloadableV8Context(v8_context,
+    preload_realm::OnCreatePreloadableV8Context(v8_isolate, v8_context,
                                                 service_worker_data);
   }
 }

shell/renderer/electron_sandboxed_renderer_client.h

@@ -31,13 +31,15 @@ class ElectronSandboxedRendererClient : public RendererClientBase {
       const ElectronSandboxedRendererClient&) = delete;
 
   void InitializeBindings(v8::Local<v8::Object> binding,
+                          v8::Isolate* isolate,
                           v8::Local<v8::Context> context,
                           content::RenderFrame* render_frame);
   // electron::RendererClientBase:
   void DidCreateScriptContext(v8::Isolate* isolate,
                               v8::Local<v8::Context> context,
                               content::RenderFrame* render_frame) override;
-  void WillReleaseScriptContext(v8::Local<v8::Context> context,
+  void WillReleaseScriptContext(v8::Isolate* isolate,
+                                v8::Local<v8::Context> context,
                                 content::RenderFrame* render_frame) override;
   // content::ContentRendererClient:
   void RenderFrameCreated(content::RenderFrame*) override;

shell/renderer/preload_realm_context.cc

@@ -181,7 +181,7 @@ class PreloadRealmLifetimeController
 
     v8::LocalVector<v8::Value> preload_realm_bundle_args(isolate, {binding});
 
-    util::CompileAndCall(context, "electron/js2c/preload_realm_bundle",
+    util::CompileAndCall(isolate, context, "electron/js2c/preload_realm_bundle",
                          &preload_realm_bundle_params,
                          &preload_realm_bundle_args);
   }
@@ -232,9 +232,9 @@ electron::ServiceWorkerData* GetServiceWorkerData(
 }
 
 void OnCreatePreloadableV8Context(
+    v8::Isolate* const isolate,
     v8::Local<v8::Context> initiator_context,
     electron::ServiceWorkerData* service_worker_data) {
-  v8::Isolate* isolate = initiator_context->GetIsolate();
   blink::ScriptState* initiator_script_state =
       blink::ScriptState::MaybeFrom(isolate, initiator_context);
   DCHECK(initiator_script_state);

shell/renderer/preload_realm_context.h

@@ -26,6 +26,7 @@ electron::ServiceWorkerData* GetServiceWorkerData(
 
 // Create
 void OnCreatePreloadableV8Context(
+    v8::Isolate* const isolate,
     v8::Local<v8::Context> initiator_context,
     electron::ServiceWorkerData* service_worker_data);
 

shell/renderer/renderer_client_base.cc

@@ -196,6 +196,7 @@ void RendererClientBase::BindProcess(v8::Isolate* isolate,
 }
 
 bool RendererClientBase::ShouldLoadPreload(
+    v8::Isolate* const isolate,
     v8::Local<v8::Context> context,
     content::RenderFrame* render_frame) const {
   auto prefs = render_frame->GetBlinkPreferences();
@@ -205,7 +206,7 @@ bool RendererClientBase::ShouldLoadPreload(
   bool allow_node_in_sub_frames = prefs.node_integration_in_sub_frames;
 
   return (is_main_frame || is_devtools || allow_node_in_sub_frames) &&
-         !IsWebViewFrame(context, render_frame);
+         !IsWebViewFrame(isolate, context, render_frame);
 }
 
 void RendererClientBase::RenderThreadStarted() {
@@ -545,10 +546,9 @@ v8::Local<v8::Context> RendererClientBase::GetContext(
 }
 
 bool RendererClientBase::IsWebViewFrame(
+    v8::Isolate* const isolate,
     v8::Local<v8::Context> context,
     content::RenderFrame* render_frame) const {
-  auto* isolate = context->GetIsolate();
-
   if (render_frame->IsMainFrame())
     return false;
 
@@ -566,6 +566,7 @@ bool RendererClientBase::IsWebViewFrame(
 }
 
 void RendererClientBase::SetupMainWorldOverrides(
+    v8::Isolate* const isolate,
     v8::Local<v8::Context> context,
     content::RenderFrame* render_frame) {
   auto prefs = render_frame->GetBlinkPreferences();
@@ -576,9 +577,8 @@ void RendererClientBase::SetupMainWorldOverrides(
   // Setup window overrides in the main world context
   // Wrap the bundle into a function that receives the isolatedApi as
   // an argument.
-  auto* isolate = context->GetIsolate();
+  v8::HandleScope handle_scope{isolate};
-  v8::HandleScope handle_scope(isolate);
+  v8::Context::Scope context_scope{context};
-  v8::Context::Scope context_scope(context);
 
   auto isolated_api = gin_helper::Dictionary::CreateEmpty(isolate);
   isolated_api.SetMethod("allowGuestViewElementDefinition",
@@ -604,7 +604,7 @@ void RendererClientBase::SetupMainWorldOverrides(
   v8::LocalVector<v8::Value> isolated_bundle_args(isolate,
                                                   {isolated_api.GetHandle()});
 
-  util::CompileAndCall(context, "electron/js2c/isolated_bundle",
+  util::CompileAndCall(isolate, context, "electron/js2c/isolated_bundle",
                        &isolated_bundle_params, &isolated_bundle_args);
 }
 

shell/renderer/renderer_client_base.h

@@ -59,10 +59,12 @@ class RendererClientBase : public content::ContentRendererClient
   virtual void DidCreateScriptContext(v8::Isolate* isolate,
                                       v8::Local<v8::Context> context,
                                       content::RenderFrame* render_frame) = 0;
-  virtual void WillReleaseScriptContext(v8::Local<v8::Context> context,
+  virtual void WillReleaseScriptContext(v8::Isolate* isolate,
+                                        v8::Local<v8::Context> context,
                                         content::RenderFrame* render_frame) = 0;
   virtual void DidClearWindowObject(content::RenderFrame* render_frame);
-  virtual void SetupMainWorldOverrides(v8::Local<v8::Context> context,
+  virtual void SetupMainWorldOverrides(v8::Isolate* isolate,
+                                       v8::Local<v8::Context> context,
                                        content::RenderFrame* render_frame);
 
   std::unique_ptr<blink::WebPrescientNetworking> CreatePrescientNetworking(
@@ -77,7 +79,8 @@ class RendererClientBase : public content::ContentRendererClient
       v8::Local<v8::Object> context,
       v8::Local<v8::Function> register_cb);
 
-  bool IsWebViewFrame(v8::Local<v8::Context> context,
+  bool IsWebViewFrame(v8::Isolate* isolate,
+                      v8::Local<v8::Context> context,
                       content::RenderFrame* render_frame) const;
 
 #if BUILDFLAG(ENABLE_BUILTIN_SPELLCHECKER)
@@ -89,7 +92,8 @@ class RendererClientBase : public content::ContentRendererClient
                    gin_helper::Dictionary* process,
                    content::RenderFrame* render_frame);
 
-  bool ShouldLoadPreload(v8::Local<v8::Context> context,
+  bool ShouldLoadPreload(v8::Isolate* isolate,
+                         v8::Local<v8::Context> context,
                          content::RenderFrame* render_frame) const;
 
   // content::ContentRendererClient:

shell/renderer/service_worker_data.cc

@@ -18,11 +18,12 @@ ServiceWorkerData::~ServiceWorkerData() = default;
 
 ServiceWorkerData::ServiceWorkerData(blink::WebServiceWorkerContextProxy* proxy,
                                      int64_t service_worker_version_id,
+                                     v8::Isolate* const isolate,
                                      const v8::Local<v8::Context>& v8_context)
-    : proxy_(proxy),
+    : proxy_{proxy},
-      service_worker_version_id_(service_worker_version_id),
+      service_worker_version_id_{service_worker_version_id},
-      isolate_(v8_context->GetIsolate()),
+      isolate_{isolate},
-      v8_context_(v8_context->GetIsolate(), v8_context) {
+      v8_context_(isolate_, v8_context) {
   proxy_->GetAssociatedInterfaceRegistry()
       .AddInterface<mojom::ElectronRenderer>(
           base::BindRepeating(&ServiceWorkerData::OnElectronRendererRequest,

shell/renderer/service_worker_data.h

@@ -25,6 +25,7 @@ class ServiceWorkerData : public mojom::ElectronRenderer {
  public:
   ServiceWorkerData(blink::WebServiceWorkerContextProxy* proxy,
                     int64_t service_worker_version_id,
+                    v8::Isolate* const isolate,
                     const v8::Local<v8::Context>& v8_context);
   ~ServiceWorkerData() override;