Phantom references support internal fields

include/v8.h

Index: include/v8.h
diff --git a/include/v8.h b/include/v8.h
index 21911b460db34b23e5f89a0b1beaa4e221a0f37c..d7f0708b46826f6b7eaa727d26db971f58348af4 100644
--- a/include/v8.h
+++ b/include/v8.h
@@ -418,22 +418,57 @@ template <class T> class Eternal {
 };
 
 
+template<class T>
+class CallbackData {

[rmcilroy, 2014/12/03 11:56:35]

nit - BaseCallbackData or maybe even BaseWeakCallbackData? 

Would it make sense to put this in a non-public namespace to make it obvious
this isn't intended to be used as part of the external API.

[Erik Corry, 2014/12/15 15:12:41]

Done.

+ public:
+  V8_INLINE Isolate* GetIsolate() const { return isolate_; }
+  V8_INLINE T* GetParameter() const { return parameter_; }
+
+ protected:
+  CallbackData(Isolate* isolate, T* parameter)
+    : isolate_(isolate), parameter_(parameter) { }
+
+ private:
+  Isolate* isolate_;
+  T* parameter_;
+};
+

[rmcilroy, 2014/12/03 11:56:35]

nit - two newlines between classes in V8

[Erik Corry, 2014/12/15 15:12:41]

Done.

 template<class T, class P>
-class WeakCallbackData {
+class WeakCallbackData : public CallbackData<P> {
  public:
   typedef void (*Callback)(const WeakCallbackData<T, P>& data);
 
-  V8_INLINE Isolate* GetIsolate() const { return isolate_; }
   V8_INLINE Local<T> GetValue() const { return handle_; }
-  V8_INLINE P* GetParameter() const { return parameter_; }
 
  private:
   friend class internal::GlobalHandles;
   WeakCallbackData(Isolate* isolate, Local<T> handle, P* parameter)
-    : isolate_(isolate), handle_(handle), parameter_(parameter) { }
-  Isolate* isolate_;
+    : CallbackData<P>(isolate, parameter), handle_(handle) { }
   Local<T> handle_;
-  P* parameter_;
+};
+
+
+template<typename T>
+class PhantomCallbackData : public CallbackData<T> {
+ public:
+  typedef void (*Callback)(const PhantomCallbackData<T>& data);
+
+  V8_INLINE void* GetInternalField1() const { return internal_field1_; }
+  V8_INLINE void* GetInternalField2() const { return internal_field2_; }
+
+ private:
+  friend class internal::GlobalHandles;
+  PhantomCallbackData(Isolate* isolate, void* internalField1,
+                      void* internalField2)
+    : CallbackData<T>(isolate, NULL),

[rmcilroy, 2014/12/03 11:56:35]

Do we want to expose NULL parameters to PhantomCallbacks - it feels like that
could get error prone if the caller has to know whether to check if
GetParameter() returns null (maybe this is already the case though)?  Would it
make sense to have different callbacks for either the internal field version and
the parameter version?

[Erik Corry, 2014/12/15 15:12:41]

Fixed to pass a type that has only valid fields to the callback.

+      internal_field1_(internalField1),
+      internal_field2_(internalField2) { }
+  PhantomCallbackData(Isolate* isolate, T* parameter)
+    : CallbackData<T>(isolate, parameter),
+      internal_field1_(NULL),
+      internal_field2_(NULL) { }
+  void* internal_field1_;
+  void* internal_field2_;
 };
 
 
@@ -471,22 +506,23 @@ template <class T> class PersistentBase {
   template <class S>
   V8_INLINE void Reset(Isolate* isolate, const PersistentBase<S>& other);
 
-  V8_INLINE bool IsEmpty() const { return val_ == 0; }
+  V8_INLINE bool IsEmpty() const { return val_ == NULL; }
+  V8_INLINE void Empty() { val_ = NULL; }

[jochen (gone - plz use gerrit), 2014/12/02 10:25:59]

what do you need Empty() for?

[Erik Corry, 2014/12/02 10:40:48]

It seems I don't need it any more.  Removed.

[Erik Corry, 2014/12/15 15:12:41]

Actually I need this for the corresponding Blink change.  In the callback for
the scriptwrappable we empty the handle, which has been zapped on the V8 side.
This means we don't try to touch it from the destructor later.

 
   template <class S>
   V8_INLINE bool operator==(const PersistentBase<S>& that) const {
     internal::Object** a = reinterpret_cast<internal::Object**>(this->val_);
     internal::Object** b = reinterpret_cast<internal::Object**>(that.val_);
-    if (a == 0) return b == 0;
-    if (b == 0) return false;
+    if (a == NULL) return b == NULL;
+    if (b == NULL) return false;
     return *a == *b;
   }
 
   template <class S> V8_INLINE bool operator==(const Handle<S>& that) const {
     internal::Object** a = reinterpret_cast<internal::Object**>(this->val_);
     internal::Object** b = reinterpret_cast<internal::Object**>(that.val_);
-    if (a == 0) return b == 0;
-    if (b == 0) return false;
+    if (a == NULL) return b == NULL;
+    if (b == NULL) return false;
     return *a == *b;
   }
 
@@ -521,12 +557,12 @@ template <class T> class PersistentBase {
   // This enables the garbage collector to collect the object and any objects
   // it references transitively in one GC cycle.
   template <typename P>
-  V8_INLINE void SetPhantom(P* parameter,
-                            typename WeakCallbackData<T, P>::Callback callback);
+  V8_INLINE void SetPhantom(
+      P* parameter, typename PhantomCallbackData<P>::Callback callback);
 
-  template <typename S, typename P>
-  V8_INLINE void SetPhantom(P* parameter,
-                            typename WeakCallbackData<S, P>::Callback callback);
+  V8_INLINE void SetPhantom(
+      typename PhantomCallbackData<void>::Callback callback,
+      int internalFieldOffset1, int internalFieldOffset2);

[jochen (gone - plz use gerrit), 2014/12/02 10:25:59]

internal_field_index1

[Erik Corry, 2014/12/02 10:40:48]

Done.

 
   template<typename P>
   V8_INLINE P* ClearWeak();
@@ -2483,6 +2519,8 @@ class V8_EXPORT Object : public Value {
   /** Gets the number of internal fields for this Object. */
   int InternalFieldCount();
 
+  static const int kNoInternalFieldIndex = -1;
+
   /** Same as above, but works for Persistents */
   V8_INLINE static int InternalFieldCount(
       const PersistentBase<Object>& object) {
@@ -5487,8 +5525,14 @@ class V8_EXPORT V8 {
   static internal::Object** CopyPersistent(internal::Object** handle);
   static void DisposeGlobal(internal::Object** global_handle);
   typedef WeakCallbackData<Value, void>::Callback WeakCallback;
+  typedef PhantomCallbackData<void>::Callback PhantomCallback;
   static void MakeWeak(internal::Object** global_handle, void* data,
-                       WeakCallback weak_callback, WeakHandleType phantom);
+                       WeakCallback weak_callback);
+  static void MakePhantom(
+      internal::Object** global_handle, void* data,
+      PhantomCallback weak_callback,
+      int internal_field_index1 = Object::kNoInternalFieldIndex,
+      int internal_field_index2 = Object::kNoInternalFieldIndex);
   static void* ClearWeak(internal::Object** global_handle);
   static void Eternalize(Isolate* isolate,
                          Value* handle,
@@ -6097,12 +6141,12 @@ class Internals {
 
   static const int kNodeClassIdOffset = 1 * kApiPointerSize;
   static const int kNodeFlagsOffset = 1 * kApiPointerSize + 3;
-  static const int kNodeStateMask = 0xf;
+  static const int kNodeStateMask = 0x7;
   static const int kNodeStateIsWeakValue = 2;
   static const int kNodeStateIsPendingValue = 3;
   static const int kNodeStateIsNearDeathValue = 4;
-  static const int kNodeIsIndependentShift = 4;
-  static const int kNodeIsPartiallyDependentShift = 5;
+  static const int kNodeIsIndependentShift = 3;
+  static const int kNodeIsPartiallyDependentShift = 4;
 
   static const int kJSObjectType = 0xbd;
   static const int kFirstNonstringType = 0x80;
@@ -6360,7 +6404,7 @@ void PersistentBase<T>::SetWeak(
   TYPE_CHECK(S, T);
   typedef typename WeakCallbackData<Value, void>::Callback Callback;
   V8::MakeWeak(reinterpret_cast<internal::Object**>(this->val_), parameter,
-               reinterpret_cast<Callback>(callback), V8::NonphantomHandle);
+               reinterpret_cast<Callback>(callback));
 }
 
 
@@ -6374,21 +6418,24 @@ void PersistentBase<T>::SetWeak(
 
 
 template <class T>
-template <typename S, typename P>
+template <typename P>
 void PersistentBase<T>::SetPhantom(
-    P* parameter, typename WeakCallbackData<S, P>::Callback callback) {
-  TYPE_CHECK(S, T);
-  typedef typename WeakCallbackData<Value, void>::Callback Callback;
-  V8::MakeWeak(reinterpret_cast<internal::Object**>(this->val_), parameter,
-               reinterpret_cast<Callback>(callback), V8::PhantomHandle);
+    P* parameter, typename PhantomCallbackData<P>::Callback callback) {
+  typedef typename PhantomCallbackData<void>::Callback Callback;
+  V8::MakePhantom(reinterpret_cast<internal::Object**>(this->val_), parameter,
+                  reinterpret_cast<Callback>(callback),
+                  Object::kNoInternalFieldIndex, Object::kNoInternalFieldIndex);
 }
 
 
 template <class T>
-template <typename P>
 void PersistentBase<T>::SetPhantom(
-    P* parameter, typename WeakCallbackData<T, P>::Callback callback) {
-  SetPhantom<T, P>(parameter, callback);
+    typename PhantomCallbackData<void>::Callback callback,
+    int internal_field_index1, int internal_field_index2) {
+  typedef typename PhantomCallbackData<void>::Callback Callback;
+  V8::MakePhantom(reinterpret_cast<internal::Object**>(this->val_), 0,
+                  reinterpret_cast<Callback>(callback), internal_field_index1,
+                  internal_field_index2);
 }