fix: do not leak IPC or context bridge promises (#23321)

shell/renderer/api/electron_api_context_bridge.cc

@@ -5,6 +5,7 @@
 #include "shell/renderer/api/electron_api_context_bridge.h"
 
 #include <map>
+#include <memory>
 #include <set>
 #include <string>
 #include <utility>
@@ -195,18 +196,32 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
     v8::Context::Scope destination_scope(destination_context);
     {
       auto source_promise = v8::Local<v8::Promise>::Cast(value);
-      auto* proxied_promise = new gin_helper::Promise<v8::Local<v8::Value>>(
-          destination_context->GetIsolate());
+      // Make the promise a shared_ptr so that when the original promise is
+      // freed the proxy promise is correctly freed as well instead of being
+      // left dangling
+      std::shared_ptr<gin_helper::Promise<v8::Local<v8::Value>>>
+          proxied_promise(new gin_helper::Promise<v8::Local<v8::Value>>(
+              destination_context->GetIsolate()));
       v8::Local<v8::Promise> proxied_promise_handle =
           proxied_promise->GetHandle();
 
+      v8::Global<v8::Context> global_then_source_context(
+          source_context->GetIsolate(), source_context);
+      v8::Global<v8::Context> global_then_destination_context(
+          destination_context->GetIsolate(), destination_context);
+      global_then_source_context.SetWeak();
+      global_then_destination_context.SetWeak();
       auto then_cb = base::BindOnce(
-          [](gin_helper::Promise<v8::Local<v8::Value>>* proxied_promise,
+          [](std::shared_ptr<gin_helper::Promise<v8::Local<v8::Value>>>
+                 proxied_promise,
              v8::Isolate* isolate,
              v8::Global<v8::Context> global_source_context,
              v8::Global<v8::Context> global_destination_context,
              context_bridge::RenderFrameFunctionStore* store,
              v8::Local<v8::Value> result) {
+            if (global_source_context.IsEmpty() ||
+                global_destination_context.IsEmpty())
+              return;
             context_bridge::ObjectCache object_cache;
             auto val =
                 PassValueToOtherContext(global_source_context.Get(isolate),
@@ -214,20 +229,28 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
                                         result, store, &object_cache, false, 0);
             if (!val.IsEmpty())
               proxied_promise->Resolve(val.ToLocalChecked());
-            delete proxied_promise;
           },
           proxied_promise, destination_context->GetIsolate(),
-          v8::Global<v8::Context>(source_context->GetIsolate(), source_context),
-          v8::Global<v8::Context>(destination_context->GetIsolate(),
-                                  destination_context),
-          store);
+          std::move(global_then_source_context),
+          std::move(global_then_destination_context), store);
+
+      v8::Global<v8::Context> global_catch_source_context(
+          source_context->GetIsolate(), source_context);
+      v8::Global<v8::Context> global_catch_destination_context(
+          destination_context->GetIsolate(), destination_context);
+      global_catch_source_context.SetWeak();
+      global_catch_destination_context.SetWeak();
       auto catch_cb = base::BindOnce(
-          [](gin_helper::Promise<v8::Local<v8::Value>>* proxied_promise,
+          [](std::shared_ptr<gin_helper::Promise<v8::Local<v8::Value>>>
+                 proxied_promise,
              v8::Isolate* isolate,
              v8::Global<v8::Context> global_source_context,
              v8::Global<v8::Context> global_destination_context,
              context_bridge::RenderFrameFunctionStore* store,
              v8::Local<v8::Value> result) {
+            if (global_source_context.IsEmpty() ||
+                global_destination_context.IsEmpty())
+              return;
             context_bridge::ObjectCache object_cache;
             auto val =
                 PassValueToOtherContext(global_source_context.Get(isolate),
@@ -235,13 +258,10 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
                                         result, store, &object_cache, false, 0);
             if (!val.IsEmpty())
               proxied_promise->Reject(val.ToLocalChecked());
-            delete proxied_promise;
           },
           proxied_promise, destination_context->GetIsolate(),
-          v8::Global<v8::Context>(source_context->GetIsolate(), source_context),
-          v8::Global<v8::Context>(destination_context->GetIsolate(),
-                                  destination_context),
-          store);
+          std::move(global_catch_source_context),
+          std::move(global_catch_destination_context), store);
 
       ignore_result(source_promise->Then(
           source_context,

shell/renderer/api/electron_api_ipc_renderer.cc

@@ -7,6 +7,7 @@
 #include "base/task/post_task.h"
 #include "base/values.h"
 #include "content/public/renderer/render_frame.h"
+#include "content/public/renderer/render_frame_observer.h"
 #include "gin/dictionary.h"
 #include "gin/handle.h"
 #include "gin/object_template_builder.h"
@@ -36,7 +37,8 @@ RenderFrame* GetCurrentRenderFrame() {
   return RenderFrame::FromWebFrame(frame);
 }
 
-class IPCRenderer : public gin::Wrappable<IPCRenderer> {
+class IPCRenderer : public gin::Wrappable<IPCRenderer>,
+                    public content::RenderFrameObserver {
  public:
   static gin::WrapperInfo kWrapperInfo;
 
@@ -44,19 +46,32 @@ class IPCRenderer : public gin::Wrappable<IPCRenderer> {
     return gin::CreateHandle(isolate, new IPCRenderer(isolate));
   }
 
-  explicit IPCRenderer(v8::Isolate* isolate) {
+  explicit IPCRenderer(v8::Isolate* isolate)
+      : content::RenderFrameObserver(GetCurrentRenderFrame()) {
     RenderFrame* render_frame = GetCurrentRenderFrame();
     DCHECK(render_frame);
+    weak_context_ =
+        v8::Global<v8::Context>(isolate, isolate->GetCurrentContext());
+    weak_context_.SetWeak();
 
     render_frame->GetRemoteInterfaces()->GetInterface(
         mojo::MakeRequest(&electron_browser_ptr_));
   }
 
+  void OnDestruct() override { electron_browser_ptr_.reset(); }
+
+  void WillReleaseScriptContext(v8::Local<v8::Context> context,
+                                int32_t world_id) override {
+    if (weak_context_.IsEmpty() ||
+        weak_context_.Get(context->GetIsolate()) == context)
+      electron_browser_ptr_.reset();
+  }
+
   // gin::Wrappable:
   gin::ObjectTemplateBuilder GetObjectTemplateBuilder(
       v8::Isolate* isolate) override {
     return gin::Wrappable<IPCRenderer>::GetObjectTemplateBuilder(isolate)
-        .SetMethod("send", &IPCRenderer::Send)
+        .SetMethod("send", &IPCRenderer::SendMessage)
         .SetMethod("sendSync", &IPCRenderer::SendSync)
         .SetMethod("sendTo", &IPCRenderer::SendTo)
         .SetMethod("sendToHost", &IPCRenderer::SendToHost)
@@ -67,10 +82,10 @@ class IPCRenderer : public gin::Wrappable<IPCRenderer> {
   const char* GetTypeName() override { return "IPCRenderer"; }
 
  private:
-  void Send(v8::Isolate* isolate,
-            bool internal,
-            const std::string& channel,
-            v8::Local<v8::Value> arguments) {
+  void SendMessage(v8::Isolate* isolate,
+                   bool internal,
+                   const std::string& channel,
+                   v8::Local<v8::Value> arguments) {
     blink::CloneableMessage message;
     if (!electron::SerializeV8Value(isolate, arguments, &message)) {
       return;
@@ -175,6 +190,7 @@ class IPCRenderer : public gin::Wrappable<IPCRenderer> {
     return electron::DeserializeV8Value(isolate, result);
   }
 
+  v8::Global<v8::Context> weak_context_;
   electron::mojom::ElectronBrowserPtr electron_browser_ptr_;
 };