Unify phantom and internal fields weak handle callbacks

include/v8.h

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 /** \mainpage V8 API Reference Guide
  *
  * V8 is Google's open source JavaScript engine.
  *
  * This set of documents provides reference material generated from the
  * V8 header file, include/v8.h.
@@ skipping 119 matching lines @@
 class Heap;
 class HeapObject;
 class Isolate;
 class Object;
 struct StreamedSource;
 template<typename T> class CustomArguments;
 class PropertyCallbackArguments;
 class FunctionCallbackArguments;
 class GlobalHandles;
 
+template <typename T>
 class CallbackData {

[dcarney, 2015/01/09 14:31:10]

one comment disappeared from my previous mail - there's no reason not to have
one callbackdata - WeakCallbackData - and an enum describing the 4 types of weak
callbacks

[Erik Corry Chromium.org, 2015/01/12 11:37:53]

The reason is static type checking.  If your function takes a structure that has
a handle then that's an indication that it may want to use that handle, but if
you set it up with MakePhantom, there isn't going to be one.

  public:
   V8_INLINE v8::Isolate* GetIsolate() const { return isolate_; }
 
- protected:
-  explicit CallbackData(v8::Isolate* isolate) : isolate_(isolate) {}
+  explicit CallbackData(v8::Isolate* isolate, T* parameter)
+      : isolate_(isolate), parameter_(parameter) {}
+  V8_INLINE T* GetParameter() const { return parameter_; }
 
  private:
   v8::Isolate* isolate_;
+  T* parameter_;
 };
 }
 
 
 /**
  * General purpose unique identifier.
  */
 class UniqueId {
  public:
   explicit UniqueId(intptr_t data)
@@ skipping 259 matching lines @@
   V8_INLINE Local<T> Get(Isolate* isolate);
   V8_INLINE bool IsEmpty() { return index_ == kInitialValue; }
   template<class S> V8_INLINE void Set(Isolate* isolate, Local<S> handle);
 
  private:
   static const int kInitialValue = -1;
   int index_;
 };
 
 
-template <typename T>
-class PhantomCallbackData : public internal::CallbackData {
+template <typename T, typename U = void, typename V = void>
+class PhantomCallbackData : public internal::CallbackData<T> {
  public:
-  typedef void (*Callback)(const PhantomCallbackData<T>& data);
+  typedef void (*Callback)(const PhantomCallbackData<T, U, V>& data);
 
-  V8_INLINE T* GetParameter() const { return parameter_; }
+  V8_INLINE U* GetInternalField1() const { return internal_field1_; }
+  V8_INLINE V* GetInternalField2() const { return internal_field2_; }
 
-  PhantomCallbackData<T>(Isolate* isolate, T* parameter)
-      : internal::CallbackData(isolate), parameter_(parameter) {}
+  PhantomCallbackData(Isolate* isolate, T* parameter, U* internal_field1,
+                      V* internal_field2)
+      : internal::CallbackData<T>(isolate, parameter),
+        internal_field1_(internal_field1),
+        internal_field2_(internal_field2) {}
 
  private:
-  T* parameter_;
+  U* internal_field1_;
+  V* internal_field2_;
 };
 
 
 template <class T, class P>
-class WeakCallbackData : public PhantomCallbackData<P> {
+class WeakCallbackData : public internal::CallbackData<P> {
  public:
   typedef void (*Callback)(const WeakCallbackData<T, P>& data);
 
   V8_INLINE Local<T> GetValue() const { return handle_; }
 
  private:
   friend class internal::GlobalHandles;
   WeakCallbackData(Isolate* isolate, P* parameter, Local<T> handle)
-      : PhantomCallbackData<P>(isolate, parameter), handle_(handle) {}
+      : internal::CallbackData<P>(isolate, parameter), handle_(handle) {}
   Local<T> handle_;
 };
 
 
-template <typename T, typename U>
-class InternalFieldsCallbackData : public internal::CallbackData {
- public:
-  typedef void (*Callback)(const InternalFieldsCallbackData<T, U>& data);
-
-  InternalFieldsCallbackData(Isolate* isolate, T* internalField1,
-                             U* internalField2)
-      : internal::CallbackData(isolate),
-        internal_field1_(internalField1),
-        internal_field2_(internalField2) {}
-
-  V8_INLINE T* GetInternalField1() const { return internal_field1_; }
-  V8_INLINE U* GetInternalField2() const { return internal_field2_; }
-
- private:
-  T* internal_field1_;
-  U* internal_field2_;
-};
-
-
 /**
  * An object reference that is independent of any handle scope.  Where
  * a Local handle only lives as long as the HandleScope in which it was
  * allocated, a PersistentBase handle remains valid until it is explicitly
  * disposed.
  *
  * A persistent handle contains a reference to a storage cell within
  * the v8 engine which holds an object value and which is updated by
  * the garbage collector whenever the object is moved.  A new storage
  * cell can be created using the constructor or PersistentBase::Reset and
@@ skipping 51 matching lines @@
   }
 
   /**
    *  Install a finalization callback on this object.
    *  NOTE: There is no guarantee as to *when* or even *if* the callback is
    *  invoked. The invocation is performed solely on a best effort basis.
    *  As always, GC-based finalization should *not* be relied upon for any
    *  critical form of resource management!
    */
   template<typename P>
   V8_INLINE void SetWeak(

[dcarney, 2015/01/09 14:22:52]

another follow up:

make two new SetWeak(P*, WeakCallbackData<T, P>::Callback, WeakCallbackType)
functions where WeakCallbackType contains the four relevant types and mark these
functions as needing deprecation

[Erik Corry Chromium.org, 2015/01/12 11:37:53]

Again, I'd rather not move to this more dynamic version where it is harder to
write asserts.

       P* parameter,
       typename WeakCallbackData<T, P>::Callback callback);
 
   template<typename S, typename P>
   V8_INLINE void SetWeak(
       P* parameter,
       typename WeakCallbackData<S, P>::Callback callback);
 
   // Phantom persistents work like weak persistents, except that the pointer to
   // the object being collected is not available in the finalization callback.
   // This enables the garbage collector to collect the object and any objects
   // it references transitively in one GC cycle. At the moment you can either
   // specify a parameter for the callback or the location of two internal
   // fields in the dying object.
   template <typename P>
-  V8_INLINE void SetPhantom(P* parameter,
-                            typename PhantomCallbackData<P>::Callback callback);
-
+  V8_INLINE void SetPhantom(
+      P* parameter,
+      typename PhantomCallbackData<P, void, void>::Callback callback);
   template <typename P, typename Q>
   V8_INLINE void SetPhantom(
-      void (*callback)(const InternalFieldsCallbackData<P, Q>&),
-      int internal_field_index1, int internal_field_index2);
+      P* parameter, int internal_field_index1,

[dcarney, 2015/01/09 14:22:52]

i'm not sure i see the point in setting these fields, but since the api is in
flux, that can wait for a follow up

[Erik Corry Chromium.org, 2015/01/12 11:37:53]

The code is not setting any fields from these parameters; the index numbers are
passed so we can assert on them in V8.

+      typename PhantomCallbackData<P, Q, void>::Callback callback);
+  template <typename P, typename Q, typename R>
+  V8_INLINE void SetPhantom(
+      P* parameter, int internal_field_index1, int internal_field_index2,
+      typename PhantomCallbackData<P, Q, R>::Callback callback);
 
   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
    * to ignore any object groups containing this object. Weak callback for an
@@ skipping 5006 matching lines @@
 
   enum WeakHandleType { PhantomHandle, NonphantomHandle };
 
   static internal::Object** GlobalizeReference(internal::Isolate* isolate,
                                                internal::Object** handle);
   static internal::Object** CopyPersistent(internal::Object** handle);
   static void DisposeGlobal(internal::Object** global_handle);
   typedef WeakCallbackData<Value, void>::Callback WeakCallback;
   static void MakeWeak(internal::Object** global_handle, void* data,
                        WeakCallback weak_callback);
-  static void MakePhantom(internal::Object** global_handle, void* data,
-                          PhantomCallbackData<void>::Callback weak_callback);
   static void MakePhantom(
-      internal::Object** global_handle,
-      InternalFieldsCallbackData<void, void>::Callback weak_callback,
+      internal::Object** global_handle, void* data,
+      // Must be 0 or kNoInternalFieldIndex.
       int internal_field_index1,
-      int internal_field_index2 = Object::kNoInternalFieldIndex);
+      // Must be 1 or kNoInternalFieldIndex.
+      int internal_field_index2,
+      PhantomCallbackData<void, void, void>::Callback weak_callback);
   static void* ClearWeak(internal::Object** global_handle);
   static void Eternalize(Isolate* isolate,
                          Value* handle,
                          int* index);
   static Local<Value> GetEternal(Isolate* isolate, int index);
 
   template <class T> friend class Handle;
   template <class T> friend class Local;
   template <class T> friend class Eternal;
   template <class T> friend class PersistentBase;
@@ skipping 867 matching lines @@
 void PersistentBase<T>::SetWeak(
     P* parameter,
     typename WeakCallbackData<T, P>::Callback callback) {
   SetWeak<T, P>(parameter, callback);
 }
 
 
 template <class T>
 template <typename P>
 void PersistentBase<T>::SetPhantom(
-    P* parameter, typename PhantomCallbackData<P>::Callback callback) {
-  typedef typename PhantomCallbackData<void>::Callback Callback;
+    P* parameter,
+    typename PhantomCallbackData<P, void, void>::Callback callback) {
+  typedef typename PhantomCallbackData<void, void, void>::Callback Callback;
   V8::MakePhantom(reinterpret_cast<internal::Object**>(this->val_), parameter,
+                  Object::kNoInternalFieldIndex, Object::kNoInternalFieldIndex,
                   reinterpret_cast<Callback>(callback));
 }
 
 
 template <class T>
-template <typename U, typename V>
+template <typename P, typename Q>
 void PersistentBase<T>::SetPhantom(
-    void (*callback)(const InternalFieldsCallbackData<U, V>&),
-    int internal_field_index1, int internal_field_index2) {
-  typedef typename InternalFieldsCallbackData<void, void>::Callback Callback;
-  V8::MakePhantom(reinterpret_cast<internal::Object**>(this->val_),
-                  reinterpret_cast<Callback>(callback), internal_field_index1,
-                  internal_field_index2);
+    P* parameter, int internal_field_index1,
+    typename PhantomCallbackData<P, Q, void>::Callback callback) {
+  typedef typename PhantomCallbackData<void, void, void>::Callback Callback;
+  V8::MakePhantom(reinterpret_cast<internal::Object**>(this->val_), parameter,
+                  internal_field_index1, Object::kNoInternalFieldIndex,
+                  reinterpret_cast<Callback>(callback));
 }
 
 
+template <class T>
+template <typename P, typename Q, typename R>
+void PersistentBase<T>::SetPhantom(
+    P* parameter, int internal_field_index1, int internal_field_index2,
+    typename PhantomCallbackData<P, Q, R>::Callback callback) {
+  typedef typename PhantomCallbackData<void, void, void>::Callback Callback;
+  V8::MakePhantom(reinterpret_cast<internal::Object**>(this->val_), parameter,
+                  internal_field_index1, internal_field_index2,
+                  reinterpret_cast<Callback>(callback));
+}
+
+
 template <class T>
 template <typename P>
 P* PersistentBase<T>::ClearWeak() {
   return reinterpret_cast<P*>(
     V8::ClearWeak(reinterpret_cast<internal::Object**>(this->val_)));
 }
 
 
 template <class T>
 void PersistentBase<T>::MarkIndependent() {
@@ skipping 973 matching lines @@
  */
 
 
 }  // namespace v8
 
 
 #undef TYPE_CHECK
 
 
 #endif  // V8_H_