From d02413de0071f8496087ae219e02c8f9105a5791 Mon Sep 17 00:00:00 2001
From: Cheng Zhao <zcbenz@gmail.com>
Date: Wed, 24 Jun 2015 13:35:39 +0800
Subject: [PATCH] Make the JS IDWeakMap a thin wrapper of C++ IDWeakMap

---
 atom/common/api/atom_api_id_weak_map.cc | 51 ++++++++-------------------------
 atom/common/api/atom_api_id_weak_map.h  | 14 ++-------
 atom/common/id_weak_map.cc              |  2 +-
 atom/common/id_weak_map.h               |  2 +-
 4 files changed, 16 insertions(+), 53 deletions(-)

diff --git a/atom/common/api/atom_api_id_weak_map.cc b/atom/common/api/atom_api_id_weak_map.cc
index 614683c..c5fbf09 100644
--- a/atom/common/api/atom_api_id_weak_map.cc
+++ b/atom/common/api/atom_api_id_weak_map.cc
@@ -4,9 +4,6 @@
 
 #include "atom/common/api/atom_api_id_weak_map.h"
 
-#include <algorithm>
-
-#include "base/logging.h"
 #include "native_mate/constructor.h"
 #include "native_mate/object_template_builder.h"
 
@@ -16,53 +13,37 @@ namespace atom {
 
 namespace api {
 
-IDWeakMap::IDWeakMap()
-    : next_id_(0) {
+IDWeakMap::IDWeakMap() {
 }
 
 IDWeakMap::~IDWeakMap() {
 }
 
 int32_t IDWeakMap::Add(v8::Isolate* isolate, v8::Local<v8::Object> object) {
-  int32_t key = GetNextID();
-  object->SetHiddenValue(mate::StringToV8(isolate, "IDWeakMapKey"),
-                         mate::Converter<int32_t>::ToV8(isolate, key));
-
-  map_[key] = new mate::RefCountedPersistent<v8::Object>(isolate, object);
-  map_[key]->SetWeak(this, WeakCallback);
-  return key;
+  return map_.Add(isolate, object);
 }
 
 v8::Local<v8::Value> IDWeakMap::Get(v8::Isolate* isolate, int32_t key) {
-  if (!Has(key)) {
-    node::ThrowError(isolate, "Invalid key");
+  v8::MaybeLocal<v8::Object> result = map_.Get(isolate, key);
+  if (result.IsEmpty()) {
+    isolate->ThrowException(v8::Exception::Error(
+        mate::StringToV8(isolate, "Invalid key")));
     return v8::Undefined(isolate);
+  } else {
+    return result.ToLocalChecked();
   }
-
-  return map_[key]->NewHandle();
 }
 
 bool IDWeakMap::Has(int32_t key) const {
-  return map_.find(key) != map_.end();
+  return map_.Has(key);
 }
 
 std::vector<int32_t> IDWeakMap::Keys() const {
-  std::vector<int32_t> keys;
-  keys.reserve(map_.size());
-  for (auto it = map_.begin(); it != map_.end(); ++it)
-    keys.push_back(it->first);
-  return keys;
+  return map_.Keys();
 }
 
 void IDWeakMap::Remove(int32_t key) {
-  if (Has(key))
-    map_.erase(key);
-  else
-    LOG(WARNING) << "Object with key " << key << " is being GCed for twice.";
-}
-
-int IDWeakMap::GetNextID() {
-  return ++next_id_;
+  map_.Remove(key);
 }
 
 // static
@@ -76,14 +57,6 @@ void IDWeakMap::BuildPrototype(v8::Isolate* isolate,
       .SetMethod("remove", &IDWeakMap::Remove);
 }
 
-// static
-void IDWeakMap::WeakCallback(
-    const v8::WeakCallbackData<v8::Object, IDWeakMap>& data) {
-  int32_t key = data.GetValue()->GetHiddenValue(
-      mate::StringToV8(data.GetIsolate(), "IDWeakMapKey"))->Int32Value();
-  data.GetParameter()->Remove(key);
-}
-
 }  // namespace api
 
 }  // namespace atom
@@ -99,7 +72,7 @@ void Initialize(v8::Local<v8::Object> exports, v8::Local<v8::Value> unused,
       isolate,
       "IDWeakMap",
       base::Bind(&mate::NewOperatorFactory<IDWeakMap>));
-  exports->Set(mate::StringToV8(isolate, "IDWeakMap"), constructor);
+  exports->Set(mate::StringToSymbol(isolate, "IDWeakMap"), constructor);
 }
 
 }  // namespace
diff --git a/atom/common/api/atom_api_id_weak_map.h b/atom/common/api/atom_api_id_weak_map.h
index 0648ce9..955bd83 100644
--- a/atom/common/api/atom_api_id_weak_map.h
+++ b/atom/common/api/atom_api_id_weak_map.h
@@ -6,11 +6,9 @@
 #ifndef ATOM_COMMON_API_ATOM_API_ID_WEAK_MAP_H_
 #define ATOM_COMMON_API_ATOM_API_ID_WEAK_MAP_H_
 
-#include <map>
 #include <vector>
 
-#include "base/basictypes.h"
-#include "native_mate/scoped_persistent.h"
+#include "atom/common/id_weak_map.h"
 #include "native_mate/wrappable.h"
 
 namespace atom {
@@ -33,16 +31,8 @@ class IDWeakMap : public mate::Wrappable {
   bool Has(int32_t key) const;
   std::vector<int32_t> Keys() const;
   void Remove(int32_t key);
-  int GetNextID();
 
-  static void WeakCallback(
-      const v8::WeakCallbackData<v8::Object, IDWeakMap>& data);
-
-  int32_t next_id_;
-
-  typedef scoped_refptr<mate::RefCountedPersistent<v8::Object> >
-      RefCountedV8Object;
-  std::map<int32_t, RefCountedV8Object> map_;
+  atom::IDWeakMap map_;
 
   DISALLOW_COPY_AND_ASSIGN(IDWeakMap);
 };
diff --git a/atom/common/id_weak_map.cc b/atom/common/id_weak_map.cc
index 556b9ba..68923b1 100644
--- a/atom/common/id_weak_map.cc
+++ b/atom/common/id_weak_map.cc
@@ -55,7 +55,7 @@ void IDWeakMap::Remove(int32_t id) {
     map_.erase(iter);
 }
 
-int IDWeakMap::GetNextID() {
+int32_t IDWeakMap::GetNextID() {
   return ++next_id_;
 }
 
diff --git a/atom/common/id_weak_map.h b/atom/common/id_weak_map.h
index 4ef3c05..cd01098 100644
--- a/atom/common/id_weak_map.h
+++ b/atom/common/id_weak_map.h
@@ -36,7 +36,7 @@ class IDWeakMap {
 
  private:
   // Returns next available ID.
-  int GetNextID();
+  int32_t GetNextID();
 
   static void WeakCallback(
       const v8::WeakCallbackData<v8::Object, IDWeakMap>& data);
-- 
2.7.4