refactor: remove renderer-side refcount in remote (#24054)

lib/browser/remote/objects-registry.ts

@@ -51,19 +51,11 @@ class ObjectsRegistry {
   // Dereference an object according to its ID.
   // Note that an object may be double-freed (cleared when page is reloaded, and
   // then garbage collected in old page).
-  // rendererSideRefCount is the ref count that the renderer process reported
+  remove (webContents: WebContents, contextId: string, id: number) {
-  // at time of GC if this is different to the number of references we sent to
-  // the given owner then a GC occurred between a ref being sent and the value
-  // being pulled out of the weak map.
-  // In this case we decrement out ref count and do not delete the stored
-  // object
-  // For more details on why we do renderer side ref counting see
-  // https://github.com/electron/electron/pull/17464
-  remove (webContents: WebContents, contextId: string, id: number, rendererSideRefCount: number) {
     const ownerKey = getOwnerKey(webContents, contextId);
     const owner = this.owners[ownerKey];
     if (owner && owner.has(id)) {
-      const newRefCount = owner.get(id)! - rendererSideRefCount;
+      const newRefCount = owner.get(id)! - 1;
 
       // Only completely remove if the number of references GCed in the
       // renderer is the same as the number of references we sent them

lib/browser/remote/server.ts

@@ -557,8 +557,8 @@ handleRemoteCommand('ELECTRON_BROWSER_MEMBER_GET', function (event, contextId, i
   return valueToMeta(event.sender, contextId, obj[name]);
 });
 
-handleRemoteCommand('ELECTRON_BROWSER_DEREFERENCE', function (event, contextId, id, rendererSideRefCount) {
+handleRemoteCommand('ELECTRON_BROWSER_DEREFERENCE', function (event, contextId, id) {
-  objectsRegistry.remove(event.sender, contextId, id, rendererSideRefCount);
+  objectsRegistry.remove(event.sender, contextId, id);
 });
 
 handleRemoteCommand('ELECTRON_BROWSER_CONTEXT_RELEASE', (event, contextId) => {

lib/renderer/api/remote.js

@@ -231,7 +231,6 @@ function metaToValue (meta) {
   } else {
     let ret;
     if (remoteObjectCache.has(meta.id)) {
-      v8Util.addRemoteObjectRef(contextId, meta.id);
       return remoteObjectCache.get(meta.id);
     }
 
@@ -261,7 +260,6 @@ function metaToValue (meta) {
     // Track delegate obj's lifetime & tell browser to clean up when object is GCed.
     v8Util.setRemoteObjectFreer(ret, contextId, meta.id);
     v8Util.setHiddenValue(ret, 'electronId', meta.id);
-    v8Util.addRemoteObjectRef(contextId, meta.id);
     remoteObjectCache.set(meta.id, ret);
     return ret;
   }

shell/browser/api/electron_api_web_contents.cc

@@ -1228,12 +1228,10 @@ void WebContents::MessageHost(const std::string& channel,
 
 #if BUILDFLAG(ENABLE_REMOTE_MODULE)
 void WebContents::DereferenceRemoteJSObject(const std::string& context_id,
-                                            int object_id,
+                                            int object_id) {
-                                            int ref_count) {
   base::ListValue args;
   args.Append(context_id);
   args.Append(object_id);
-  args.Append(ref_count);
   EmitWithSender("-ipc-message", bindings_.dispatch_context(), InvokeCallback(),
                  /* internal */ true, "ELECTRON_BROWSER_DEREFERENCE",
                  std::move(args));

shell/browser/api/electron_api_web_contents.h

@@ -599,8 +599,7 @@ class WebContents : public gin_helper::TrackableObject<WebContents>,
                    blink::CloneableMessage arguments) override;
 #if BUILDFLAG(ENABLE_REMOTE_MODULE)
   void DereferenceRemoteJSObject(const std::string& context_id,
-                                 int object_id,
+                                 int object_id) override;
-                                 int ref_count) override;
 #endif
   void UpdateDraggableRegions(
       std::vector<mojom::DraggableRegionPtr> regions) override;

shell/common/api/api.mojom

@@ -84,8 +84,7 @@ interface ElectronBrowser {
   [EnableIf=enable_remote_module]
   DereferenceRemoteJSObject(
     string context_id,
-    int32 object_id,
+    int32 object_id);
-    int32 ref_count);
 
   UpdateDraggableRegions(
     array<DraggableRegion> regions);

shell/common/api/electron_api_v8_util.cc

@@ -149,7 +149,6 @@ void Initialize(v8::Local<v8::Object> exports,
   dict.SetMethod("setRemoteCallbackFreer",
                  &electron::RemoteCallbackFreer::BindTo);
   dict.SetMethod("setRemoteObjectFreer", &electron::RemoteObjectFreer::BindTo);
-  dict.SetMethod("addRemoteObjectRef", &electron::RemoteObjectFreer::AddRef);
   dict.SetMethod(
       "createDoubleIDWeakMap",
       &electron::api::KeyWeakMap<std::pair<std::string, int32_t>>::Create);

shell/common/api/remote/remote_object_freer.cc

@@ -35,14 +35,6 @@ void RemoteObjectFreer::BindTo(v8::Isolate* isolate,
   new RemoteObjectFreer(isolate, target, context_id, object_id);
 }
 
-// static
-void RemoteObjectFreer::AddRef(const std::string& context_id, int object_id) {
-  ref_mapper_[context_id][object_id]++;
-}
-
-// static
-std::map<std::string, std::map<int, int>> RemoteObjectFreer::ref_mapper_;
-
 RemoteObjectFreer::RemoteObjectFreer(v8::Isolate* isolate,
                                      v8::Local<v8::Object> target,
                                      const std::string& context_id,
@@ -65,25 +57,10 @@ void RemoteObjectFreer::RunDestructor() {
   if (!render_frame)
     return;
 
-  // Reset our local ref count in case we are in a GC race condition
-  // and will get more references in an inbound IPC message
-  int ref_count = 0;
-  const auto objects_it = ref_mapper_.find(context_id_);
-  if (objects_it != std::end(ref_mapper_)) {
-    auto& objects = objects_it->second;
-    const auto ref_it = objects.find(object_id_);
-    if (ref_it != std::end(objects)) {
-      ref_count = ref_it->second;
-      objects.erase(ref_it);
-    }
-    if (objects.empty())
-      ref_mapper_.erase(objects_it);
-  }
-
   mojom::ElectronBrowserPtr electron_ptr;
   render_frame->GetRemoteInterfaces()->GetInterface(
       mojo::MakeRequest(&electron_ptr));
-  electron_ptr->DereferenceRemoteJSObject(context_id_, object_id_, ref_count);
+  electron_ptr->DereferenceRemoteJSObject(context_id_, object_id_);
 }
 
 }  // namespace electron

shell/common/api/remote/remote_object_freer.h

@@ -18,7 +18,6 @@ class RemoteObjectFreer : public ObjectLifeMonitor {
                      v8::Local<v8::Object> target,
                      const std::string& context_id,
                      int object_id);
-  static void AddRef(const std::string& context_id, int object_id);
 
  protected:
   RemoteObjectFreer(v8::Isolate* isolate,
@@ -29,9 +28,6 @@ class RemoteObjectFreer : public ObjectLifeMonitor {
 
   void RunDestructor() override;
 
-  // { context_id => { object_id => ref_count }}
-  static std::map<std::string, std::map<int, int>> ref_mapper_;
-
  private:
   std::string context_id_;
   int object_id_;