Revert "First attempt at providing default traits for PersistentValueMap."

This reverts r20038.

TBR=vogelheim@chromium.org

Review URL: https://codereview.chromium.org/203553002

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@20042 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

include/v8-util.h

@@ -29,7 +29,6 @@
 #define V8_UTIL_H_
 
 #include "v8.h"
-#include <map>
 
 /**
  * Support for Persistent containers.
@@ -43,90 +42,6 @@ namespace v8 {
 typedef uintptr_t PersistentContainerValue;
 static const uintptr_t kPersistentContainerNotFound = 0;
 
-
-/**
- * A default trait implemenation for PersistentValueMap which uses std::map
- * as a backing map.
- *
- * Users will have to implement their own weak callbacks & dispose traits.
- */
-template<typename K, typename V>
-class StdMapTraits {
- public:
-  // STL map & related:
-  typedef std::map<K, v8::PersistentContainerValue> Impl;
-  typedef typename Impl::iterator Iterator;
-
-  static bool Empty(Impl* impl) { return impl->empty(); }
-  static size_t Size(Impl* impl) { return impl->size(); }
-  static void Swap(Impl& a, Impl& b) { std::swap(a, b); }  // NOLINT
-  static Iterator Begin(Impl* impl) { return impl->begin(); }
-  static Iterator End(Impl* impl) { return impl->end(); }
-  static K Key(Iterator it) { return it->first; }
-  static v8::PersistentContainerValue Value(Iterator it) { return it->second; }
-  static v8::PersistentContainerValue Set(Impl* impl, K key,
-      v8::PersistentContainerValue value) {
-    std::pair<Iterator, bool> res = impl->insert(std::make_pair(key, value));
-    v8::PersistentContainerValue old_value = v8::kPersistentContainerNotFound;
-    if (!res.second) {
-      old_value = res.first->second;
-      res.first->second = value;
-    }
-    return old_value;
-  }
-  static v8::PersistentContainerValue Get(Impl* impl, K key) {
-    Iterator it = impl->find(key);
-    if (it == impl->end()) return v8::kPersistentContainerNotFound;
-    return it->second;
-  }
-  static v8::PersistentContainerValue Remove(Impl* impl, K key) {
-    Iterator it = impl->find(key);
-    if (it == impl->end()) return v8::kPersistentContainerNotFound;
-    v8::PersistentContainerValue value = it->second;
-    impl->erase(it);
-    return value;
-  }
-};
-
-
-/**
- * A default trait implementation for PersistentValueMap, which inherits
- * a std:map backing map from StdMapTraits and holds non-weak persistent
- * objects.
- *
- * Users have to implement their own dispose trait.
- */
-template<typename K, typename V>
-class StrongMapTraits : public StdMapTraits<K, V> {
- public:
-  // Weak callback & friends:
-  static const bool kIsWeak = false;
-  typedef typename StdMapTraits<K, V>::Impl Impl;
-  typedef void WeakCallbackDataType;
-  static WeakCallbackDataType* WeakCallbackParameter(
-      Impl* impl, const K& key, Local<V> value);
-  static Impl* ImplFromWeakCallbackData(
-      const v8::WeakCallbackData<V, WeakCallbackDataType>& data);
-  static K KeyFromWeakCallbackData(
-      const v8::WeakCallbackData<V, WeakCallbackDataType>& data);
-  static void DisposeCallbackData(WeakCallbackDataType* data);
-};
-
-
-/**
- * A default trait implementation for PersistentValueMap, with a std::map
- * backing map, non-weak persistents as values, and no special dispose
- * handling. Can be used as-is.
- */
-template<typename K, typename V>
-class DefaultPersistentValueMapTraits : public StrongMapTraits<K, V> {
- public:
-  typedef typename StrongMapTraits<K, V>::Impl Impl;
-  static void Dispose(v8::Isolate* isolate, v8::UniquePersistent<V> value,
-      Impl* impl, K key) { }
-};
-
-
 /**
  * A map wrapper that allows using UniquePersistent as a mapped value.
  * C++11 embedders don't need this class, as they can use UniquePersistent
@@ -137,7 +52,7 @@ class DefaultPersistentValueMapTraits : public StrongMapTraits<K, V> {
  * PersistentContainerValue, with all conversion into and out of V8
  * handles being transparently handled by this class.
  */
-template<typename K, typename V, typename Traits>
+template<class K, class V, class Traits>
 class PersistentValueMap {
  public:
   V8_INLINE explicit PersistentValueMap(Isolate* isolate) : isolate_(isolate) {}
@@ -151,11 +66,6 @@ class PersistentValueMap {
    */
   V8_INLINE size_t Size() { return Traits::Size(&impl_); }
 
-  /**
-   * Return whether the map holds weak persistents.
-   */
-  V8_INLINE bool IsWeak() { return Traits::kIsWeak; }
-
   /**
    * Get value stored in map.
    */
@@ -175,15 +85,7 @@ class PersistentValueMap {
    * Return true if a value was found.
    */
   V8_INLINE bool SetReturnValue(const K& key,
-      ReturnValue<Value>& returnValue) {
-    PersistentContainerValue value = Traits::Get(&impl_, key);
-    bool hasValue = value != 0;
-    if (hasValue) {
-      returnValue.SetInternal(
-          *reinterpret_cast<internal::Object**>(FromVal(value)));
-    }
-    return hasValue;
-  }
+    ReturnValue<Value>& returnValue);
 
   /**
    * Call Isolate::SetReference with the given parent and the map value.
@@ -223,19 +125,7 @@ class PersistentValueMap {
   * Traverses the map repeatedly,
   * in case side effects of disposal cause insertions.
   **/
-  void Clear() {
-    typedef typename Traits::Iterator It;
-    HandleScope handle_scope(isolate_);
-    // TODO(dcarney): figure out if this swap and loop is necessary.
-    while (!Traits::Empty(&impl_)) {
-      typename Traits::Impl impl;
-      Traits::Swap(impl_, impl);
-      for (It i = Traits::Begin(&impl); i != Traits::End(&impl); ++i) {
-        Traits::Dispose(isolate_, Release(Traits::Value(i)).Pass(), &impl,
-          Traits::Key(i));
-      }
-    }
-  }
+  void Clear();
 
  private:
   PersistentValueMap(PersistentValueMap&);
@@ -257,19 +147,10 @@ class PersistentValueMap {
   }
 
   static void WeakCallback(
-      const WeakCallbackData<V, typename Traits::WeakCallbackDataType>& data) {
-    if (Traits::kIsWeak) {
-      typename Traits::Impl* impl = Traits::ImplFromWeakCallbackData(data);
-      K key = Traits::KeyFromWeakCallbackData(data);
-      PersistentContainerValue value = Traits::Remove(impl, key);
-      Traits::Dispose(data.GetIsolate(), Release(value).Pass(), impl, key);
-    }
-  }
-
+      const WeakCallbackData<V, typename Traits::WeakCallbackDataType>& data);
   V8_INLINE static V* FromVal(PersistentContainerValue v) {
     return reinterpret_cast<V*>(v);
   }
-
   V8_INLINE static PersistentContainerValue ClearAndLeak(
       UniquePersistent<V>* persistent) {
     V* v = persistent->val_;
@@ -296,21 +177,42 @@ class PersistentValueMap {
   typename Traits::Impl impl_;
 };
 
+template <class K, class V, class Traits>
+bool PersistentValueMap<K, V, Traits>::SetReturnValue(const K& key,
+    ReturnValue<Value>& returnValue) {
+  PersistentContainerValue value = Traits::Get(&impl_, key);
+  bool hasValue = value != 0;
+  if (hasValue) {
+    returnValue.SetInternal(
+        *reinterpret_cast<internal::Object**>(FromVal(value)));
+  }
+  return hasValue;
+}
+
+template <class K, class V, class Traits>
+void PersistentValueMap<K, V, Traits>::Clear() {
+  typedef typename Traits::Iterator It;
+  HandleScope handle_scope(isolate_);
+  // TODO(dcarney): figure out if this swap and loop is necessary.
+  while (!Traits::Empty(&impl_)) {
+    typename Traits::Impl impl;
+    Traits::Swap(impl_, impl);
+    for (It i = Traits::Begin(&impl); i != Traits::End(&impl); ++i) {
+      Traits::Dispose(isolate_, Release(Traits::Value(i)).Pass(), &impl,
+        Traits::Key(i));
+    }
+  }
+}
 
-/**
- * A map that uses UniquePersistent as value and std::map as the backing
- * implementation. Persistents are held non-weak.
- *
- * C++11 embedders don't need this class, as they can use
- * UniquePersistent directly in std containers.
- */
-template<typename K, typename V,
-    typename Traits = DefaultPersistentValueMapTraits<K, V> >
-class StdPersistentValueMap : public PersistentValueMap<K, V, Traits> {
- public:
-  explicit StdPersistentValueMap(v8::Isolate* isolate)
-      : PersistentValueMap<K, V, Traits>(isolate) {}
-};
+
+template <class K, class V, class Traits>
+void PersistentValueMap<K, V, Traits>::WeakCallback(
+    const WeakCallbackData<V, typename Traits::WeakCallbackDataType>& data) {
+  typename Traits::Impl* impl = Traits::ImplFromWeakCallbackData(data);
+  K key = Traits::KeyFromWeakCallbackData(data);
+  PersistentContainerValue value = Traits::Remove(impl, key);
+  Traits::Dispose(data.GetIsolate(), Release(value).Pass(), impl, key);
+}
 
 }  // namespace v8
 

test/cctest/test-api.cc

@@ -3445,15 +3445,47 @@ THREADED_TEST(UniquePersistent) {
 }
 
 
-template<typename K, typename V>
-class WeakStdMapTraits : public v8::StdMapTraits<K, V> {
+template<typename K, typename V, bool is_weak>
+class StdPersistentValueMapTraits {
  public:
-  typedef typename v8::DefaultPersistentValueMapTraits<K, V>::Impl Impl;
-  static const bool kIsWeak = true;
+  static const bool kIsWeak = is_weak;
+  typedef v8::PersistentContainerValue VInt;
+  typedef std::map<K, VInt> Impl;
   struct WeakCallbackDataType {
     Impl* impl;
     K key;
   };
+  typedef typename Impl::iterator Iterator;
+  static bool Empty(Impl* impl) { return impl->empty(); }
+  static size_t Size(Impl* impl) { return impl->size(); }
+  static void Swap(Impl& a, Impl& b) { std::swap(a, b); }  // NOLINT
+  static Iterator Begin(Impl* impl) { return impl->begin(); }
+  static Iterator End(Impl* impl) { return impl->end(); }
+  static K Key(Iterator it) { return it->first; }
+  static VInt Value(Iterator it) { return it->second; }
+  static VInt Set(Impl* impl, K key, VInt value) {
+    std::pair<Iterator, bool> res = impl->insert(std::make_pair(key, value));
+    VInt old_value = v8::kPersistentContainerNotFound;
+    if (!res.second) {
+      old_value = res.first->second;
+      res.first->second = value;
+    }
+    return old_value;
+  }
+  static VInt Get(Impl* impl, K key) {
+    Iterator it = impl->find(key);
+    if (it == impl->end()) return v8::kPersistentContainerNotFound;
+    return it->second;
+  }
+  static VInt Remove(Impl* impl, K key) {
+    Iterator it = impl->find(key);
+    if (it == impl->end()) return v8::kPersistentContainerNotFound;
+    VInt value = it->second;
+    impl->erase(it);
+    return value;
+  }
+  static void Dispose(v8::Isolate* isolate, v8::UniquePersistent<V> value,
+      Impl* impl, K key) {}
   static WeakCallbackDataType* WeakCallbackParameter(
       Impl* impl, const K& key, Local<V> value) {
     WeakCallbackDataType* data = new WeakCallbackDataType;
@@ -3472,15 +3504,15 @@ class WeakStdMapTraits : public v8::StdMapTraits<K, V> {
   static void DisposeCallbackData(WeakCallbackDataType* data) {
     delete data;
   }
-  static void Dispose(v8::Isolate* isolate, v8::UniquePersistent<V> value,
-      Impl* impl, K key) { }
 };
 
 
-template<typename Map>
+template<bool is_weak>
 static void TestPersistentValueMap() {
   LocalContext env;
   v8::Isolate* isolate = env->GetIsolate();
+  typedef v8::PersistentValueMap<int, v8::Object,
+      StdPersistentValueMapTraits<int, v8::Object, is_weak> > Map;
   Map map(isolate);
   v8::internal::GlobalHandles* global_handles =
       reinterpret_cast<v8::internal::Isolate*>(isolate)->global_handles();
@@ -3506,7 +3538,7 @@ static void TestPersistentValueMap() {
     CHECK_EQ(1, static_cast<int>(map.Size()));
   }
   CHECK_EQ(initial_handle_count + 1, global_handles->global_handles_count());
-  if (map.IsWeak()) {
+  if (is_weak) {
     reinterpret_cast<v8::internal::Isolate*>(isolate)->heap()->
         CollectAllGarbage(i::Heap::kAbortIncrementalMarkingMask);
   } else {
@@ -3518,13 +3550,8 @@ static void TestPersistentValueMap() {
 
 
 TEST(PersistentValueMap) {
-  // Default case, w/o weak callbacks:
-  TestPersistentValueMap<v8::StdPersistentValueMap<int, v8::Object> >();
-
-  // Custom traits with weak callbacks:
-  typedef v8::StdPersistentValueMap<int, v8::Object,
-      WeakStdMapTraits<int, v8::Object> > WeakPersistentValueMap;
-  TestPersistentValueMap<WeakPersistentValueMap>();
+  TestPersistentValueMap<false>();
+  TestPersistentValueMap<true>();
 }