use idweakmap for holding callbacks in browser

atom/browser/lib/rpc-server.coffee

@@ -3,8 +3,8 @@ path = require 'path'
 objectsRegistry = require './objects-registry.js'
 v8Util = process.atomBinding 'v8_util'
 
-# caches callback with their registry ID.
+# weak refereence to callback with their registry ID.
-rendererCallbacks = {}
+rendererCallbacks = v8Util.createWeakMap()
 
 # Convert a real value into meta data.
 valueToMeta = (sender, value, optimizeSimpleObject=false) ->
@@ -77,18 +77,19 @@ unwrapArgs = (sender, args) ->
         objectsRegistry.once "clear-#{sender.getId()}", ->
           rendererReleased = true
 
-        return rendererCallbacks[meta.id] if rendererCallbacks[meta.id]?
+        if rendererCallbacks.has(meta.id)
+          return rendererCallbacks.get(meta.id)
 
         ret = ->
           if rendererReleased
             throw new Error("Attempting to call a function in a renderer window
-              that has been closed or released. Function provided here: #{meta.id}.")
+              that has been closed or released. Function provided here: #{meta.location}.")
           sender.send 'ATOM_RENDERER_CALLBACK', meta.id, valueToMeta(sender, arguments)
         v8Util.setDestructor ret, ->
           return if rendererReleased
-          delete rendererCallbacks[meta.id]
+          rendererCallbacks.remove meta.id
           sender.send 'ATOM_RENDERER_RELEASE_CALLBACK', meta.id
-        rendererCallbacks[meta.id] = ret
+        rendererCallbacks.set meta.id, ret
         ret
       else throw new TypeError("Unknown type: #{meta.type}")
 

atom/common/api/atom_api_v8_util.cc

@@ -3,7 +3,9 @@
 // found in the LICENSE file.
 
 #include "atom/common/api/object_life_monitor.h"
+#include "atom/common/id_weak_map.h"
 #include "atom/common/node_includes.h"
+#include "native_mate/handle.h"
 #include "native_mate/dictionary.h"
 #include "v8/include/v8-profiler.h"
 
@@ -46,6 +48,11 @@ void TakeHeapSnapshot(v8::Isolate* isolate) {
   isolate->GetHeapProfiler()->TakeHeapSnapshot();
 }
 
+mate::Handle<atom::IDWeakMap> CreateWeakMap(v8::Isolate* isolate) {
+  auto handle = mate::CreateHandle(isolate, new atom::IDWeakMap);
+  return handle;
+}
+
 void Initialize(v8::Local<v8::Object> exports, v8::Local<v8::Value> unused,
                 v8::Local<v8::Context> context, void* priv) {
   mate::Dictionary dict(context->GetIsolate(), exports);
@@ -56,6 +63,7 @@ void Initialize(v8::Local<v8::Object> exports, v8::Local<v8::Value> unused,
   dict.SetMethod("getObjectHash", &GetObjectHash);
   dict.SetMethod("setDestructor", &SetDestructor);
   dict.SetMethod("takeHeapSnapshot", &TakeHeapSnapshot);
+  dict.SetMethod("createWeakMap", &CreateWeakMap);
 }
 
 }  // namespace

atom/common/api/lib/callbacks-registry.coffee

@@ -1,3 +1,5 @@
+v8Util = process.atomBinding 'v8_util'
+
 module.exports =
 class CallbacksRegistry
   constructor: ->
@@ -7,10 +9,6 @@ class CallbacksRegistry
   add: (callback) ->
     id = ++@nextId
 
-    for id,value of @callbacks
-      if value == callback
-        return id
-
     # Capture the location of the function and put it in the ID string,
     # so that release errors can be tracked down easily.
     regexp = /at (.*)/gi
@@ -21,10 +19,14 @@ class CallbacksRegistry
       continue if location.indexOf('(native)') isnt -1
       continue if location.indexOf('atom.asar') isnt -1
       [x, filenameAndLine] = /([^/^\)]*)\)?$/gi.exec(location)
-      id = "#{filenameAndLine} (#{id})"
       break
 
+    if v8Util.getHiddenValue(callback, 'metaId')?
+      return v8Util.getHiddenValue(callback, 'metaId')
+
     @callbacks[id] = callback
+    v8Util.setHiddenValue callback, 'metaId', id
+    v8Util.setHiddenValue callback, 'location', filenameAndLine
     id
 
   get: (id) ->

atom/common/id_weak_map.cc

@@ -8,6 +8,18 @@
 
 #include "native_mate/converter.h"
 
+namespace mate {
+
+template<typename T>
+struct Converter<v8::MaybeLocal<T>> {
+  static v8::Local<v8::Value> ToV8(v8::Isolate* isolate,
+                                   v8::MaybeLocal<T> val) {
+    return ConvertToV8(isolate, val.ToLocalChecked());
+  }
+};
+
+}  // namespace mate
+
 namespace atom {
 
 namespace {
@@ -41,6 +53,15 @@ int32_t IDWeakMap::Add(v8::Isolate* isolate, v8::Local<v8::Object> object) {
   return id;
 }
 
+void IDWeakMap::Set(v8::Isolate* isolate,
+                    int32_t id,
+                    v8::Local<v8::Object> object) {
+  auto global = make_linked_ptr(new v8::Global<v8::Object>(isolate, object));
+  ObjectKey* key = new ObjectKey(id, this);
+  global->SetWeak(key, OnObjectGC, v8::WeakCallbackType::kParameter);
+  map_[id] = global;
+}
+
 v8::MaybeLocal<v8::Object> IDWeakMap::Get(v8::Isolate* isolate, int32_t id) {
   auto iter = map_.find(id);
   if (iter == map_.end())
@@ -85,4 +106,13 @@ int32_t IDWeakMap::GetNextID() {
   return ++next_id_;
 }
 
+mate::ObjectTemplateBuilder IDWeakMap::GetObjectTemplateBuilder(
+    v8::Isolate* isolate) {
+  return mate::ObjectTemplateBuilder(isolate)
+      .SetMethod("set", &IDWeakMap::Set)
+      .SetMethod("get", &IDWeakMap::Get)
+      .SetMethod("has", &IDWeakMap::Has)
+      .SetMethod("remove", &IDWeakMap::Remove);
+}
+
 }  // namespace atom

atom/common/id_weak_map.h

@@ -9,12 +9,14 @@
 #include <vector>
 
 #include "base/memory/linked_ptr.h"
+#include "native_mate/object_template_builder.h"
+#include "native_mate/wrappable.h"
 #include "v8/include/v8.h"
 
 namespace atom {
 
 // Like ES6's WeakMap, but the key is Integer and the value is Weak Pointer.
-class IDWeakMap {
+class IDWeakMap : public mate::Wrappable {
  public:
   IDWeakMap();
   ~IDWeakMap();
@@ -22,6 +24,9 @@ class IDWeakMap {
   // Adds |object| to WeakMap and returns its allocated |id|.
   int32_t Add(v8::Isolate* isolate, v8::Local<v8::Object> object);
 
+  // Sets the object to WeakMap with the given |id|.
+  void Set(v8::Isolate* isolate, int32_t id, v8::Local<v8::Object> object);
+
   // Gets the object from WeakMap by its |id|.
   v8::MaybeLocal<v8::Object> Get(v8::Isolate* isolate, int32_t id);
 
@@ -40,6 +45,10 @@ class IDWeakMap {
   // Clears the weak map.
   void Clear();
 
+ protected:
+  mate::ObjectTemplateBuilder GetObjectTemplateBuilder(
+      v8::Isolate* isolate) override;
+
  private:
   // Returns next available ID.
   int32_t GetNextID();

atom/renderer/api/lib/remote.coffee

@@ -33,7 +33,7 @@ wrapArgs = (args, visited=[]) ->
     else if typeof value is 'function' and v8Util.getHiddenValue value, 'returnValue'
       type: 'function-with-return-value', value: valueToMeta(value())
     else if typeof value is 'function'
-      type: 'function', id: callbacksRegistry.add(value)
+      type: 'function', id: callbacksRegistry.add(value), location: v8Util.getHiddenValue value, 'location'
     else
       type: 'value', value: value