Implement PersistentValueMap, a map that stores UniquePersistent values.

This is preparatory work to get rid of UnsafePersistent in blink.

The previous version had to be reverted due to timeouts in win32/Debug: https://codereview.chromium.org/197173002/

The timeouts happened because the STL version on that platform contains sanity checking code which opens a 'debug window' in the GUI, patiently waiting for the user to click ok/cancel/somethirdoption. It turns out, the cause for that debug window was totally valid and the test had a use-after-free issue.

The 1st patch set is the code as before. The 2nd patch set contains the fix.

Related blink changes are here: https://codereview.chromium.org/180363004/

This patch is largely based on https://codereview.chromium.org/175503003/, with some methods added to support the blink change mentioned above.

BUG=
R=dcarney@chromium.org

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

Patch from Daniel Vogelheim <vogelheim@chromium.org>.

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

include/v8.h

@@ -140,6 +140,7 @@ class ObjectOperationDescriptor;
 class RawOperationDescriptor;
 class CallHandlerHelper;
 class EscapableHandleScope;
+template<typename T> class ReturnValue;
 
 namespace internal {
 class Arguments;
@@ -412,6 +413,7 @@ template <class T> class Local : public Handle<T> {
   template<class F> friend class internal::CustomArguments;
   friend class HandleScope;
   friend class EscapableHandleScope;
+  template<class F1, class F2, class F3> friend class PersistentValueMap;
 
   V8_INLINE static Local<T> New(Isolate* isolate, T* that);
 };
@@ -527,7 +529,11 @@ template <class T> class PersistentBase {
       P* parameter,
       typename WeakCallbackData<S, P>::Callback callback);
 
-  V8_INLINE void ClearWeak();
+  template<typename P>
+  V8_INLINE P* ClearWeak();
+
+  // TODO(dcarney): remove this.
+  V8_INLINE void ClearWeak() { ClearWeak<void>(); }
 
   /**
    * Marks the reference to this object independent. Garbage collector is free
@@ -576,6 +582,7 @@ template <class T> class PersistentBase {
   template<class F> friend class UniquePersistent;
   template<class F> friend class PersistentBase;
   template<class F> friend class ReturnValue;
+  template<class F1, class F2, class F3> friend class PersistentValueMap;
   friend class Object;
 
   explicit V8_INLINE PersistentBase(T* val) : val_(val) {}
@@ -744,7 +751,7 @@ class UniquePersistent : public PersistentBase<T> {
   };
 
  public:
-    /**
+  /**
    * A UniquePersistent with no storage cell.
    */
   V8_INLINE UniquePersistent() : PersistentBase<T>(0) { }
@@ -782,6 +789,7 @@ class UniquePersistent : public PersistentBase<T> {
   template<class S>
   V8_INLINE UniquePersistent& operator=(UniquePersistent<S> rhs) {
     TYPE_CHECK(T, S);
+    this->Reset();
     this->val_ = rhs.val_;
     rhs.val_ = 0;
     return *this;
@@ -801,6 +809,145 @@ class UniquePersistent : public PersistentBase<T> {
 };
 
 
+typedef uintptr_t PersistentContainerValue;
+static const uintptr_t kPersistentContainerNotFound = 0;
+
+/**
+ * A map wrapper that allows using UniquePersistent as a mapped value.
+ * C++11 embedders don't need this class, as they can use UniquePersistent
+ * directly in std containers.
+ *
+ * The map relies on a backing map, whose type and accessors are described
+ * by the Traits class. The backing map will handle values of type
+ * PersistentContainerValue, with all conversion into and out of V8
+ * handles being transparently handled by this class.
+ */
+template<class K, class V, class Traits>
+class PersistentValueMap {
+ public:
+  V8_INLINE explicit PersistentValueMap(Isolate* isolate) : isolate_(isolate) {}
+
+  V8_INLINE ~PersistentValueMap() { Clear(); }
+
+  V8_INLINE Isolate* GetIsolate() { return isolate_; }
+
+  /**
+   * Return size of the map.
+   */
+  V8_INLINE size_t Size() { return Traits::Size(&impl_); }
+
+  /**
+   * Get value stored in map.
+   */
+  V8_INLINE Local<V> Get(const K& key) {
+    return Local<V>::New(isolate_, FromVal(Traits::Get(&impl_, key)));
+  }
+
+  /**
+   * Check whether a value is contained in the map.
+   */
+  V8_INLINE bool Contains(const K& key) {
+    return Traits::Get(&impl_, key) != 0;
+  }
+
+  /**
+   * Get value stored in map and set it in returnValue.
+   * Return true if a value was found.
+   */
+  V8_INLINE bool SetReturnValue(const K& key,
+    ReturnValue<Value>& returnValue);
+
+  /**
+   * Call Isolate::SetReference with the given parent and the map value.
+   */
+  V8_INLINE void SetReference(const K& key,
+      const v8::Persistent<v8::Object>& parent) {
+    GetIsolate()->SetReference(
+      reinterpret_cast<internal::Object**>(parent.val_),
+      reinterpret_cast<internal::Object**>(FromVal(Traits::Get(&impl_, key))));
+  }
+
+  /**
+   * Put value into map. Depending on Traits::kIsWeak, the value will be held
+   * by the map strongly or weakly.
+   * Returns old value as UniquePersistent.
+   */
+  UniquePersistent<V> Set(const K& key, Local<V> value) {
+    UniquePersistent<V> persistent(isolate_, value);
+    return SetUnique(key, &persistent);
+  }
+
+  /**
+   * Put value into map, like Set(const K&, Local<V>).
+   */
+  UniquePersistent<V> Set(const K& key, UniquePersistent<V> value) {
+    return SetUnique(key, &value);
+  }
+
+  /**
+   * Return value for key and remove it from the map.
+   */
+  V8_INLINE UniquePersistent<V> Remove(const K& key) {
+    return Release(Traits::Remove(&impl_, key)).Pass();
+  }
+
+  /**
+  * Traverses the map repeatedly,
+  * in case side effects of disposal cause insertions.
+  **/
+  void Clear();
+
+ private:
+  PersistentValueMap(PersistentValueMap&);
+  void operator=(PersistentValueMap&);
+
+  /**
+   * Put the value into the map, and set the 'weak' callback when demanded
+   * by the Traits class.
+   */
+  UniquePersistent<V> SetUnique(const K& key, UniquePersistent<V>* persistent) {
+    if (Traits::kIsWeak) {
+      Local<V> value(Local<V>::New(isolate_, *persistent));
+      persistent->template SetWeak<typename Traits::WeakCallbackDataType>(
+        Traits::WeakCallbackParameter(&impl_, key, value), WeakCallback);
+    }
+    PersistentContainerValue old_value =
+        Traits::Set(&impl_, key, ClearAndLeak(persistent));
+    return Release(old_value).Pass();
+  }
+
+  static void WeakCallback(
+      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_;
+    persistent->val_ = 0;
+    return reinterpret_cast<PersistentContainerValue>(v);
+  }
+
+  /**
+   * Return a container value as UniquePersistent and make sure the weak
+   * callback is properly disposed of. All remove functionality should go
+   * through this.
+   */
+  V8_INLINE static UniquePersistent<V> Release(PersistentContainerValue v) {
+    UniquePersistent<V> p;
+    p.val_ = FromVal(v);
+    if (Traits::kIsWeak && !p.IsEmpty()) {
+      Traits::DisposeCallbackData(
+          p.template ClearWeak<typename Traits::WeakCallbackDataType>());
+    }
+    return p.Pass();
+  }
+
+  Isolate* isolate_;
+  typename Traits::Impl impl_;
+};
+
+
  /**
  * A stack-allocated class that governs a number of local handles.
  * After a handle scope has been created, all local handles will be
@@ -2388,6 +2535,8 @@ class ReturnValue {
   template<class F> friend class ReturnValue;
   template<class F> friend class FunctionCallbackInfo;
   template<class F> friend class PropertyCallbackInfo;
+  template<class F, class G, class H> friend class PersistentValueMap;
+  V8_INLINE void SetInternal(internal::Object* value) { *value_ = value; }
   V8_INLINE internal::Object* GetDefaultValue();
   V8_INLINE explicit ReturnValue(internal::Object** slot);
   internal::Object** value_;
@@ -4235,6 +4384,8 @@ class V8_EXPORT Isolate {
   void SetEventLogger(LogEventCallback that);
 
  private:
+  template<class K, class V, class Traits> friend class PersistentValueMap;
+
   Isolate();
   Isolate(const Isolate&);
   ~Isolate();
@@ -4829,7 +4980,7 @@ class V8_EXPORT V8 {
   static void MakeWeak(internal::Object** global_handle,
                        void* data,
                        WeakCallback weak_callback);
-  static void ClearWeak(internal::Object** global_handle);
+  static void* ClearWeak(internal::Object** global_handle);
   static void Eternalize(Isolate* isolate,
                          Value* handle,
                          int* index);
@@ -5742,8 +5893,10 @@ void PersistentBase<T>::SetWeak(
 
 
 template <class T>
-void PersistentBase<T>::ClearWeak() {
-  V8::ClearWeak(reinterpret_cast<internal::Object**>(this->val_));
+template<typename P>
+P* PersistentBase<T>::ClearWeak() {
+  return reinterpret_cast<P*>(
+    V8::ClearWeak(reinterpret_cast<internal::Object**>(this->val_)));
 }
 
 
@@ -5796,6 +5949,44 @@ uint16_t PersistentBase<T>::WrapperClassId() const {
 }
 
 
+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));
+    }
+  }
+}
+
+
+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);
+}
+
+
 template<typename T>
 ReturnValue<T>::ReturnValue(internal::Object** slot) : value_(slot) {}
 

src/api.cc

@@ -560,8 +560,8 @@ void V8::MakeWeak(i::Object** object,
 }
 
 
-void V8::ClearWeak(i::Object** obj) {
-  i::GlobalHandles::ClearWeakness(obj);
+void* V8::ClearWeak(i::Object** obj) {
+  return i::GlobalHandles::ClearWeakness(obj);
 }
 
 

src/global-handles.cc

@@ -235,10 +235,12 @@ class GlobalHandles::Node {
     weak_callback_ = weak_callback;
   }
 
-  void ClearWeakness() {
+  void* ClearWeakness() {
     ASSERT(state() != FREE);
+    void* p = parameter();
     set_state(NORMAL);
     set_parameter(NULL);
+    return p;
   }
 
   bool PostGarbageCollectionProcessing(Isolate* isolate) {
@@ -502,8 +504,8 @@ void GlobalHandles::MakeWeak(Object** location,
 }
 
 
-void GlobalHandles::ClearWeakness(Object** location) {
-  Node::FromLocation(location)->ClearWeakness();
+void* GlobalHandles::ClearWeakness(Object** location) {
+  return Node::FromLocation(location)->ClearWeakness();
 }
 
 

src/global-handles.h

@@ -161,7 +161,7 @@ class GlobalHandles {
   }
 
   // Clear the weakness of a global handle.
-  static void ClearWeakness(Object** location);
+  static void* ClearWeakness(Object** location);
 
   // Clear the weakness of a global handle.
   static void MarkIndependent(Object** location);

test/cctest/test-api.cc

@@ -3444,6 +3444,116 @@ THREADED_TEST(UniquePersistent) {
 }
 
 
+template<typename K, typename V, bool is_weak>
+class StdPersistentValueMapTraits {
+ public:
+  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;
+    data->impl = impl;
+    data->key = key;
+    return data;
+  }
+  static Impl* ImplFromWeakCallbackData(
+      const v8::WeakCallbackData<V, WeakCallbackDataType>& data) {
+    return data.GetParameter()->impl;
+  }
+  static K KeyFromWeakCallbackData(
+      const v8::WeakCallbackData<V, WeakCallbackDataType>& data) {
+    return data.GetParameter()->key;
+  }
+  static void DisposeCallbackData(WeakCallbackDataType* data) {
+    delete data;
+  }
+};
+
+
+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();
+  int initial_handle_count = global_handles->global_handles_count();
+  CHECK_EQ(0, static_cast<int>(map.Size()));
+  {
+    HandleScope scope(isolate);
+    Local<v8::Object> obj = map.Get(7);
+    CHECK(obj.IsEmpty());
+    Local<v8::Object> expected = v8::Object::New(isolate);
+    map.Set(7, expected);
+    CHECK_EQ(1, static_cast<int>(map.Size()));
+    obj = map.Get(7);
+    CHECK_EQ(expected, obj);
+    v8::UniquePersistent<v8::Object> removed = map.Remove(7);
+    CHECK_EQ(0, static_cast<int>(map.Size()));
+    CHECK(expected == removed);
+    removed = map.Remove(7);
+    CHECK(removed.IsEmpty());
+    map.Set(8, expected);
+    CHECK_EQ(1, static_cast<int>(map.Size()));
+    map.Set(8, expected);
+    CHECK_EQ(1, static_cast<int>(map.Size()));
+  }
+  CHECK_EQ(initial_handle_count + 1, global_handles->global_handles_count());
+  if (is_weak) {
+    reinterpret_cast<v8::internal::Isolate*>(isolate)->heap()->
+        CollectAllGarbage(i::Heap::kAbortIncrementalMarkingMask);
+  } else {
+    map.Clear();
+  }
+  CHECK_EQ(0, static_cast<int>(map.Size()));
+  CHECK_EQ(initial_handle_count, global_handles->global_handles_count());
+}
+
+
+TEST(PersistentValueMap) {
+  TestPersistentValueMap<false>();
+  TestPersistentValueMap<true>();
+}
+
+
 THREADED_TEST(GlobalHandleUpcast) {
   v8::Isolate* isolate = CcTest::isolate();
   v8::HandleScope scope(isolate);