Introduce phantom weak handles in the API and use them internally for debug info

src/global-handles.cc

Index: src/global-handles.cc
diff --git a/src/global-handles.cc b/src/global-handles.cc
index 282ca2d5d86a63d86dd00b1eb4b43888c3289e6b..574b2489ea2bc1e21dc8d917a2935700a050bdee 100644
--- a/src/global-handles.cc
+++ b/src/global-handles.cc
@@ -146,6 +146,13 @@ class GlobalHandles::Node {
     flags_ = IsInNewSpaceList::update(flags_, v);
   }
 
+  bool is_zapped_during_weak_callback() {
+    return IsZappedDuringWeakCallback::decode(flags_);
+  }
+  void set_is_zapped_during_weak_callback(bool v) {
+    flags_ = IsZappedDuringWeakCallback::update(flags_, v);
+  }
+
   bool IsNearDeath() const {
     // Check for PENDING to ensure correct answer when processing callbacks.
     return state() == PENDING || state() == NEAR_DEATH;
@@ -204,12 +211,14 @@ class GlobalHandles::Node {
     parameter_or_next_free_.next_free = value;
   }
 
-  void MakeWeak(void* parameter, WeakCallback weak_callback) {
+  void MakeWeak(void* parameter, WeakCallback weak_callback,
+                bool is_zapped_during_weak_callback = false) {
     DCHECK(weak_callback != NULL);
     DCHECK(state() != FREE);
     CHECK(object_ != NULL);
     set_state(WEAK);
     set_parameter(parameter);
+    set_is_zapped_during_weak_callback(is_zapped_during_weak_callback);
     weak_callback_ = weak_callback;
   }
 
@@ -227,7 +236,7 @@ class GlobalHandles::Node {
       Release();
       return false;
     }
-    void* par = parameter();
+    void* param = parameter();
     set_state(NEAR_DEATH);
     set_parameter(NULL);
 
@@ -240,14 +249,28 @@ class GlobalHandles::Node {
       DCHECK(!object_->IsExternalTwoByteString() ||
              ExternalTwoByteString::cast(object_)->resource() != NULL);
       // Leaving V8.
-      VMState<EXTERNAL> state(isolate);
+      VMState<EXTERNAL> vmstate(isolate);
       HandleScope handle_scope(isolate);
-      Handle<Object> handle(*object, isolate);
-      v8::WeakCallbackData<v8::Value, void> data(
-          reinterpret_cast<v8::Isolate*>(isolate),
-          v8::Utils::ToLocal(handle),
-          par);
-      weak_callback_(data);
+      if (is_zapped_during_weak_callback()) {
+        // Phantom weak pointer case.
+        DCHECK(*object == Smi::FromInt(kPhantomReferenceZap));
+        // Make data with a null handle.
+        v8::WeakCallbackData<v8::Value, void> data(
+            reinterpret_cast<v8::Isolate*>(isolate), v8::Local<v8::Object>(),
+            param);
+        weak_callback_(data);
+        if (state() != FREE) {
+          // Callback does not have to clear the global handle if it is a
+          // phantom handle.
+          Release();
+        }
+      } else {
+        Handle<Object> handle(*object, isolate);

[Erik Corry, 2014/11/02 18:41:45]

This CL is a reland of https://codereview.chromium.org/649563006/

This line has been changed from

Handle<Object> handle(object).

That caused the new handle to have the same backing global handle slot, which
didn't work with Blink, where some of the bindings reset the GetParameter handle
and then expect to still be able to use the GetValue handle.

I added a new test in test-api.cc, so that V8's own tests can catch this in
future (ResetWeakHandle).

+        v8::WeakCallbackData<v8::Value, void> data(
+            reinterpret_cast<v8::Isolate*>(isolate), v8::Utils::ToLocal(handle),
+            param);
+        weak_callback_(data);
+      }
     }
     // Absence of explicit cleanup or revival of weak handle
     // in most of the cases would lead to memory leak.
@@ -277,10 +300,11 @@ class GlobalHandles::Node {
 
   // This stores three flags (independent, partially_dependent and
   // in_new_space_list) and a State.
-  class NodeState:            public BitField<State, 0, 4> {};
-  class IsIndependent:        public BitField<bool,  4, 1> {};
-  class IsPartiallyDependent: public BitField<bool,  5, 1> {};
-  class IsInNewSpaceList:     public BitField<bool,  6, 1> {};
+  class NodeState : public BitField<State, 0, 4> {};
+  class IsIndependent : public BitField<bool, 4, 1> {};
+  class IsPartiallyDependent : public BitField<bool, 5, 1> {};
+  class IsInNewSpaceList : public BitField<bool, 6, 1> {};
+  class IsZappedDuringWeakCallback : public BitField<bool, 7, 1> {};
 
   uint8_t flags_;
 
@@ -475,10 +499,10 @@ void GlobalHandles::Destroy(Object** location) {
 }
 
 
-void GlobalHandles::MakeWeak(Object** location,
-                             void* parameter,
-                             WeakCallback weak_callback) {
-  Node::FromLocation(location)->MakeWeak(parameter, weak_callback);
+void GlobalHandles::MakeWeak(Object** location, void* parameter,
+                             WeakCallback weak_callback, PhantomState phantom) {
+  Node::FromLocation(location)
+      ->MakeWeak(parameter, weak_callback, phantom == Phantom);
 }
 
 
@@ -514,7 +538,15 @@ bool GlobalHandles::IsWeak(Object** location) {
 
 void GlobalHandles::IterateWeakRoots(ObjectVisitor* v) {
   for (NodeIterator it(this); !it.done(); it.Advance()) {
-    if (it.node()->IsWeakRetainer()) v->VisitPointer(it.node()->location());
+    Node* node = it.node();
+    if (node->IsWeakRetainer()) {
+      if (node->state() == Node::PENDING &&
+          node->is_zapped_during_weak_callback()) {
+        *(node->location()) = Smi::FromInt(kPhantomReferenceZap);
+      } else {
+        v->VisitPointer(node->location());
+      }
+    }
   }
 }
 
@@ -559,7 +591,11 @@ void GlobalHandles::IterateNewSpaceWeakIndependentRoots(ObjectVisitor* v) {
     DCHECK(node->is_in_new_space_list());
     if ((node->is_independent() || node->is_partially_dependent()) &&
         node->IsWeakRetainer()) {
-      v->VisitPointer(node->location());
+      if (node->is_zapped_during_weak_callback()) {
+        *(node->location()) = Smi::FromInt(kPhantomReferenceZap);
+      } else {
+        v->VisitPointer(node->location());
+      }
     }
   }
 }