refactor: use WeakRef on main process side of remote (#24115)

BUILD.gn

@@ -602,13 +602,6 @@ source_set("electron_lib") {
     ]
   }
 
-  if (enable_remote_module) {
-    sources += [
-      "shell/common/api/remote/remote_callback_freer.cc",
-      "shell/common/api/remote/remote_callback_freer.h",
-    ]
-  }
-
   if (enable_desktop_capturer) {
     if (is_component_build && !is_linux) {
       # On windows the implementation relies on unexported

lib/browser/remote/server.ts

@@ -14,16 +14,48 @@ if (!features.isRemoteModuleEnabled()) {
   throw new Error('remote module is disabled');
 }
 
-const hasProp = {}.hasOwnProperty;
-
 // The internal properties of Function.
 const FUNCTION_PROPERTIES = [
   'length', 'name', 'arguments', 'caller', 'prototype'
 ];
 
+type RendererFunctionId = [string, number] // [contextId, funcId]
+type FinalizerInfo = { id: RendererFunctionId, webContents: electron.WebContents, frameId: number };
+type WeakRef<T> = { deref(): T | undefined }
+type CallIntoRenderer = (...args: any[]) => void
+
 // The remote functions in renderer processes.
-// id => Function
+const rendererFunctionCache = new Map<string, WeakRef<CallIntoRenderer>>();
-const rendererFunctions = v8Util.createDoubleIDWeakMap<(...args: any[]) => void>();
+// eslint-disable-next-line no-undef
+const finalizationRegistry = new (globalThis as any).FinalizationRegistry((fi: FinalizerInfo) => {
+  const mapKey = fi.id[0] + '~' + fi.id[1];
+  const ref = rendererFunctionCache.get(mapKey);
+  if (ref !== undefined && ref.deref() === undefined) {
+    rendererFunctionCache.delete(mapKey);
+    if (!fi.webContents.isDestroyed()) { fi.webContents.sendToFrame(fi.frameId, 'ELECTRON_RENDERER_RELEASE_CALLBACK', fi.id[0], fi.id[1]); }
+  }
+});
+
+function getCachedRendererFunction (id: RendererFunctionId): CallIntoRenderer | undefined {
+  const mapKey = id[0] + '~' + id[1];
+  const ref = rendererFunctionCache.get(mapKey);
+  if (ref !== undefined) {
+    const deref = ref.deref();
+    if (deref !== undefined) return deref;
+  }
+}
+function setCachedRendererFunction (id: RendererFunctionId, wc: electron.WebContents, frameId: number, value: CallIntoRenderer) {
+  // eslint-disable-next-line no-undef
+  const wr = new (globalThis as any).WeakRef(value) as WeakRef<CallIntoRenderer>;
+  const mapKey = id[0] + '~' + id[1];
+  rendererFunctionCache.set(mapKey, wr);
+  finalizationRegistry.register(value, {
+    id,
+    webContents: wc,
+    frameId
+  } as FinalizerInfo);
+  return value;
+}
 
 // Return the description of object's members:
 const getObjectMembers = function (object: any): ObjectMember[] {
@@ -80,7 +112,7 @@ const valueToMeta = function (sender: electron.WebContents, contextId: string, v
         type = 'value';
       } else if (isPromise(value)) {
         type = 'promise';
-      } else if (hasProp.call(value, 'callee') && value.length != null) {
+      } else if (Object.prototype.hasOwnProperty.call(value, 'callee') && value.length != null) {
         // Treat the arguments object as array.
         type = 'array';
       } else if (optimizeSimpleObject && v8Util.getHiddenValue(value, 'simple')) {
@@ -226,9 +258,8 @@ const unwrapArgs = function (sender: electron.WebContents, frameId: number, cont
         const objectId: [string, number] = [contextId, meta.id];
 
         // Cache the callbacks in renderer.
-        if (rendererFunctions.has(objectId)) {
+        const cachedFunction = getCachedRendererFunction(objectId);
-          return rendererFunctions.get(objectId);
+        if (cachedFunction !== undefined) { return cachedFunction; }
-        }
 
         const callIntoRenderer = function (this: any, ...args: any[]) {
           let succeed = false;
@@ -242,8 +273,7 @@ const unwrapArgs = function (sender: electron.WebContents, frameId: number, cont
         v8Util.setHiddenValue(callIntoRenderer, 'location', meta.location);
         Object.defineProperty(callIntoRenderer, 'length', { value: meta.length });
 
-        v8Util.setRemoteCallbackFreer(callIntoRenderer, frameId, contextId, meta.id, sender);
+        setCachedRendererFunction(objectId, sender, frameId, callIntoRenderer);
-        rendererFunctions.set(objectId, callIntoRenderer);
         return callIntoRenderer;
       }
       default:
@@ -308,11 +338,12 @@ const logStack = function (contents: electron.WebContents, code: string, stack:
 
 handleRemoteCommand('ELECTRON_BROWSER_WRONG_CONTEXT_ERROR', function (event, contextId, passedContextId, id) {
   const objectId: [string, number] = [passedContextId, id];
-  if (!rendererFunctions.has(objectId)) {
+  const cachedFunction = getCachedRendererFunction(objectId);
+  if (cachedFunction === undefined) {
     // Do nothing if the error has already been reported before.
     return;
   }
-  removeRemoteListenersAndLogWarning(event.sender, rendererFunctions.get(objectId)!);
+  removeRemoteListenersAndLogWarning(event.sender, cachedFunction);
 });
 
 handleRemoteCommand('ELECTRON_BROWSER_REQUIRE', function (event, contextId, moduleName, stack) {

shell/common/api/api.mojom

@@ -15,13 +15,6 @@ interface ElectronRenderer {
 
   ReceivePostMessage(string channel, blink.mojom.TransferableMessage message);
 
-  // This is an API specific to the "remote" module, and will ultimately be
-  // replaced by generic IPC once WeakRef is generally available.
-  [EnableIf=enable_remote_module]
-  DereferenceRemoteJSCallback(
-    string context_id,
-    int32 object_id);
-
   NotifyUserActivation();
 
   TakeHeapSnapshot(handle file) => (bool success);

shell/common/api/electron_api_v8_util.cc

@@ -16,10 +16,6 @@
 #include "url/origin.h"
 #include "v8/include/v8-profiler.h"
 
-#if BUILDFLAG(ENABLE_REMOTE_MODULE)
-#include "shell/common/api/remote/remote_callback_freer.h"
-#endif
-
 namespace std {
 
 // The hash function used by DoubleIDWeakMap.
@@ -144,13 +140,6 @@ void Initialize(v8::Local<v8::Object> exports,
   dict.SetMethod("deleteHiddenValue", &DeleteHiddenValue);
   dict.SetMethod("getObjectHash", &GetObjectHash);
   dict.SetMethod("takeHeapSnapshot", &TakeHeapSnapshot);
-#if BUILDFLAG(ENABLE_REMOTE_MODULE)
-  dict.SetMethod("setRemoteCallbackFreer",
-                 &electron::RemoteCallbackFreer::BindTo);
-  dict.SetMethod(
-      "createDoubleIDWeakMap",
-      &electron::api::KeyWeakMap<std::pair<std::string, int32_t>>::Create);
-#endif
   dict.SetMethod("requestGarbageCollectionForTesting",
                  &RequestGarbageCollectionForTesting);
   dict.SetMethod("isSameOrigin", &IsSameOrigin);

shell/common/api/remote/remote_callback_freer.cc

@@ -1,61 +0,0 @@
-// Copyright (c) 2016 GitHub, Inc.
-// Use of this source code is governed by the MIT license that can be
-// found in the LICENSE file.
-
-#include "shell/common/api/remote/remote_callback_freer.h"
-
-#include "base/strings/utf_string_conversions.h"
-#include "base/values.h"
-#include "content/public/browser/render_frame_host.h"
-#include "content/public/browser/web_contents.h"
-#include "electron/shell/common/api/api.mojom.h"
-#include "mojo/public/cpp/bindings/associated_remote.h"
-#include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
-
-namespace electron {
-
-// static
-void RemoteCallbackFreer::BindTo(v8::Isolate* isolate,
-                                 v8::Local<v8::Object> target,
-                                 int frame_id,
-                                 const std::string& context_id,
-                                 int object_id,
-                                 content::WebContents* web_contents) {
-  new RemoteCallbackFreer(isolate, target, frame_id, context_id, object_id,
-                          web_contents);
-}
-
-RemoteCallbackFreer::RemoteCallbackFreer(v8::Isolate* isolate,
-                                         v8::Local<v8::Object> target,
-                                         int frame_id,
-                                         const std::string& context_id,
-                                         int object_id,
-                                         content::WebContents* web_contents)
-    : ObjectLifeMonitor(isolate, target),
-      content::WebContentsObserver(web_contents),
-      frame_id_(frame_id),
-      context_id_(context_id),
-      object_id_(object_id) {}
-
-RemoteCallbackFreer::~RemoteCallbackFreer() = default;
-
-void RemoteCallbackFreer::RunDestructor() {
-  auto frames = web_contents()->GetAllFrames();
-  auto iter = std::find_if(frames.begin(), frames.end(), [this](auto* f) {
-    return f->GetRoutingID() == frame_id_;
-  });
-
-  if (iter != frames.end() && (*iter)->IsRenderFrameLive()) {
-    mojo::AssociatedRemote<mojom::ElectronRenderer> electron_renderer;
-    (*iter)->GetRemoteAssociatedInterfaces()->GetInterface(&electron_renderer);
-    electron_renderer->DereferenceRemoteJSCallback(context_id_, object_id_);
-  }
-
-  Observe(nullptr);
-}
-
-void RemoteCallbackFreer::RenderViewDeleted(content::RenderViewHost*) {
-  delete this;
-}
-
-}  // namespace electron

shell/common/api/remote/remote_callback_freer.h

@@ -1,49 +0,0 @@
-// Copyright (c) 2016 GitHub, Inc.
-// Use of this source code is governed by the MIT license that can be
-// found in the LICENSE file.
-
-#ifndef SHELL_COMMON_API_REMOTE_REMOTE_CALLBACK_FREER_H_
-#define SHELL_COMMON_API_REMOTE_REMOTE_CALLBACK_FREER_H_
-
-#include <string>
-
-#include "content/public/browser/web_contents_observer.h"
-#include "shell/common/api/object_life_monitor.h"
-
-namespace electron {
-
-class RemoteCallbackFreer : public ObjectLifeMonitor,
-                            public content::WebContentsObserver {
- public:
-  static void BindTo(v8::Isolate* isolate,
-                     v8::Local<v8::Object> target,
-                     int frame_id,
-                     const std::string& context_id,
-                     int object_id,
-                     content::WebContents* web_conents);
-
- protected:
-  RemoteCallbackFreer(v8::Isolate* isolate,
-                      v8::Local<v8::Object> target,
-                      int frame_id,
-                      const std::string& context_id,
-                      int object_id,
-                      content::WebContents* web_conents);
-  ~RemoteCallbackFreer() override;
-
-  void RunDestructor() override;
-
-  // content::WebContentsObserver:
-  void RenderViewDeleted(content::RenderViewHost*) override;
-
- private:
-  int frame_id_;
-  std::string context_id_;
-  int object_id_;
-
-  DISALLOW_COPY_AND_ASSIGN(RemoteCallbackFreer);
-};
-
-}  // namespace electron
-
-#endif  // SHELL_COMMON_API_REMOTE_REMOTE_CALLBACK_FREER_H_

shell/renderer/electron_api_service_impl.cc

@@ -209,33 +209,6 @@ void ElectronApiServiceImpl::ReceivePostMessage(
                0);
 }
 
-#if BUILDFLAG(ENABLE_REMOTE_MODULE)
-void ElectronApiServiceImpl::DereferenceRemoteJSCallback(
-    const std::string& context_id,
-    int32_t object_id) {
-  const auto* channel = "ELECTRON_RENDERER_RELEASE_CALLBACK";
-  if (!document_created_)
-    return;
-  blink::WebLocalFrame* frame = render_frame()->GetWebFrame();
-  if (!frame)
-    return;
-
-  v8::Isolate* isolate = blink::MainThreadIsolate();
-  v8::HandleScope handle_scope(isolate);
-
-  v8::Local<v8::Context> context = renderer_client_->GetContext(frame, isolate);
-  v8::Context::Scope context_scope(context);
-
-  base::ListValue args;
-  args.AppendString(context_id);
-  args.AppendInteger(object_id);
-
-  v8::Local<v8::Value> v8_args = gin::ConvertToV8(isolate, args);
-  EmitIPCEvent(context, true /* internal */, channel, {}, v8_args,
-               0 /* sender_id */);
-}
-#endif
-
 void ElectronApiServiceImpl::NotifyUserActivation() {
   blink::WebLocalFrame* frame = render_frame()->GetWebFrame();
   if (frame)

shell/renderer/electron_api_service_impl.h

@@ -35,10 +35,6 @@ class ElectronApiServiceImpl : public mojom::ElectronRenderer,
                int32_t sender_id) override;
   void ReceivePostMessage(const std::string& channel,
                           blink::TransferableMessage message) override;
-#if BUILDFLAG(ENABLE_REMOTE_MODULE)
-  void DereferenceRemoteJSCallback(const std::string& context_id,
-                                   int32_t object_id) override;
-#endif
   void NotifyUserActivation() override;
   void TakeHeapSnapshot(mojo::ScopedHandle file,
                         TakeHeapSnapshotCallback callback) override;

typings/internal-ambient.d.ts

@@ -38,13 +38,10 @@ declare namespace NodeJS {
     setHiddenValue<T>(obj: any, key: string, value: T): void;
     deleteHiddenValue(obj: any, key: string): void;
     requestGarbageCollectionForTesting(): void;
-    createDoubleIDWeakMap<V>(): ElectronInternal.KeyWeakMap<[string, number], V>;
-    setRemoteCallbackFreer(fn: Function, frameId: number, contextId: String, id: number, sender: any): void
     weaklyTrackValue(value: any): void;
     clearWeaklyTrackedValues(): void;
     getWeaklyTrackedValues(): any[];
     addRemoteObjectRef(contextId: string, id: number): void;
-    setRemoteCallbackFreer(fn: Function, contextId: string, id: number, sender: any): void
   }
 
   type DataPipe = {