Browse Source

refactor: use WeakRef on renderer side of remote (#24037)

Jeremy Rose 4 years ago
parent
commit
379bb174e9

+ 0 - 2
BUILD.gn

@@ -595,8 +595,6 @@ source_set("electron_lib") {
     sources += [
       "shell/common/api/remote/remote_callback_freer.cc",
       "shell/common/api/remote/remote_callback_freer.h",
-      "shell/common/api/remote/remote_object_freer.cc",
-      "shell/common/api/remote/remote_object_freer.h",
     ]
   }
 

+ 26 - 5
lib/renderer/api/remote.ts

@@ -11,7 +11,28 @@ const { hasSwitch } = process.electronBinding('command_line');
 const { NativeImage } = process.electronBinding('native_image');
 
 const callbacksRegistry = new CallbacksRegistry();
-const remoteObjectCache = v8Util.createIDWeakMap();
+const remoteObjectCache = new Map();
+const finalizationRegistry = new (window as any).FinalizationRegistry((id: number) => {
+  const ref = remoteObjectCache.get(id);
+  if (ref !== undefined && ref.deref() === undefined) {
+    remoteObjectCache.delete(id);
+    ipcRendererInternal.send('ELECTRON_BROWSER_DEREFERENCE', contextId, id, 0);
+  }
+});
+
+function getCachedRemoteObject (id: number) {
+  const ref = remoteObjectCache.get(id);
+  if (ref !== undefined) {
+    const deref = ref.deref();
+    if (deref !== undefined) return deref;
+  }
+}
+function setCachedRemoteObject (id: number, value: any) {
+  const wr = new (window as any).WeakRef(value);
+  remoteObjectCache.set(id, wr);
+  finalizationRegistry.register(value, id);
+  return value;
+}
 
 // An unique ID that can represent current context.
 const contextId = v8Util.getHiddenValue<string>(global, 'contextId');
@@ -234,8 +255,9 @@ function metaToValue (meta: MetaType): any {
     if (meta.value.type === 'error') { throw metaToError(meta.value); } else { throw new Error(`Unexpected value type in exception: ${meta.value.type}`); }
   } else {
     let ret;
-    if ('id' in meta && remoteObjectCache.has(meta.id)) {
-      return remoteObjectCache.get(meta.id);
+    if ('id' in meta) {
+      const cached = getCachedRemoteObject(meta.id);
+      if (cached !== undefined) { return cached; }
     }
 
     // A shadow class to represent the remote function object.
@@ -262,9 +284,8 @@ function metaToValue (meta: MetaType): any {
     }
 
     // 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);
-    remoteObjectCache.set(meta.id, ret);
+    setCachedRemoteObject(meta.id, ret);
     return ret;
   }
 }

+ 0 - 2
shell/common/api/electron_api_v8_util.cc

@@ -18,7 +18,6 @@
 
 #if BUILDFLAG(ENABLE_REMOTE_MODULE)
 #include "shell/common/api/remote/remote_callback_freer.h"
-#include "shell/common/api/remote/remote_object_freer.h"
 #endif
 
 namespace std {
@@ -148,7 +147,6 @@ void Initialize(v8::Local<v8::Object> exports,
 #if BUILDFLAG(ENABLE_REMOTE_MODULE)
   dict.SetMethod("setRemoteCallbackFreer",
                  &electron::RemoteCallbackFreer::BindTo);
-  dict.SetMethod("setRemoteObjectFreer", &electron::RemoteObjectFreer::BindTo);
   dict.SetMethod(
       "createDoubleIDWeakMap",
       &electron::api::KeyWeakMap<std::pair<std::string, int32_t>>::Create);

+ 0 - 66
shell/common/api/remote/remote_object_freer.cc

@@ -1,66 +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_object_freer.h"
-
-#include "base/strings/utf_string_conversions.h"
-#include "base/values.h"
-#include "content/public/renderer/render_frame.h"
-#include "services/service_manager/public/cpp/interface_provider.h"
-#include "shell/common/api/api.mojom.h"
-#include "third_party/blink/public/web/web_local_frame.h"
-
-using blink::WebLocalFrame;
-
-namespace electron {
-
-namespace {
-
-content::RenderFrame* GetCurrentRenderFrame() {
-  WebLocalFrame* frame = WebLocalFrame::FrameForCurrentContext();
-  if (!frame)
-    return nullptr;
-
-  return content::RenderFrame::FromWebFrame(frame);
-}
-
-}  // namespace
-
-// static
-void RemoteObjectFreer::BindTo(v8::Isolate* isolate,
-                               v8::Local<v8::Object> target,
-                               const std::string& context_id,
-                               int object_id) {
-  new RemoteObjectFreer(isolate, target, context_id, object_id);
-}
-
-RemoteObjectFreer::RemoteObjectFreer(v8::Isolate* isolate,
-                                     v8::Local<v8::Object> target,
-                                     const std::string& context_id,
-                                     int object_id)
-    : ObjectLifeMonitor(isolate, target),
-      context_id_(context_id),
-      object_id_(object_id),
-      routing_id_(MSG_ROUTING_NONE) {
-  content::RenderFrame* render_frame = GetCurrentRenderFrame();
-  if (render_frame) {
-    routing_id_ = render_frame->GetRoutingID();
-  }
-}
-
-RemoteObjectFreer::~RemoteObjectFreer() = default;
-
-void RemoteObjectFreer::RunDestructor() {
-  content::RenderFrame* render_frame =
-      content::RenderFrame::FromRoutingID(routing_id_);
-  if (!render_frame)
-    return;
-
-  mojom::ElectronBrowserPtr electron_ptr;
-  render_frame->GetRemoteInterfaces()->GetInterface(
-      mojo::MakeRequest(&electron_ptr));
-  electron_ptr->DereferenceRemoteJSObject(context_id_, object_id_);
-}
-
-}  // namespace electron

+ 0 - 41
shell/common/api/remote/remote_object_freer.h

@@ -1,41 +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_OBJECT_FREER_H_
-#define SHELL_COMMON_API_REMOTE_REMOTE_OBJECT_FREER_H_
-
-#include <map>
-#include <string>
-
-#include "shell/common/api/object_life_monitor.h"
-
-namespace electron {
-
-class RemoteObjectFreer : public ObjectLifeMonitor {
- public:
-  static void BindTo(v8::Isolate* isolate,
-                     v8::Local<v8::Object> target,
-                     const std::string& context_id,
-                     int object_id);
-
- protected:
-  RemoteObjectFreer(v8::Isolate* isolate,
-                    v8::Local<v8::Object> target,
-                    const std::string& context_id,
-                    int object_id);
-  ~RemoteObjectFreer() override;
-
-  void RunDestructor() override;
-
- private:
-  std::string context_id_;
-  int object_id_;
-  int routing_id_;
-
-  DISALLOW_COPY_AND_ASSIGN(RemoteObjectFreer);
-};
-
-}  // namespace electron
-
-#endif  // SHELL_COMMON_API_REMOTE_REMOTE_OBJECT_FREER_H_

+ 0 - 1
typings/internal-ambient.d.ts

@@ -46,7 +46,6 @@ declare namespace NodeJS {
     getWeaklyTrackedValues(): any[];
     addRemoteObjectRef(contextId: string, id: number): void;
     setRemoteCallbackFreer(fn: Function, contextId: string, id: number, sender: any): void
-    setRemoteObjectFreer(object: any, contextId: string, id: number): void
   }
 
   type DataPipe = {