fix double-freeing remote references

After the page does navigations, garbage collection can still happen in
the old context. This commit changes to store references to remote objects
by _pages_, instead of by _WebContents_.

atom/common/api/atom_api_v8_util.cc

@@ -32,7 +32,7 @@ namespace std {
 template <typename Type1, typename Type2>
 struct hash<std::pair<Type1, Type2>> {
   std::size_t operator()(std::pair<Type1, Type2> value) const {
-    return base::HashInts<Type1, Type2>(value.first, value.second);
+    return base::HashInts(base::Hash(value.first), value.second);
   }
 };
 
@@ -137,8 +137,9 @@ void Initialize(v8::Local<v8::Object> exports,
   dict.SetMethod("setRemoteCallbackFreer", &atom::RemoteCallbackFreer::BindTo);
   dict.SetMethod("setRemoteObjectFreer", &atom::RemoteObjectFreer::BindTo);
   dict.SetMethod("createIDWeakMap", &atom::api::KeyWeakMap<int32_t>::Create);
-  dict.SetMethod("createDoubleIDWeakMap",
-                 &atom::api::KeyWeakMap<std::pair<int64_t, int32_t>>::Create);
+  dict.SetMethod(
+      "createDoubleIDWeakMap",
+      &atom::api::KeyWeakMap<std::pair<std::string, int32_t>>::Create);
   dict.SetMethod("requestGarbageCollectionForTesting",
                  &RequestGarbageCollectionForTesting);
   dict.SetMethod("isSameOrigin", &IsSameOrigin);

atom/common/api/remote_callback_freer.cc

@@ -15,17 +15,20 @@ namespace atom {
 // static
 void RemoteCallbackFreer::BindTo(v8::Isolate* isolate,
                                  v8::Local<v8::Object> target,
+                                 const std::string& context_id,
                                  int object_id,
                                  content::WebContents* web_contents) {
-  new RemoteCallbackFreer(isolate, target, object_id, web_contents);
+  new RemoteCallbackFreer(isolate, target, context_id, object_id, web_contents);
 }
 
 RemoteCallbackFreer::RemoteCallbackFreer(v8::Isolate* isolate,
                                          v8::Local<v8::Object> target,
+                                         const std::string& context_id,
                                          int object_id,
                                          content::WebContents* web_contents)
     : ObjectLifeMonitor(isolate, target),
       content::WebContentsObserver(web_contents),
+      context_id_(context_id),
       object_id_(object_id) {}
 
 RemoteCallbackFreer::~RemoteCallbackFreer() {}
@@ -34,6 +37,7 @@ void RemoteCallbackFreer::RunDestructor() {
   base::string16 channel =
       base::ASCIIToUTF16("ELECTRON_RENDERER_RELEASE_CALLBACK");
   base::ListValue args;
+  args.AppendString(context_id_);
   args.AppendInteger(object_id_);
   auto* frame_host = web_contents()->GetMainFrame();
   if (frame_host) {

atom/common/api/remote_callback_freer.h

@@ -4,6 +4,9 @@
 
 #ifndef ATOM_COMMON_API_REMOTE_CALLBACK_FREER_H_
 #define ATOM_COMMON_API_REMOTE_CALLBACK_FREER_H_
+
+#include <string>
+
 #include "atom/common/api/object_life_monitor.h"
 #include "content/public/browser/web_contents_observer.h"
 
@@ -14,12 +17,14 @@ class RemoteCallbackFreer : public ObjectLifeMonitor,
  public:
   static void BindTo(v8::Isolate* isolate,
                      v8::Local<v8::Object> target,
+                     const std::string& context_id,
                      int object_id,
                      content::WebContents* web_conents);
 
  protected:
   RemoteCallbackFreer(v8::Isolate* isolate,
                       v8::Local<v8::Object> target,
+                      const std::string& context_id,
                       int object_id,
                       content::WebContents* web_conents);
   ~RemoteCallbackFreer() override;
@@ -30,6 +35,7 @@ class RemoteCallbackFreer : public ObjectLifeMonitor,
   void RenderViewDeleted(content::RenderViewHost*) override;
 
  private:
+  std::string context_id_;
   int object_id_;
 
   DISALLOW_COPY_AND_ASSIGN(RemoteCallbackFreer);

atom/common/api/remote_object_freer.cc

@@ -29,14 +29,17 @@ content::RenderFrame* GetCurrentRenderFrame() {
 // static
 void RemoteObjectFreer::BindTo(v8::Isolate* isolate,
                                v8::Local<v8::Object> target,
+                               const std::string& context_id,
                                int object_id) {
-  new RemoteObjectFreer(isolate, target, 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();
@@ -56,6 +59,7 @@ void RemoteObjectFreer::RunDestructor() {
   base::string16 channel = base::ASCIIToUTF16("ipc-message");
   base::ListValue args;
   args.AppendString("ELECTRON_BROWSER_DEREFERENCE");
+  args.AppendString(context_id_);
   args.AppendInteger(object_id_);
   render_frame->Send(new AtomFrameHostMsg_Message(render_frame->GetRoutingID(),
                                                   channel, args));

atom/common/api/remote_object_freer.h

@@ -5,6 +5,8 @@
 #ifndef ATOM_COMMON_API_REMOTE_OBJECT_FREER_H_
 #define ATOM_COMMON_API_REMOTE_OBJECT_FREER_H_
 
+#include <string>
+
 #include "atom/common/api/object_life_monitor.h"
 
 namespace atom {
@@ -13,17 +15,20 @@ 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_;
 

lib/browser/objects-registry.js

@@ -17,16 +17,15 @@ class ObjectsRegistry {
 
   // Register a new object and return its assigned ID. If the object is already
   // registered then the already assigned ID would be returned.
-  add (webContents, obj) {
+  add (webContents, contextId, obj) {
     // Get or assign an ID to the object.
     const id = this.saveToStorage(obj)
 
     // Add object to the set of referenced objects.
-    const webContentsId = webContents.getId()
-    let owner = this.owners[webContentsId]
+    let owner = this.owners[contextId]
     if (!owner) {
-      owner = this.owners[webContentsId] = new Set()
-      this.registerDeleteListener(webContents, webContentsId)
+      owner = this.owners[contextId] = new Set()
+      this.registerDeleteListener(webContents, contextId)
     }
     if (!owner.has(id)) {
       owner.add(id)
@@ -43,25 +42,26 @@ class ObjectsRegistry {
   }
 
   // Dereference an object according to its ID.
-  remove (webContentsId, id) {
-    // Dereference from the storage.
-    this.dereference(id)
-
-    // Also remove the reference in owner.
-    let owner = this.owners[webContentsId]
+  // Note that an object may be double-freed (cleared when page is reloaded, and
+  // then garbage collected in old page).
+  remove (contextId, id) {
+    let owner = this.owners[contextId]
     if (owner) {
+      // Remove the reference in owner.
       owner.delete(id)
+      // Dereference from the storage.
+      this.dereference(id)
     }
   }
 
   // Clear all references to objects refrenced by the WebContents.
-  clear (webContentsId) {
-    let owner = this.owners[webContentsId]
+  clear (contextId) {
+    let owner = this.owners[contextId]
     if (!owner) return
 
     for (let id of owner) this.dereference(id)
 
-    delete this.owners[webContentsId]
+    delete this.owners[contextId]
   }
 
   // Private: Saves the object into storage and assigns an ID for it.
@@ -92,12 +92,12 @@ class ObjectsRegistry {
   }
 
   // Private: Clear the storage when webContents is reloaded/navigated.
-  registerDeleteListener (webContents, webContentsId) {
+  registerDeleteListener (webContents, contextId) {
     const processId = webContents.getProcessId()
     const listener = (event, deletedProcessId) => {
       if (deletedProcessId === processId) {
         webContents.removeListener('render-view-deleted', listener)
-        this.clear(webContentsId)
+        this.clear(contextId)
       }
     }
     webContents.on('render-view-deleted', listener)