Merge pull request #5293 from electron/native-gc

Dereference remote objects with native code

atom/browser/api/atom_api_web_view_manager.cc

@@ -2,9 +2,9 @@
 // Use of this source code is governed by the MIT license that can be
 // found in the LICENSE file.
 
-#include "atom/browser/api/atom_api_web_contents.h"
 #include "atom/browser/web_contents_preferences.h"
 #include "atom/browser/web_view_manager.h"
+#include "atom/common/native_mate_converters/content_converter.h"
 #include "atom/common/native_mate_converters/value_converter.h"
 #include "atom/common/node_includes.h"
 #include "content/public/browser/browser_context.h"
@@ -12,22 +12,6 @@
 
 using atom::WebContentsPreferences;
 
-namespace mate {
-
-template<>
-struct Converter<content::WebContents*> {
-  static bool FromV8(v8::Isolate* isolate, v8::Local<v8::Value> val,
-                     content::WebContents** out) {
-    atom::api::WebContents* contents;
-    if (!Converter<atom::api::WebContents*>::FromV8(isolate, val, &contents))
-      return false;
-    *out = contents->web_contents();
-    return true;
-  }
-};
-
-}  // namespace mate
-
 namespace {
 
 atom::WebViewManager* GetWebViewManager(content::WebContents* web_contents) {

atom/common/api/atom_api_v8_util.cc

@@ -4,7 +4,9 @@
 
 #include <string>
 
-#include "atom/common/api/object_life_monitor.h"
+#include "atom/common/api/remote_callback_freer.h"
+#include "atom/common/api/remote_object_freer.h"
+#include "atom/common/native_mate_converters/content_converter.h"
 #include "atom/common/node_includes.h"
 #include "native_mate/dictionary.h"
 #include "v8/include/v8-profiler.h"
@@ -51,12 +53,6 @@ int32_t GetObjectHash(v8::Local<v8::Object> object) {
   return object->GetIdentityHash();
 }
 
-void SetDestructor(v8::Isolate* isolate,
-                   v8::Local<v8::Object> object,
-                   v8::Local<v8::Function> callback) {
-  atom::ObjectLifeMonitor::BindTo(isolate, object, callback);
-}
-
 void TakeHeapSnapshot(v8::Isolate* isolate) {
   isolate->GetHeapProfiler()->TakeHeapSnapshot();
 }
@@ -68,8 +64,9 @@ void Initialize(v8::Local<v8::Object> exports, v8::Local<v8::Value> unused,
   dict.SetMethod("setHiddenValue", &SetHiddenValue);
   dict.SetMethod("deleteHiddenValue", &DeleteHiddenValue);
   dict.SetMethod("getObjectHash", &GetObjectHash);
-  dict.SetMethod("setDestructor", &SetDestructor);
   dict.SetMethod("takeHeapSnapshot", &TakeHeapSnapshot);
+  dict.SetMethod("setRemoteCallbackFreer", &atom::RemoteCallbackFreer::BindTo);
+  dict.SetMethod("setRemoteObjectFreer", &atom::RemoteObjectFreer::BindTo);
 }
 
 }  // namespace

atom/common/api/object_life_monitor.cc

@@ -10,30 +10,28 @@
 
 namespace atom {
 
-// static
-void ObjectLifeMonitor::BindTo(v8::Isolate* isolate,
-                               v8::Local<v8::Object> target,
-                               v8::Local<v8::Function> destructor) {
-  new ObjectLifeMonitor(isolate, target, destructor);
-}
-
 ObjectLifeMonitor::ObjectLifeMonitor(v8::Isolate* isolate,
-                                     v8::Local<v8::Object> target,
+                                     v8::Local<v8::Object> target)
-                                     v8::Local<v8::Function> destructor)
     : isolate_(isolate),
       context_(isolate, isolate->GetCurrentContext()),
       target_(isolate, target),
-      destructor_(isolate, destructor),
       weak_ptr_factory_(this) {
   target_.SetWeak(this, OnObjectGC, v8::WeakCallbackType::kParameter);
 }
 
+ObjectLifeMonitor::~ObjectLifeMonitor() {
+  if (target_.IsEmpty())
+    return;
+  target_.ClearWeak();
+  target_.Reset();
+}
+
 // static
 void ObjectLifeMonitor::OnObjectGC(
     const v8::WeakCallbackInfo<ObjectLifeMonitor>& data) {
   ObjectLifeMonitor* self = data.GetParameter();
   self->target_.Reset();
-  self->RunCallback();
+  self->RunDestructor();
   data.SetSecondPassCallback(Free);
 }
 
@@ -43,13 +41,4 @@ void ObjectLifeMonitor::Free(
   delete data.GetParameter();
 }
 
-void ObjectLifeMonitor::RunCallback() {
-  v8::HandleScope handle_scope(isolate_);
-  v8::Local<v8::Context> context = v8::Local<v8::Context>::New(
-      isolate_, context_);
-  v8::Context::Scope context_scope(context);
-  v8::Local<v8::Function>::New(isolate_, destructor_)->Call(
-      context->Global(), 0, nullptr);
-}
-
 }  // namespace atom

atom/common/api/object_life_monitor.h

@@ -12,25 +12,19 @@
 namespace atom {
 
 class ObjectLifeMonitor {
- public:
+ protected:
-  static void BindTo(v8::Isolate* isolate,
+  ObjectLifeMonitor(v8::Isolate* isolate, v8::Local<v8::Object> target);
-                     v8::Local<v8::Object> target,
+  virtual ~ObjectLifeMonitor();
-                     v8::Local<v8::Function> destructor);
+
+  virtual void RunDestructor() = 0;
 
  private:
-  ObjectLifeMonitor(v8::Isolate* isolate,
-                    v8::Local<v8::Object> target,
-                    v8::Local<v8::Function> destructor);
-
   static void OnObjectGC(const v8::WeakCallbackInfo<ObjectLifeMonitor>& data);
   static void Free(const v8::WeakCallbackInfo<ObjectLifeMonitor>& data);
 
-  void RunCallback();
-
   v8::Isolate* isolate_;
   v8::Global<v8::Context> context_;
   v8::Global<v8::Object> target_;
-  v8::Global<v8::Function> destructor_;
 
   base::WeakPtrFactory<ObjectLifeMonitor> weak_ptr_factory_;
 

atom/common/api/remote_callback_freer.cc

@@ -0,0 +1,47 @@
+// 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 "atom/common/api/remote_callback_freer.h"
+
+#include "atom/common/api/api_messages.h"
+#include "base/strings/utf_string_conversions.h"
+#include "base/values.h"
+
+namespace atom {
+
+// static
+void RemoteCallbackFreer::BindTo(v8::Isolate* isolate,
+                                 v8::Local<v8::Object> target,
+                                 int object_id,
+                                 content::WebContents* web_contents) {
+  new RemoteCallbackFreer(isolate, target, object_id, web_contents);
+}
+
+RemoteCallbackFreer::RemoteCallbackFreer(v8::Isolate* isolate,
+                                         v8::Local<v8::Object> target,
+                                         int object_id,
+                                         content::WebContents* web_contents)
+    : ObjectLifeMonitor(isolate, target),
+      content::WebContentsObserver(web_contents),
+      object_id_(object_id) {
+}
+
+RemoteCallbackFreer::~RemoteCallbackFreer() {
+}
+
+void RemoteCallbackFreer::RunDestructor() {
+  base::string16 channel =
+      base::ASCIIToUTF16("ELECTRON_RENDERER_RELEASE_CALLBACK");
+  base::ListValue args;
+  args.AppendInteger(object_id_);
+  Send(new AtomViewMsg_Message(routing_id(), channel, args));
+
+  Observe(nullptr);
+}
+
+void RemoteCallbackFreer::RenderViewDeleted(content::RenderViewHost*) {
+  delete this;
+}
+
+}  // namespace atom

atom/common/api/remote_callback_freer.h

@@ -0,0 +1,40 @@
+// 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 ATOM_COMMON_API_REMOTE_CALLBACK_FREER_H_
+#define ATOM_COMMON_API_REMOTE_CALLBACK_FREER_H_
+#include "atom/common/api/object_life_monitor.h"
+#include "content/public/browser/web_contents_observer.h"
+
+namespace atom {
+
+class RemoteCallbackFreer : public ObjectLifeMonitor,
+                            public content::WebContentsObserver {
+ public:
+  static void BindTo(v8::Isolate* isolate,
+                     v8::Local<v8::Object> target,
+                     int object_id,
+                     content::WebContents* web_conents);
+
+ protected:
+  RemoteCallbackFreer(v8::Isolate* isolate,
+                      v8::Local<v8::Object> target,
+                      int object_id,
+                      content::WebContents* web_conents);
+  ~RemoteCallbackFreer() override;
+
+  void RunDestructor() override;
+
+  // content::WebContentsObserver:
+  void RenderViewDeleted(content::RenderViewHost*) override;
+
+ private:
+  int object_id_;
+
+  DISALLOW_COPY_AND_ASSIGN(RemoteCallbackFreer);
+};
+
+}  // namespace atom
+
+#endif  // ATOM_COMMON_API_REMOTE_CALLBACK_FREER_H_

atom/common/api/remote_object_freer.cc

@@ -0,0 +1,63 @@
+// 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 "atom/common/api/remote_object_freer.h"
+
+#include "atom/common/api/api_messages.h"
+#include "base/strings/utf_string_conversions.h"
+#include "base/values.h"
+#include "content/public/renderer/render_view.h"
+#include "third_party/WebKit/public/web/WebLocalFrame.h"
+#include "third_party/WebKit/public/web/WebView.h"
+
+using blink::WebLocalFrame;
+using blink::WebView;
+
+namespace atom {
+
+namespace {
+
+content::RenderView* GetCurrentRenderView() {
+  WebLocalFrame* frame = WebLocalFrame::frameForCurrentContext();
+  if (!frame)
+    return nullptr;
+
+  WebView* view = frame->view();
+  if (!view)
+    return nullptr;  // can happen during closing.
+
+  return content::RenderView::FromWebView(view);
+}
+
+}  // namespace
+
+// static
+void RemoteObjectFreer::BindTo(
+    v8::Isolate* isolate, v8::Local<v8::Object> target, int object_id) {
+  new RemoteObjectFreer(isolate, target, object_id);
+}
+
+RemoteObjectFreer::RemoteObjectFreer(
+    v8::Isolate* isolate, v8::Local<v8::Object> target, int object_id)
+    : ObjectLifeMonitor(isolate, target),
+      object_id_(object_id) {
+}
+
+RemoteObjectFreer::~RemoteObjectFreer() {
+}
+
+void RemoteObjectFreer::RunDestructor() {
+  content::RenderView* render_view = GetCurrentRenderView();
+  if (!render_view)
+    return;
+
+  base::string16 channel = base::ASCIIToUTF16("ipc-message");
+  base::ListValue args;
+  args.AppendString("ELECTRON_BROWSER_DEREFERENCE");
+  args.AppendInteger(object_id_);
+  render_view->Send(
+      new AtomViewHostMsg_Message(render_view->GetRoutingID(), channel, args));
+}
+
+}  // namespace atom

atom/common/api/remote_object_freer.h

@@ -0,0 +1,32 @@
+// 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 ATOM_COMMON_API_REMOTE_OBJECT_FREER_H_
+#define ATOM_COMMON_API_REMOTE_OBJECT_FREER_H_
+
+#include "atom/common/api/object_life_monitor.h"
+
+namespace atom {
+
+class RemoteObjectFreer : public ObjectLifeMonitor {
+ public:
+  static void BindTo(
+      v8::Isolate* isolate, v8::Local<v8::Object> target, int object_id);
+
+ protected:
+  RemoteObjectFreer(
+      v8::Isolate* isolate, v8::Local<v8::Object> target, int object_id);
+  ~RemoteObjectFreer() override;
+
+  void RunDestructor() override;
+
+ private:
+  int object_id_;
+
+  DISALLOW_COPY_AND_ASSIGN(RemoteObjectFreer);
+};
+
+}  // namespace atom
+
+#endif  // ATOM_COMMON_API_REMOTE_OBJECT_FREER_H_

atom/common/native_mate_converters/content_converter.cc

@@ -180,4 +180,17 @@ v8::Local<v8::Value> Converter<content::WebContents*>::ToV8(
   return atom::api::WebContents::CreateFrom(isolate, val).ToV8();
 }
 
+// static
+bool Converter<content::WebContents*>::FromV8(
+    v8::Isolate* isolate,
+    v8::Local<v8::Value> val,
+    content::WebContents** out) {
+  atom::api::WebContents* web_contents = nullptr;
+  if (!ConvertFromV8(isolate, val, &web_contents) || !web_contents)
+    return false;
+
+  *out = web_contents->web_contents();
+  return true;
+}
+
 }  // namespace mate

atom/common/native_mate_converters/content_converter.h

@@ -57,6 +57,8 @@ template<>
 struct Converter<content::WebContents*> {
   static v8::Local<v8::Value> ToV8(v8::Isolate* isolate,
                                    content::WebContents* val);
+  static bool FromV8(v8::Isolate* isolate, v8::Local<v8::Value> val,
+                     content::WebContents** out);
 };
 
 }  // namespace mate

filenames.gypi

@@ -303,6 +303,10 @@
       'atom/common/api/locker.h',
       'atom/common/api/object_life_monitor.cc',
       'atom/common/api/object_life_monitor.h',
+      'atom/common/api/remote_callback_freer.cc',
+      'atom/common/api/remote_callback_freer.h',
+      'atom/common/api/remote_object_freer.cc',
+      'atom/common/api/remote_object_freer.h',
       'atom/common/asar/archive.cc',
       'atom/common/asar/archive.h',
       'atom/common/asar/asar_util.cc',

lib/browser/rpc-server.js

@@ -12,8 +12,19 @@ const FUNCTION_PROPERTIES = [
 ]
 
 // The remote functions in renderer processes.
-// (webContentsId) => {id: Function}
+// id => Function
-let rendererFunctions = {}
+let rendererFunctions = new IDWeakMap()
+
+// Merge two IDs together.
+let mergeIds = function (webContentsId, metaId) {
+  const PADDING_BITS = 20
+  if ((webContentsId << PADDING_BITS) < 0) {
+    throw new Error(`webContents ID is too large: ${webContentsId}`)
+  } else if (metaId > (1 << PADDING_BITS)) {
+    throw new Error(`Object ID is too large: ${metaId}`)
+  }
+  return (webContentsId << PADDING_BITS) + metaId
+}
 
 // Return the description of object's members:
 let getObjectMembers = function (object) {
@@ -165,32 +176,26 @@ var unwrapArgs = function (sender, args) {
           return returnValue
         }
       case 'function': {
+        // Merge webContentsId and meta.id, since meta.id can be the same in
+        // different webContents.
+        const webContentsId = sender.getId()
+        const objectId = mergeIds(webContentsId, meta.id)
+
         // Cache the callbacks in renderer.
-        let webContentsId = sender.getId()
+        if (rendererFunctions.has(objectId)) {
-        let callbacks = rendererFunctions[webContentsId]
+          return rendererFunctions.get(objectId)
-        if (!callbacks) {
-          callbacks = rendererFunctions[webContentsId] = new IDWeakMap()
-          sender.once('render-view-deleted', function (event, id) {
-            callbacks.clear()
-            delete rendererFunctions[id]
-          })
         }
 
-        if (callbacks.has(meta.id)) return callbacks.get(meta.id)
-
         let callIntoRenderer = function (...args) {
-          if ((webContentsId in rendererFunctions) && !sender.isDestroyed()) {
+          if (!sender.isDestroyed() && webContentsId === sender.getId()) {
             sender.send('ELECTRON_RENDERER_CALLBACK', meta.id, valueToMeta(sender, args))
           } else {
             throw new Error(`Attempting to call a function in a renderer window that has been closed or released. Function provided here: ${meta.location}.`)
           }
         }
-        v8Util.setDestructor(callIntoRenderer, function () {
+
-          if ((webContentsId in rendererFunctions) && !sender.isDestroyed()) {
+        v8Util.setRemoteCallbackFreer(callIntoRenderer, meta.id, sender)
-            sender.send('ELECTRON_RENDERER_RELEASE_CALLBACK', meta.id)
+        rendererFunctions.set(objectId, callIntoRenderer)
-          }
-        })
-        callbacks.set(meta.id, callIntoRenderer)
         return callIntoRenderer
       }
       default:

lib/renderer/api/remote.js

@@ -210,9 +210,7 @@ let metaToValue = function (meta) {
 
       // Track delegate object's life time, and tell the browser to clean up
       // when the object is GCed.
-      v8Util.setDestructor(ret, function () {
+      v8Util.setRemoteObjectFreer(ret, meta.id)
-        ipcRenderer.send('ELECTRON_BROWSER_DEREFERENCE', meta.id)
-      })
 
       // Remember object's id.
       v8Util.setHiddenValue(ret, 'atomId', meta.id)