collect phantom handle data before it gets overwritten

R=jochen@chromium.org, hpayer@chromium.org, erikcorry@chromium.org

BUG=

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

Cr-Commit-Position: refs/heads/master@{#27786}

src/global-handles.cc

@@ -169,12 +169,16 @@ class GlobalHandles::Node {
 
   bool IsInUse() const { return state() != FREE; }
 
-  bool IsRetainer() const { return state() != FREE; }
+  bool IsRetainer() const {
+    return state() != FREE &&
+           !(state() == NEAR_DEATH && weakness_type() != NORMAL_WEAK);
+  }
 
   bool IsStrongRetainer() const { return state() == NORMAL; }
 
   bool IsWeakRetainer() const {
-    return state() == WEAK || state() == PENDING || state() == NEAR_DEATH;
+    return state() == WEAK || state() == PENDING ||
+           (state() == NEAR_DEATH && weakness_type() == NORMAL_WEAK);
   }
 
   void MarkPending() {
@@ -260,35 +264,31 @@ class GlobalHandles::Node {
   void CollectPhantomCallbackData(
       Isolate* isolate,
       List<PendingPhantomCallback>* pending_phantom_callbacks) {
-    if (state() != PENDING) return;
-    if (weak_callback_ != NULL) {
-      if (weakness_type() == NORMAL_WEAK) return;
-
-      DCHECK(weakness_type() == PHANTOM_WEAK ||
-             weakness_type() == PHANTOM_WEAK_2_INTERNAL_FIELDS);
-
-      void* internal_fields[v8::kInternalFieldsInWeakCallback] = {nullptr,
-                                                                  nullptr};
-      if (weakness_type() != PHANTOM_WEAK && object()->IsJSObject()) {
-        auto jsobject = JSObject::cast(object());
-        int field_count = jsobject->GetInternalFieldCount();
-        for (int i = 0; i < v8::kInternalFieldsInWeakCallback; ++i) {
-          if (field_count == i) break;
-          auto field = jsobject->GetInternalField(i);
-          if (field->IsSmi()) internal_fields[i] = field;
-        }
+    DCHECK(weakness_type() == PHANTOM_WEAK ||
+           weakness_type() == PHANTOM_WEAK_2_INTERNAL_FIELDS);
+    DCHECK(state() == PENDING);
+
+    void* internal_fields[v8::kInternalFieldsInWeakCallback] = {nullptr,
+                                                                nullptr};
+    if (weakness_type() != PHANTOM_WEAK && object()->IsJSObject()) {
+      auto jsobject = JSObject::cast(object());
+      int field_count = jsobject->GetInternalFieldCount();
+      for (int i = 0; i < v8::kInternalFieldsInWeakCallback; ++i) {
+        if (field_count == i) break;
+        auto field = jsobject->GetInternalField(i);
+        if (field->IsSmi()) internal_fields[i] = field;
       }
+    }
 
-      // Zap with something dangerous.
-      *location() = reinterpret_cast<Object*>(0x6057ca11);
+    // Zap with something dangerous.
+    *location() = reinterpret_cast<Object*>(0x6057ca11);
 
-      typedef v8::WeakCallbackInfo<void> Data;
-      auto callback = reinterpret_cast<Data::Callback>(weak_callback_);
-      pending_phantom_callbacks->Add(
-          PendingPhantomCallback(this, callback, parameter(), internal_fields));
-      DCHECK(IsInUse());
-      set_state(NEAR_DEATH);
-    }
+    typedef v8::WeakCallbackInfo<void> Data;
+    auto callback = reinterpret_cast<Data::Callback>(weak_callback_);
+    pending_phantom_callbacks->Add(
+        PendingPhantomCallback(this, callback, parameter(), internal_fields));
+    DCHECK(IsInUse());
+    set_state(NEAR_DEATH);
   }
 
   bool PostGarbageCollectionProcessing(Isolate* isolate) {
@@ -562,23 +562,6 @@ void GlobalHandles::MakeWeak(Object** location, void* parameter,
 }
 
 
-void GlobalHandles::CollectAllPhantomCallbackData() {
-  for (NodeIterator it(this); !it.done(); it.Advance()) {
-    Node* node = it.node();
-    node->CollectPhantomCallbackData(isolate(), &pending_phantom_callbacks_);
-  }
-}
-
-
-void GlobalHandles::CollectYoungPhantomCallbackData() {
-  for (int i = 0; i < new_space_nodes_.length(); ++i) {
-    Node* node = new_space_nodes_[i];
-    DCHECK(node->is_in_new_space_list());
-    node->CollectPhantomCallbackData(isolate(), &pending_phantom_callbacks_);
-  }
-}
-
-
 void* GlobalHandles::ClearWeakness(Object** location) {
   return Node::FromLocation(location)->ClearWeakness();
 }
@@ -616,18 +599,12 @@ void GlobalHandles::IterateWeakRoots(ObjectVisitor* v) {
       // Weakness type can be normal or phantom, with or without internal
       // fields).  For normal weakness we mark through the handle so that the
       // object and things reachable from it are available to the callback.
-      //
-      // In the case of phantom with no internal fields, we can zap the object
-      // handle now and we won't need it, so we don't need to mark through it.
-      // In the internal fields case we will need the internal
-      // fields, so we can't zap the handle.
       if (node->state() == Node::PENDING) {
-        if (node->weakness_type() == PHANTOM_WEAK) {
-          *(node->location()) = Smi::FromInt(0);
-        } else if (node->weakness_type() == NORMAL_WEAK) {
+        if (node->weakness_type() == NORMAL_WEAK) {
           v->VisitPointer(node->location());
         } else {
-          DCHECK(node->weakness_type() == PHANTOM_WEAK_2_INTERNAL_FIELDS);
+          node->CollectPhantomCallbackData(isolate(),
+                                           &pending_phantom_callbacks_);
         }
       } else {
         // Node is not pending, so that means the object survived.  We still
@@ -680,19 +657,11 @@ void GlobalHandles::IterateNewSpaceWeakIndependentRoots(ObjectVisitor* v) {
     DCHECK(node->is_in_new_space_list());
     if ((node->is_independent() || node->is_partially_dependent()) &&
         node->IsWeakRetainer()) {
-      if (node->weakness_type() == PHANTOM_WEAK) {
-        *(node->location()) = Smi::FromInt(0);
-      } else if (node->weakness_type() == NORMAL_WEAK) {
+      if (node->weakness_type() == NORMAL_WEAK) {
         v->VisitPointer(node->location());
-      } else {
-        DCHECK(node->weakness_type() == PHANTOM_WEAK_2_INTERNAL_FIELDS);
-        // For this case we only need to trace if it's alive: The tracing of
-        // something that is already alive is just to get the pointer updated
-        // to the new location of the object).
-        DCHECK(node->state() != Node::NEAR_DEATH);
-        if (node->state() != Node::PENDING) {
-          v->VisitPointer(node->location());
-        }
+      } else if (node->state() == Node::PENDING) {
+        node->CollectPhantomCallbackData(isolate(),
+                                         &pending_phantom_callbacks_);
       }
     }
   }

src/global-handles.h

@@ -162,14 +162,6 @@ class GlobalHandles {
     return number_of_global_handles_;
   }
 
-  // Collect up data for the weak handle callbacks after GC has completed, but
-  // before memory is reclaimed.
-  void CollectAllPhantomCallbackData();
-
-  // Collect up data for the weak handle callbacks referenced by young
-  // generation after GC has completed, but before memory is reclaimed.
-  void CollectYoungPhantomCallbackData();
-
   // Clear the weakness of a global handle.
   static void* ClearWeakness(Object** location);
 

src/heap/heap.cc

@@ -1573,11 +1573,6 @@ void Heap::Scavenge() {
   ScavengeWeakObjectRetainer weak_object_retainer(this);
   ProcessYoungWeakReferences(&weak_object_retainer);
 
-  // Collects callback info for handles referenced by young generation that are
-  // pending (about to be collected) and either phantom or internal-fields.
-  // Releases the global handles.  See also PostGarbageCollectionProcessing.
-  isolate()->global_handles()->CollectYoungPhantomCallbackData();
-
   DCHECK(new_space_front == new_space_.top());
 
   // Set age mark.

src/heap/mark-compact.cc

@@ -3800,11 +3800,6 @@ void MarkCompactCollector::EvacuateNewSpaceAndCandidates() {
   EvacuationWeakObjectRetainer evacuation_object_retainer;
   heap()->ProcessAllWeakReferences(&evacuation_object_retainer);
 
-  // Collects callback info for handles that are pending (about to be
-  // collected) and either phantom or internal-fields.  Releases the global
-  // handles.  See also PostGarbageCollectionProcessing.
-  isolate()->global_handles()->CollectAllPhantomCallbackData();
-
   // Visit invalidated code (we ignored all slots on it) and clear mark-bits
   // under it.
   ProcessInvalidatedCode(&updating_visitor);