From 62d15953ffea6932622c771daba56b408a311fee Mon Sep 17 00:00:00 2001 From: Robo Date: Wed, 28 Oct 2015 03:45:46 +0530 Subject: [PATCH 1/5] remote: track listeners for browser side --- atom/browser/lib/rpc-server.coffee | 7 +++++++ atom/common/api/lib/callbacks-registry.coffee | 4 ++++ spec/api-ipc-spec.coffee | 13 +++++++++++++ 3 files changed, 24 insertions(+) diff --git a/atom/browser/lib/rpc-server.coffee b/atom/browser/lib/rpc-server.coffee index 6b1f95a841ab..c4d91830b5f1 100644 --- a/atom/browser/lib/rpc-server.coffee +++ b/atom/browser/lib/rpc-server.coffee @@ -3,6 +3,9 @@ path = require 'path' objectsRegistry = require './objects-registry.js' v8Util = process.atomBinding 'v8_util' +# caches callback with their registry ID. +rendererCallbacks = {} + # Convert a real value into meta data. valueToMeta = (sender, value, optimizeSimpleObject=false) -> meta = type: typeof value @@ -74,6 +77,8 @@ unwrapArgs = (sender, args) -> objectsRegistry.once "clear-#{sender.getId()}", -> rendererReleased = true + return rendererCallbacks[meta.id] if rendererCallbacks[meta.id]? + ret = -> if rendererReleased throw new Error("Attempting to call a function in a renderer window @@ -81,7 +86,9 @@ unwrapArgs = (sender, args) -> sender.send 'ATOM_RENDERER_CALLBACK', meta.id, valueToMeta(sender, arguments) v8Util.setDestructor ret, -> return if rendererReleased + delete rendererCallbacks[meta.id] sender.send 'ATOM_RENDERER_RELEASE_CALLBACK', meta.id + rendererCallbacks[meta.id] = ret ret else throw new TypeError("Unknown type: #{meta.type}") diff --git a/atom/common/api/lib/callbacks-registry.coffee b/atom/common/api/lib/callbacks-registry.coffee index d4c37f087b1e..5151640b2b80 100644 --- a/atom/common/api/lib/callbacks-registry.coffee +++ b/atom/common/api/lib/callbacks-registry.coffee @@ -7,6 +7,10 @@ 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 diff --git a/spec/api-ipc-spec.coffee b/spec/api-ipc-spec.coffee index 142f06c00ff3..1155aa73e83d 100644 --- a/spec/api-ipc-spec.coffee +++ b/spec/api-ipc-spec.coffee @@ -88,3 +88,16 @@ describe 'ipc module', -> w.destroy() done() w.loadUrl 'file://' + path.join(fixtures, 'api', 'send-sync-message.html') + + describe 'remote listeners', -> + it 'can be added and removed correctly', -> + count = 0 + w = new BrowserWindow(show: false) + listener = () -> + count += 1 + w.removeListener 'blur', listener + w.on 'blur', listener + w.emit 'blur' + w.emit 'blur' + assert.equal count, 1 + w.destroy() From eae7c840b7ce8a954c67efb60553102dcde9f106 Mon Sep 17 00:00:00 2001 From: Robo Date: Wed, 28 Oct 2015 21:29:46 +0530 Subject: [PATCH 2/5] use idweakmap for holding callbacks in browser --- atom/browser/lib/rpc-server.coffee | 13 ++++---- atom/common/api/atom_api_v8_util.cc | 8 +++++ atom/common/api/lib/callbacks-registry.coffee | 12 ++++---- atom/common/id_weak_map.cc | 30 +++++++++++++++++++ atom/common/id_weak_map.h | 11 ++++++- atom/renderer/api/lib/remote.coffee | 2 +- 6 files changed, 63 insertions(+), 13 deletions(-) diff --git a/atom/browser/lib/rpc-server.coffee b/atom/browser/lib/rpc-server.coffee index c4d91830b5f1..13c8ceb16e03 100644 --- a/atom/browser/lib/rpc-server.coffee +++ b/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. -rendererCallbacks = {} +# weak refereence to callback with their registry ID. +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}") diff --git a/atom/common/api/atom_api_v8_util.cc b/atom/common/api/atom_api_v8_util.cc index bba3399a8dbd..4a321f84d80e 100644 --- a/atom/common/api/atom_api_v8_util.cc +++ b/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 CreateWeakMap(v8::Isolate* isolate) { + auto handle = mate::CreateHandle(isolate, new atom::IDWeakMap); + return handle; +} + void Initialize(v8::Local exports, v8::Local unused, v8::Local context, void* priv) { mate::Dictionary dict(context->GetIsolate(), exports); @@ -56,6 +63,7 @@ void Initialize(v8::Local exports, v8::Local unused, dict.SetMethod("getObjectHash", &GetObjectHash); dict.SetMethod("setDestructor", &SetDestructor); dict.SetMethod("takeHeapSnapshot", &TakeHeapSnapshot); + dict.SetMethod("createWeakMap", &CreateWeakMap); } } // namespace diff --git a/atom/common/api/lib/callbacks-registry.coffee b/atom/common/api/lib/callbacks-registry.coffee index 5151640b2b80..b777f7a462df 100644 --- a/atom/common/api/lib/callbacks-registry.coffee +++ b/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) -> diff --git a/atom/common/id_weak_map.cc b/atom/common/id_weak_map.cc index c5c4b60cac5c..3d7e1513597c 100644 --- a/atom/common/id_weak_map.cc +++ b/atom/common/id_weak_map.cc @@ -8,6 +8,18 @@ #include "native_mate/converter.h" +namespace mate { + +template +struct Converter> { + static v8::Local ToV8(v8::Isolate* isolate, + v8::MaybeLocal val) { + return ConvertToV8(isolate, val.ToLocalChecked()); + } +}; + +} // namespace mate + namespace atom { namespace { @@ -41,6 +53,15 @@ int32_t IDWeakMap::Add(v8::Isolate* isolate, v8::Local object) { return id; } +void IDWeakMap::Set(v8::Isolate* isolate, + int32_t id, + v8::Local object) { + auto global = make_linked_ptr(new v8::Global(isolate, object)); + ObjectKey* key = new ObjectKey(id, this); + global->SetWeak(key, OnObjectGC, v8::WeakCallbackType::kParameter); + map_[id] = global; +} + v8::MaybeLocal 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 diff --git a/atom/common/id_weak_map.h b/atom/common/id_weak_map.h index 9fe71ebb616f..c291f76a430e 100644 --- a/atom/common/id_weak_map.h +++ b/atom/common/id_weak_map.h @@ -9,12 +9,14 @@ #include #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 object); + // Sets the object to WeakMap with the given |id|. + void Set(v8::Isolate* isolate, int32_t id, v8::Local object); + // Gets the object from WeakMap by its |id|. v8::MaybeLocal 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(); diff --git a/atom/renderer/api/lib/remote.coffee b/atom/renderer/api/lib/remote.coffee index 8a5565f06562..b5a3a694ee37 100644 --- a/atom/renderer/api/lib/remote.coffee +++ b/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 From ac4df34ecd8a306e5cf71fd6696fae4362f3e515 Mon Sep 17 00:00:00 2001 From: Robo Date: Thu, 29 Oct 2015 16:27:30 +0530 Subject: [PATCH 3/5] create binding to idweakmap --- atom/browser/lib/rpc-server.coffee | 11 ++-- atom/common/api/atom_api_v8_util.cc | 7 --- atom/common/api/atom_api_weak_map.cc | 79 ++++++++++++++++++++++++++++ atom/common/api/atom_api_weak_map.h | 46 ++++++++++++++++ atom/common/id_weak_map.cc | 21 -------- atom/common/id_weak_map.h | 8 +-- atom/common/node_bindings.cc | 1 + filenames.gypi | 2 + 8 files changed, 135 insertions(+), 40 deletions(-) create mode 100644 atom/common/api/atom_api_weak_map.cc create mode 100644 atom/common/api/atom_api_weak_map.h diff --git a/atom/browser/lib/rpc-server.coffee b/atom/browser/lib/rpc-server.coffee index 13c8ceb16e03..3a4f896da5ec 100644 --- a/atom/browser/lib/rpc-server.coffee +++ b/atom/browser/lib/rpc-server.coffee @@ -2,9 +2,10 @@ ipc = require 'ipc' path = require 'path' objectsRegistry = require './objects-registry.js' v8Util = process.atomBinding 'v8_util' +WeakMap = process.atomBinding('weak_map').WeakMap -# weak refereence to callback with their registry ID. -rendererCallbacks = v8Util.createWeakMap() +# Weak reference to callback with their registry ID. +rendererCallbacks = new WeakMap() # Convert a real value into meta data. valueToMeta = (sender, value, optimizeSimpleObject=false) -> @@ -73,13 +74,13 @@ unwrapArgs = (sender, args) -> returnValue = metaToValue meta.value -> returnValue when 'function' + if rendererCallbacks.has(meta.id) + return rendererCallbacks.get(meta.id) + rendererReleased = false objectsRegistry.once "clear-#{sender.getId()}", -> rendererReleased = true - 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 diff --git a/atom/common/api/atom_api_v8_util.cc b/atom/common/api/atom_api_v8_util.cc index 4a321f84d80e..1c8c8c9f7e09 100644 --- a/atom/common/api/atom_api_v8_util.cc +++ b/atom/common/api/atom_api_v8_util.cc @@ -5,7 +5,6 @@ #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" @@ -48,11 +47,6 @@ void TakeHeapSnapshot(v8::Isolate* isolate) { isolate->GetHeapProfiler()->TakeHeapSnapshot(); } -mate::Handle CreateWeakMap(v8::Isolate* isolate) { - auto handle = mate::CreateHandle(isolate, new atom::IDWeakMap); - return handle; -} - void Initialize(v8::Local exports, v8::Local unused, v8::Local context, void* priv) { mate::Dictionary dict(context->GetIsolate(), exports); @@ -63,7 +57,6 @@ void Initialize(v8::Local exports, v8::Local unused, dict.SetMethod("getObjectHash", &GetObjectHash); dict.SetMethod("setDestructor", &SetDestructor); dict.SetMethod("takeHeapSnapshot", &TakeHeapSnapshot); - dict.SetMethod("createWeakMap", &CreateWeakMap); } } // namespace diff --git a/atom/common/api/atom_api_weak_map.cc b/atom/common/api/atom_api_weak_map.cc new file mode 100644 index 000000000000..6cc75c43c927 --- /dev/null +++ b/atom/common/api/atom_api_weak_map.cc @@ -0,0 +1,79 @@ +// Copyright (c) 2015 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/atom_api_weak_map.h" + +#include "atom/common/node_includes.h" +#include "native_mate/constructor.h" +#include "native_mate/dictionary.h" + +namespace atom { + +namespace api { + +WeakMap::WeakMap() { + id_weak_map_.reset(new atom::IDWeakMap); +} + +WeakMap::~WeakMap() { +} + +void WeakMap::Set(v8::Isolate* isolate, + int32_t id, + v8::Local object) { + id_weak_map_->Set(isolate, id, object); +} + +v8::Local WeakMap::Get(v8::Isolate* isolate, int32_t id) { + return id_weak_map_->Get(isolate, id).ToLocalChecked(); +} + +bool WeakMap::Has(int32_t id) { + return id_weak_map_->Has(id); +} + +void WeakMap::Remove(int32_t id) { + id_weak_map_->Remove(id); +} + +bool WeakMap::IsDestroyed() const { + return !id_weak_map_; +} + +// static +void WeakMap::BuildPrototype(v8::Isolate* isolate, + v8::Local prototype) { + mate::ObjectTemplateBuilder(isolate, prototype) + .SetMethod("set", &WeakMap::Set) + .SetMethod("get", &WeakMap::Get) + .SetMethod("has", &WeakMap::Has) + .SetMethod("remove", &WeakMap::Remove); +} + +// static +mate::Wrappable* WeakMap::Create(v8::Isolate* isolate) { + return new WeakMap; +} + +} // namespace api + +} // namespace atom + +namespace { + +using atom::api::WeakMap; + +void Initialize(v8::Local exports, v8::Local unused, + v8::Local context, void* priv) { + v8::Isolate* isolate = context->GetIsolate(); + v8::Local constructor = mate::CreateConstructor( + isolate, "WeakMap", base::Bind(&WeakMap::Create)); + mate::Dictionary weak_map(isolate, constructor); + mate::Dictionary dict(isolate, exports); + dict.Set("WeakMap", weak_map); +} + +} // namespace + +NODE_MODULE_CONTEXT_AWARE_BUILTIN(atom_common_weak_map, Initialize) diff --git a/atom/common/api/atom_api_weak_map.h b/atom/common/api/atom_api_weak_map.h new file mode 100644 index 000000000000..7c747e1a384e --- /dev/null +++ b/atom/common/api/atom_api_weak_map.h @@ -0,0 +1,46 @@ +// Copyright (c) 2015 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_ATOM_API_WEAK_MAP_H_ +#define ATOM_COMMON_API_ATOM_API_WEAK_MAP_H_ + +#include "atom/common/id_weak_map.h" +#include "native_mate/object_template_builder.h" +#include "native_mate/handle.h" + +namespace atom { + +namespace api { + +class WeakMap : public mate::Wrappable { + public: + static mate::Wrappable* Create(v8::Isolate* isolate); + + static void BuildPrototype(v8::Isolate* isolate, + v8::Local prototype); + + protected: + WeakMap(); + virtual ~WeakMap(); + + // mate::Wrappable: + bool IsDestroyed() const override; + + private: + // Api for IDWeakMap. + void Set(v8::Isolate* isolate, int32_t id, v8::Local object); + v8::Local Get(v8::Isolate* isolate, int32_t id); + bool Has(int32_t id); + void Remove(int32_t id); + + scoped_ptr id_weak_map_; + + DISALLOW_COPY_AND_ASSIGN(WeakMap); +}; + +} // namespace api + +} // namespace atom + +#endif // ATOM_COMMON_API_ATOM_API_WEAK_MAP_H_ diff --git a/atom/common/id_weak_map.cc b/atom/common/id_weak_map.cc index 3d7e1513597c..7fda009e0ee1 100644 --- a/atom/common/id_weak_map.cc +++ b/atom/common/id_weak_map.cc @@ -8,18 +8,6 @@ #include "native_mate/converter.h" -namespace mate { - -template -struct Converter> { - static v8::Local ToV8(v8::Isolate* isolate, - v8::MaybeLocal val) { - return ConvertToV8(isolate, val.ToLocalChecked()); - } -}; - -} // namespace mate - namespace atom { namespace { @@ -106,13 +94,4 @@ 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 diff --git a/atom/common/id_weak_map.h b/atom/common/id_weak_map.h index c291f76a430e..688e85cd0cce 100644 --- a/atom/common/id_weak_map.h +++ b/atom/common/id_weak_map.h @@ -9,14 +9,12 @@ #include #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 : public mate::Wrappable { +class IDWeakMap { public: IDWeakMap(); ~IDWeakMap(); @@ -45,10 +43,6 @@ class IDWeakMap : public mate::Wrappable { // Clears the weak map. void Clear(); - protected: - mate::ObjectTemplateBuilder GetObjectTemplateBuilder( - v8::Isolate* isolate) override; - private: // Returns next available ID. int32_t GetNextID(); diff --git a/atom/common/node_bindings.cc b/atom/common/node_bindings.cc index 2da68854ad14..b6fcdb82f819 100644 --- a/atom/common/node_bindings.cc +++ b/atom/common/node_bindings.cc @@ -52,6 +52,7 @@ REFERENCE_MODULE(atom_common_native_image); REFERENCE_MODULE(atom_common_screen); REFERENCE_MODULE(atom_common_shell); REFERENCE_MODULE(atom_common_v8_util); +REFERENCE_MODULE(atom_common_weak_map); REFERENCE_MODULE(atom_renderer_ipc); REFERENCE_MODULE(atom_renderer_web_frame); #undef REFERENCE_MODULE diff --git a/filenames.gypi b/filenames.gypi index f66485edd19d..0f5b794a0361 100644 --- a/filenames.gypi +++ b/filenames.gypi @@ -260,6 +260,8 @@ 'atom/common/api/atom_api_native_image_mac.mm', 'atom/common/api/atom_api_shell.cc', 'atom/common/api/atom_api_v8_util.cc', + 'atom/common/api/atom_api_weak_map.cc', + 'atom/common/api/atom_api_weak_map.h', 'atom/common/api/atom_bindings.cc', 'atom/common/api/atom_bindings.h', 'atom/common/api/event_emitter_caller.cc', From 3a154ab8ead41151977ef857490610a895f5ee87 Mon Sep 17 00:00:00 2001 From: Robo Date: Thu, 29 Oct 2015 16:52:12 +0530 Subject: [PATCH 4/5] add line and column values to callback id --- atom/browser/lib/rpc-server.coffee | 4 +- atom/common/api/atom_api_id_weak_map.cc | 79 +++++++++++++++++++ ..._api_weak_map.h => atom_api_id_weak_map.h} | 16 ++-- atom/common/api/atom_api_v8_util.cc | 1 - atom/common/api/atom_api_weak_map.cc | 79 ------------------- atom/common/api/lib/callbacks-registry.coffee | 8 +- atom/common/node_bindings.cc | 2 +- filenames.gypi | 4 +- 8 files changed, 97 insertions(+), 96 deletions(-) create mode 100644 atom/common/api/atom_api_id_weak_map.cc rename atom/common/api/{atom_api_weak_map.h => atom_api_id_weak_map.h} (72%) delete mode 100644 atom/common/api/atom_api_weak_map.cc diff --git a/atom/browser/lib/rpc-server.coffee b/atom/browser/lib/rpc-server.coffee index 3a4f896da5ec..40e1b0e46ac7 100644 --- a/atom/browser/lib/rpc-server.coffee +++ b/atom/browser/lib/rpc-server.coffee @@ -2,10 +2,10 @@ ipc = require 'ipc' path = require 'path' objectsRegistry = require './objects-registry.js' v8Util = process.atomBinding 'v8_util' -WeakMap = process.atomBinding('weak_map').WeakMap +IDWeakMap = process.atomBinding('id_weak_map').IDWeakMap # Weak reference to callback with their registry ID. -rendererCallbacks = new WeakMap() +rendererCallbacks = new IDWeakMap() # Convert a real value into meta data. valueToMeta = (sender, value, optimizeSimpleObject=false) -> diff --git a/atom/common/api/atom_api_id_weak_map.cc b/atom/common/api/atom_api_id_weak_map.cc new file mode 100644 index 000000000000..bdc298fa0c49 --- /dev/null +++ b/atom/common/api/atom_api_id_weak_map.cc @@ -0,0 +1,79 @@ +// Copyright (c) 2015 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/atom_api_id_weak_map.h" + +#include "atom/common/node_includes.h" +#include "native_mate/constructor.h" +#include "native_mate/dictionary.h" + +namespace atom { + +namespace api { + +IDWeakMap::IDWeakMap() { + id_weak_map_.reset(new atom::IDWeakMap); +} + +IDWeakMap::~IDWeakMap() { +} + +void IDWeakMap::Set(v8::Isolate* isolate, + int32_t id, + v8::Local object) { + id_weak_map_->Set(isolate, id, object); +} + +v8::Local IDWeakMap::Get(v8::Isolate* isolate, int32_t id) { + return id_weak_map_->Get(isolate, id).ToLocalChecked(); +} + +bool IDWeakMap::Has(int32_t id) { + return id_weak_map_->Has(id); +} + +void IDWeakMap::Remove(int32_t id) { + id_weak_map_->Remove(id); +} + +bool IDWeakMap::IsDestroyed() const { + return !id_weak_map_; +} + +// static +void IDWeakMap::BuildPrototype(v8::Isolate* isolate, + v8::Local prototype) { + mate::ObjectTemplateBuilder(isolate, prototype) + .SetMethod("set", &IDWeakMap::Set) + .SetMethod("get", &IDWeakMap::Get) + .SetMethod("has", &IDWeakMap::Has) + .SetMethod("remove", &IDWeakMap::Remove); +} + +// static +mate::Wrappable* IDWeakMap::Create(v8::Isolate* isolate) { + return new IDWeakMap; +} + +} // namespace api + +} // namespace atom + +namespace { + +using atom::api::IDWeakMap; + +void Initialize(v8::Local exports, v8::Local unused, + v8::Local context, void* priv) { + v8::Isolate* isolate = context->GetIsolate(); + v8::Local constructor = mate::CreateConstructor( + isolate, "IDWeakMap", base::Bind(&IDWeakMap::Create)); + mate::Dictionary id_weak_map(isolate, constructor); + mate::Dictionary dict(isolate, exports); + dict.Set("IDWeakMap", id_weak_map); +} + +} // namespace + +NODE_MODULE_CONTEXT_AWARE_BUILTIN(atom_common_id_weak_map, Initialize) diff --git a/atom/common/api/atom_api_weak_map.h b/atom/common/api/atom_api_id_weak_map.h similarity index 72% rename from atom/common/api/atom_api_weak_map.h rename to atom/common/api/atom_api_id_weak_map.h index 7c747e1a384e..3acdddc9c75f 100644 --- a/atom/common/api/atom_api_weak_map.h +++ b/atom/common/api/atom_api_id_weak_map.h @@ -2,8 +2,8 @@ // Use of this source code is governed by the MIT license that can be // found in the LICENSE file. -#ifndef ATOM_COMMON_API_ATOM_API_WEAK_MAP_H_ -#define ATOM_COMMON_API_ATOM_API_WEAK_MAP_H_ +#ifndef ATOM_COMMON_API_ATOM_API_ID_WEAK_MAP_H_ +#define ATOM_COMMON_API_ATOM_API_ID_WEAK_MAP_H_ #include "atom/common/id_weak_map.h" #include "native_mate/object_template_builder.h" @@ -13,7 +13,7 @@ namespace atom { namespace api { -class WeakMap : public mate::Wrappable { +class IDWeakMap : public mate::Wrappable { public: static mate::Wrappable* Create(v8::Isolate* isolate); @@ -21,8 +21,8 @@ class WeakMap : public mate::Wrappable { v8::Local prototype); protected: - WeakMap(); - virtual ~WeakMap(); + IDWeakMap(); + virtual ~IDWeakMap(); // mate::Wrappable: bool IsDestroyed() const override; @@ -34,13 +34,13 @@ class WeakMap : public mate::Wrappable { bool Has(int32_t id); void Remove(int32_t id); - scoped_ptr id_weak_map_; + scoped_ptr id_weak_map_; - DISALLOW_COPY_AND_ASSIGN(WeakMap); + DISALLOW_COPY_AND_ASSIGN(IDWeakMap); }; } // namespace api } // namespace atom -#endif // ATOM_COMMON_API_ATOM_API_WEAK_MAP_H_ +#endif // ATOM_COMMON_API_ATOM_API_ID_WEAK_MAP_H_ diff --git a/atom/common/api/atom_api_v8_util.cc b/atom/common/api/atom_api_v8_util.cc index 1c8c8c9f7e09..bba3399a8dbd 100644 --- a/atom/common/api/atom_api_v8_util.cc +++ b/atom/common/api/atom_api_v8_util.cc @@ -3,7 +3,6 @@ // 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/dictionary.h" #include "v8/include/v8-profiler.h" diff --git a/atom/common/api/atom_api_weak_map.cc b/atom/common/api/atom_api_weak_map.cc deleted file mode 100644 index 6cc75c43c927..000000000000 --- a/atom/common/api/atom_api_weak_map.cc +++ /dev/null @@ -1,79 +0,0 @@ -// Copyright (c) 2015 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/atom_api_weak_map.h" - -#include "atom/common/node_includes.h" -#include "native_mate/constructor.h" -#include "native_mate/dictionary.h" - -namespace atom { - -namespace api { - -WeakMap::WeakMap() { - id_weak_map_.reset(new atom::IDWeakMap); -} - -WeakMap::~WeakMap() { -} - -void WeakMap::Set(v8::Isolate* isolate, - int32_t id, - v8::Local object) { - id_weak_map_->Set(isolate, id, object); -} - -v8::Local WeakMap::Get(v8::Isolate* isolate, int32_t id) { - return id_weak_map_->Get(isolate, id).ToLocalChecked(); -} - -bool WeakMap::Has(int32_t id) { - return id_weak_map_->Has(id); -} - -void WeakMap::Remove(int32_t id) { - id_weak_map_->Remove(id); -} - -bool WeakMap::IsDestroyed() const { - return !id_weak_map_; -} - -// static -void WeakMap::BuildPrototype(v8::Isolate* isolate, - v8::Local prototype) { - mate::ObjectTemplateBuilder(isolate, prototype) - .SetMethod("set", &WeakMap::Set) - .SetMethod("get", &WeakMap::Get) - .SetMethod("has", &WeakMap::Has) - .SetMethod("remove", &WeakMap::Remove); -} - -// static -mate::Wrappable* WeakMap::Create(v8::Isolate* isolate) { - return new WeakMap; -} - -} // namespace api - -} // namespace atom - -namespace { - -using atom::api::WeakMap; - -void Initialize(v8::Local exports, v8::Local unused, - v8::Local context, void* priv) { - v8::Isolate* isolate = context->GetIsolate(); - v8::Local constructor = mate::CreateConstructor( - isolate, "WeakMap", base::Bind(&WeakMap::Create)); - mate::Dictionary weak_map(isolate, constructor); - mate::Dictionary dict(isolate, exports); - dict.Set("WeakMap", weak_map); -} - -} // namespace - -NODE_MODULE_CONTEXT_AWARE_BUILTIN(atom_common_weak_map, Initialize) diff --git a/atom/common/api/lib/callbacks-registry.coffee b/atom/common/api/lib/callbacks-registry.coffee index b777f7a462df..57f5d0343dc2 100644 --- a/atom/common/api/lib/callbacks-registry.coffee +++ b/atom/common/api/lib/callbacks-registry.coffee @@ -7,6 +7,9 @@ class CallbacksRegistry @callbacks = {} add: (callback) -> + if v8Util.getHiddenValue(callback, 'metaId')? + return v8Util.getHiddenValue(callback, 'metaId') + id = ++@nextId # Capture the location of the function and put it in the ID string, @@ -19,11 +22,10 @@ class CallbacksRegistry continue if location.indexOf('(native)') isnt -1 continue if location.indexOf('atom.asar') isnt -1 [x, filenameAndLine] = /([^/^\)]*)\)?$/gi.exec(location) + [x, line, column] = /(\d+):(\d+)/g.exec(filenameAndLine) + id += parseInt(line) + parseInt(column) break - if v8Util.getHiddenValue(callback, 'metaId')? - return v8Util.getHiddenValue(callback, 'metaId') - @callbacks[id] = callback v8Util.setHiddenValue callback, 'metaId', id v8Util.setHiddenValue callback, 'location', filenameAndLine diff --git a/atom/common/node_bindings.cc b/atom/common/node_bindings.cc index b6fcdb82f819..10da202da70d 100644 --- a/atom/common/node_bindings.cc +++ b/atom/common/node_bindings.cc @@ -48,11 +48,11 @@ REFERENCE_MODULE(atom_browser_window); REFERENCE_MODULE(atom_common_asar); REFERENCE_MODULE(atom_common_clipboard); REFERENCE_MODULE(atom_common_crash_reporter); +REFERENCE_MODULE(atom_common_id_weak_map); REFERENCE_MODULE(atom_common_native_image); REFERENCE_MODULE(atom_common_screen); REFERENCE_MODULE(atom_common_shell); REFERENCE_MODULE(atom_common_v8_util); -REFERENCE_MODULE(atom_common_weak_map); REFERENCE_MODULE(atom_renderer_ipc); REFERENCE_MODULE(atom_renderer_web_frame); #undef REFERENCE_MODULE diff --git a/filenames.gypi b/filenames.gypi index 0f5b794a0361..ba739764f492 100644 --- a/filenames.gypi +++ b/filenames.gypi @@ -255,13 +255,13 @@ 'atom/common/api/atom_api_asar.cc', 'atom/common/api/atom_api_clipboard.cc', 'atom/common/api/atom_api_crash_reporter.cc', + 'atom/common/api/atom_api_id_weak_map.cc', + 'atom/common/api/atom_api_id_weak_map.h', 'atom/common/api/atom_api_native_image.cc', 'atom/common/api/atom_api_native_image.h', 'atom/common/api/atom_api_native_image_mac.mm', 'atom/common/api/atom_api_shell.cc', 'atom/common/api/atom_api_v8_util.cc', - 'atom/common/api/atom_api_weak_map.cc', - 'atom/common/api/atom_api_weak_map.h', 'atom/common/api/atom_bindings.cc', 'atom/common/api/atom_bindings.h', 'atom/common/api/event_emitter_caller.cc', From 2c59f4567e2e1b707b0e5493188f0106801837e4 Mon Sep 17 00:00:00 2001 From: Robo Date: Fri, 30 Oct 2015 08:34:40 +0530 Subject: [PATCH 5/5] use webcontents id to identify callbacks --- atom/browser/lib/rpc-server.coffee | 15 ++++++++++++--- atom/common/api/atom_api_id_weak_map.cc | 4 ++-- atom/common/api/atom_api_id_weak_map.h | 4 ++-- atom/common/api/lib/callbacks-registry.coffee | 2 -- atom/common/id_weak_map.cc | 15 ++++++--------- atom/common/id_weak_map.h | 6 +++--- 6 files changed, 25 insertions(+), 21 deletions(-) diff --git a/atom/browser/lib/rpc-server.coffee b/atom/browser/lib/rpc-server.coffee index 40e1b0e46ac7..eb97c721396f 100644 --- a/atom/browser/lib/rpc-server.coffee +++ b/atom/browser/lib/rpc-server.coffee @@ -4,8 +4,8 @@ objectsRegistry = require './objects-registry.js' v8Util = process.atomBinding 'v8_util' IDWeakMap = process.atomBinding('id_weak_map').IDWeakMap -# Weak reference to callback with their registry ID. -rendererCallbacks = new IDWeakMap() +# Object mapping from webcontents id to their renderer callbacks weakmap. +rendererRegistry = {} # Convert a real value into meta data. valueToMeta = (sender, value, optimizeSimpleObject=false) -> @@ -74,11 +74,18 @@ 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) rendererReleased = false - objectsRegistry.once "clear-#{sender.getId()}", -> + objectsRegistry.once "clear-#{webContentsId}", -> rendererReleased = true ret = -> @@ -109,6 +116,8 @@ 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 bdc298fa0c49..8c17e8330ccc 100644 --- a/atom/common/api/atom_api_id_weak_map.cc +++ b/atom/common/api/atom_api_id_weak_map.cc @@ -12,11 +12,11 @@ namespace atom { namespace api { -IDWeakMap::IDWeakMap() { - id_weak_map_.reset(new atom::IDWeakMap); +IDWeakMap::IDWeakMap() : id_weak_map_(new atom::IDWeakMap) { } IDWeakMap::~IDWeakMap() { + id_weak_map_ = nullptr; } void IDWeakMap::Set(v8::Isolate* isolate, diff --git a/atom/common/api/atom_api_id_weak_map.h b/atom/common/api/atom_api_id_weak_map.h index 3acdddc9c75f..4a2f8e397a80 100644 --- a/atom/common/api/atom_api_id_weak_map.h +++ b/atom/common/api/atom_api_id_weak_map.h @@ -22,7 +22,7 @@ class IDWeakMap : public mate::Wrappable { protected: IDWeakMap(); - virtual ~IDWeakMap(); + ~IDWeakMap(); // mate::Wrappable: bool IsDestroyed() const override; @@ -34,7 +34,7 @@ class IDWeakMap : public mate::Wrappable { bool Has(int32_t id); void Remove(int32_t id); - scoped_ptr 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 57f5d0343dc2..001ecae14a61 100644 --- a/atom/common/api/lib/callbacks-registry.coffee +++ b/atom/common/api/lib/callbacks-registry.coffee @@ -22,8 +22,6 @@ class CallbacksRegistry continue if location.indexOf('(native)') isnt -1 continue if location.indexOf('atom.asar') isnt -1 [x, filenameAndLine] = /([^/^\)]*)\)?$/gi.exec(location) - [x, line, column] = /(\d+):(\d+)/g.exec(filenameAndLine) - id += parseInt(line) + parseInt(column) break @callbacks[id] = callback diff --git a/atom/common/id_weak_map.cc b/atom/common/id_weak_map.cc index 7fda009e0ee1..a78dcbceba53 100644 --- a/atom/common/id_weak_map.cc +++ b/atom/common/id_weak_map.cc @@ -32,15 +32,6 @@ IDWeakMap::IDWeakMap() : next_id_(0) { IDWeakMap::~IDWeakMap() { } -int32_t IDWeakMap::Add(v8::Isolate* isolate, v8::Local object) { - int32_t id = GetNextID(); - auto global = make_linked_ptr(new v8::Global(isolate, object)); - ObjectKey* key = new ObjectKey(id, this); - global->SetWeak(key, OnObjectGC, v8::WeakCallbackType::kParameter); - map_[id] = global; - return id; -} - void IDWeakMap::Set(v8::Isolate* isolate, int32_t id, v8::Local object) { @@ -50,6 +41,12 @@ void IDWeakMap::Set(v8::Isolate* isolate, map_[id] = global; } +int32_t IDWeakMap::Add(v8::Isolate* isolate, v8::Local object) { + int32_t id = GetNextID(); + Set(isolate, id, object); + return id; +} + v8::MaybeLocal IDWeakMap::Get(v8::Isolate* isolate, int32_t id) { auto iter = map_.find(id); if (iter == map_.end()) diff --git a/atom/common/id_weak_map.h b/atom/common/id_weak_map.h index 688e85cd0cce..72c64c6ae5d4 100644 --- a/atom/common/id_weak_map.h +++ b/atom/common/id_weak_map.h @@ -19,12 +19,12 @@ class IDWeakMap { IDWeakMap(); ~IDWeakMap(); - // Adds |object| to WeakMap and returns its allocated |id|. - int32_t Add(v8::Isolate* isolate, v8::Local object); - // Sets the object to WeakMap with the given |id|. void Set(v8::Isolate* isolate, int32_t id, v8::Local object); + // Adds |object| to WeakMap and returns its allocated |id|. + int32_t Add(v8::Isolate* isolate, v8::Local object); + // Gets the object from WeakMap by its |id|. v8::MaybeLocal Get(v8::Isolate* isolate, int32_t id);