From cb91d4487b2e961503f3e8e01dcc207f2ba58b7e Mon Sep 17 00:00:00 2001
From: Cheng Zhao <zcbenz@gmail.com>
Date: Sat, 31 Oct 2015 15:00:06 +0800
Subject: [PATCH] Clean up the code handling renderer callback

---
 atom/browser/lib/rpc-server.coffee            | 26 +++++++------------
 atom/common/api/atom_api_id_weak_map.cc       | 18 ++++++-------
 atom/common/api/atom_api_id_weak_map.h        |  6 ++---
 atom/common/api/lib/callbacks-registry.coffee |  7 ++---
 4 files changed, 24 insertions(+), 33 deletions(-)

diff --git a/atom/browser/lib/rpc-server.coffee b/atom/browser/lib/rpc-server.coffee
index eb97c72139..24ece2205c 100644
--- a/atom/browser/lib/rpc-server.coffee
+++ b/atom/browser/lib/rpc-server.coffee
@@ -4,9 +4,6 @@ objectsRegistry = require './objects-registry.js'
 v8Util = process.atomBinding 'v8_util'
 IDWeakMap = process.atomBinding('id_weak_map').IDWeakMap
 
-# Object mapping from webcontents id to their renderer callbacks weakmap.
-rendererRegistry = {}
-
 # Convert a real value into meta data.
 valueToMeta = (sender, value, optimizeSimpleObject=false) ->
   meta = type: typeof value
@@ -74,18 +71,15 @@ unwrapArgs = (sender, args) ->
         returnValue = metaToValue meta.value
         -> returnValue
       when 'function'
-        webContentsId = sender.getId()
-        rendererCallbacks = rendererRegistry[webContentsId]
-        if not rendererCallbacks?
-          # Weak reference to callbacks with their ID
-          rendererCallbacks = new IDWeakMap()
-          rendererRegistry[webContentsId] = rendererCallbacks
-
-        if rendererCallbacks.has(meta.id)
-          return rendererCallbacks.get(meta.id)
+        # Cache the callbacks in renderer.
+        unless sender.callbacks
+          sender.callbacks = new IDWeakMap
+          sender.on 'render-view-deleted', ->
+            sender.callbacks.clear()
+        return sender.callbacks.get meta.id if sender.callbacks.has meta.id
 
         rendererReleased = false
-        objectsRegistry.once "clear-#{webContentsId}", ->
+        objectsRegistry.once "clear-#{sender.getId()}", ->
           rendererReleased = true
 
         ret = ->
@@ -95,9 +89,9 @@ unwrapArgs = (sender, args) ->
           sender.send 'ATOM_RENDERER_CALLBACK', meta.id, valueToMeta(sender, arguments)
         v8Util.setDestructor ret, ->
           return if rendererReleased
-          rendererCallbacks.remove meta.id
+          sender.callbacks.remove meta.id
           sender.send 'ATOM_RENDERER_RELEASE_CALLBACK', meta.id
-        rendererCallbacks.set meta.id, ret
+        sender.callbacks.set meta.id, ret
         ret
       else throw new TypeError("Unknown type: #{meta.type}")
 
@@ -116,8 +110,6 @@ callFunction = (event, func, caller, args) ->
 
 # Send by BrowserWindow when its render view is deleted.
 process.on 'ATOM_BROWSER_RELEASE_RENDER_VIEW', (id) ->
-  if rendererRegistry.id?
-    delete rendererRegistry.id
   objectsRegistry.clear id
 
 ipc.on 'ATOM_BROWSER_REQUIRE', (event, module) ->
diff --git a/atom/common/api/atom_api_id_weak_map.cc b/atom/common/api/atom_api_id_weak_map.cc
index 8c17e8330c..f32e33682d 100644
--- a/atom/common/api/atom_api_id_weak_map.cc
+++ b/atom/common/api/atom_api_id_weak_map.cc
@@ -12,33 +12,32 @@ namespace atom {
 
 namespace api {
 
-IDWeakMap::IDWeakMap() : id_weak_map_(new atom::IDWeakMap) {
+IDWeakMap::IDWeakMap() {
 }
 
 IDWeakMap::~IDWeakMap() {
-  id_weak_map_ = nullptr;
 }
 
 void IDWeakMap::Set(v8::Isolate* isolate,
                     int32_t id,
                     v8::Local<v8::Object> object) {
-  id_weak_map_->Set(isolate, id, object);
+  id_weak_map_.Set(isolate, id, object);
 }
 
 v8::Local<v8::Object> IDWeakMap::Get(v8::Isolate* isolate, int32_t id) {
-  return id_weak_map_->Get(isolate, id).ToLocalChecked();
+  return id_weak_map_.Get(isolate, id).ToLocalChecked();
 }
 
 bool IDWeakMap::Has(int32_t id) {
-  return id_weak_map_->Has(id);
+  return id_weak_map_.Has(id);
 }
 
 void IDWeakMap::Remove(int32_t id) {
-  id_weak_map_->Remove(id);
+  id_weak_map_.Remove(id);
 }
 
-bool IDWeakMap::IsDestroyed() const {
-  return !id_weak_map_;
+void IDWeakMap::Clear() {
+  id_weak_map_.Clear();
 }
 
 // static
@@ -48,7 +47,8 @@ void IDWeakMap::BuildPrototype(v8::Isolate* isolate,
       .SetMethod("set", &IDWeakMap::Set)
       .SetMethod("get", &IDWeakMap::Get)
       .SetMethod("has", &IDWeakMap::Has)
-      .SetMethod("remove", &IDWeakMap::Remove);
+      .SetMethod("remove", &IDWeakMap::Remove)
+      .SetMethod("clear", &IDWeakMap::Clear);
 }
 
 // static
diff --git a/atom/common/api/atom_api_id_weak_map.h b/atom/common/api/atom_api_id_weak_map.h
index 4a2f8e397a..0cf656f455 100644
--- a/atom/common/api/atom_api_id_weak_map.h
+++ b/atom/common/api/atom_api_id_weak_map.h
@@ -24,17 +24,15 @@ class IDWeakMap : public mate::Wrappable {
   IDWeakMap();
   ~IDWeakMap();
 
-  // mate::Wrappable:
-  bool IsDestroyed() const override;
-
  private:
   // Api for IDWeakMap.
   void Set(v8::Isolate* isolate, int32_t id, v8::Local<v8::Object> object);
   v8::Local<v8::Object> Get(v8::Isolate* isolate, int32_t id);
   bool Has(int32_t id);
   void Remove(int32_t id);
+  void Clear();
 
-  atom::IDWeakMap* id_weak_map_;
+  atom::IDWeakMap id_weak_map_;
 
   DISALLOW_COPY_AND_ASSIGN(IDWeakMap);
 };
diff --git a/atom/common/api/lib/callbacks-registry.coffee b/atom/common/api/lib/callbacks-registry.coffee
index 001ecae14a..c546df34f9 100644
--- a/atom/common/api/lib/callbacks-registry.coffee
+++ b/atom/common/api/lib/callbacks-registry.coffee
@@ -7,8 +7,9 @@ class CallbacksRegistry
     @callbacks = {}
 
   add: (callback) ->
-    if v8Util.getHiddenValue(callback, 'metaId')?
-      return v8Util.getHiddenValue(callback, 'metaId')
+    # The callback is already added.
+    id = v8Util.getHiddenValue callback, 'callbackId'
+    return id if id?
 
     id = ++@nextId
 
@@ -25,7 +26,7 @@ class CallbacksRegistry
       break
 
     @callbacks[id] = callback
-    v8Util.setHiddenValue callback, 'metaId', id
+    v8Util.setHiddenValue callback, 'callbackId', id
     v8Util.setHiddenValue callback, 'location', filenameAndLine
     id