Reland "cppgc-js: heap snapshot: Add logic for querying detachedness"

This is a reland of e68285e2

Failing wasm tests seemed to recover on their own.

Original change's description:
> cppgc-js: heap snapshot: Add logic for querying detachedness
>
> Adds infrastructure to allow embedders specifying a detachedness state
> that is queried when encountering an object with a TraceReference that
> has a non-zero wrapper class id set.
>
> Change-Id: Ie7f2f253544ee25a25565eb08d82e9df5f0a74d2
> Bug: chromium:1056170
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2502345
> Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
> Reviewed-by: Omer Katz <omerkatz@chromium.org>
> Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#70841}

Bug: chromium:1056170
Change-Id: I293a9d38f841b4d0faa4af7408bb57544f11d566
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2505713
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Omer Katz <omerkatz@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70867}

include/v8-profiler.h

@@ -817,6 +817,18 @@ class V8_EXPORT HeapProfiler {
                                              v8::EmbedderGraph* graph,
                                              void* data);
 
+  /**
+   * Callback function invoked during heap snapshot generation to retrieve
+   * the detachedness state of an object referenced by a TracedReference.
+   *
+   * The callback takes Local<Value> as parameter to allow the embedder to
+   * unpack the TracedReference into a Local and reuse that Local for different
+   * purposes.
+   */
+  using GetDetachednessCallback = EmbedderGraph::Node::Detachedness (*)(
+      v8::Isolate* isolate, const v8::Local<v8::Value>& v8_value,
+      uint16_t class_id, void* data);
+
   /** Returns the number of snapshots taken. */
   int GetSnapshotCount();
 
@@ -967,6 +979,8 @@ class V8_EXPORT HeapProfiler {
   void RemoveBuildEmbedderGraphCallback(BuildEmbedderGraphCallback callback,
                                         void* data);
 
+  void SetGetDetachednessCallback(GetDetachednessCallback callback, void* data);
+
   /**
    * Default value of persistent handle class ID. Must not be used to
    * define a class. Can be used to reset a class of a persistent

src/api/api.cc

@@ -11028,6 +11028,12 @@ void HeapProfiler::RemoveBuildEmbedderGraphCallback(
       callback, data);
 }
 
+void HeapProfiler::SetGetDetachednessCallback(GetDetachednessCallback callback,
+                                              void* data) {
+  reinterpret_cast<i::HeapProfiler*>(this)->SetGetDetachednessCallback(callback,
+                                                                       data);
+}
+
 void EmbedderHeapTracer::SetStackStart(void* stack_start) {
   CHECK(isolate_);
   reinterpret_cast<i::Isolate*>(isolate_)->global_handles()->SetStackStart(

src/heap/cppgc-js/cpp-snapshot.cc

@@ -45,9 +45,15 @@ class EmbedderNode : public v8::EmbedderGraph::Node {
   }
   Node* WrapperNode() final { return wrapper_node_; }
 
+  void SetDetachedness(Detachedness detachedness) {
+    detachedness_ = detachedness;
+  }
+  Detachedness GetDetachedness() final { return detachedness_; }
+
  private:
   const char* name_;
   Node* wrapper_node_ = nullptr;
+  Detachedness detachedness_ = Detachedness::kUnknown;
 };
 
 // Node representing an artificial root group, e.g., set of Persistent handles.
@@ -405,6 +411,13 @@ class CppGraphBuilderImpl final {
               reinterpret_cast<v8::internal::Isolate*>(cpp_heap_.isolate()),
               v8_value, parent.header()->Payload())) {
         parent.get_node()->SetWrapperNode(v8_node);
+
+        auto* profiler =
+            reinterpret_cast<Isolate*>(cpp_heap_.isolate())->heap_profiler();
+        if (profiler->HasGetDetachednessCallback()) {
+          parent.get_node()->SetDetachedness(
+              profiler->GetDetachedness(v8_value, ref.WrapperClassId()));
+        }
       }
     }
   }

src/profiler/heap-profiler.cc

@@ -64,6 +64,19 @@ void HeapProfiler::BuildEmbedderGraph(Isolate* isolate,
   }
 }
 
+void HeapProfiler::SetGetDetachednessCallback(
+    v8::HeapProfiler::GetDetachednessCallback callback, void* data) {
+  get_detachedness_callback_ = {callback, data};
+}
+
+v8::EmbedderGraph::Node::Detachedness HeapProfiler::GetDetachedness(
+    const v8::Local<v8::Value> v8_value, uint16_t class_id) {
+  DCHECK(HasGetDetachednessCallback());
+  return get_detachedness_callback_.first(
+      reinterpret_cast<v8::Isolate*>(heap()->isolate()), v8_value, class_id,
+      get_detachedness_callback_.second);
+}
+
 HeapSnapshot* HeapProfiler::TakeSnapshot(
     v8::ActivityControl* control,
     v8::HeapProfiler::ObjectNameResolver* resolver,

src/profiler/heap-profiler.h

@@ -72,6 +72,14 @@ class HeapProfiler : public HeapObjectAllocationTracker {
     return !build_embedder_graph_callbacks_.empty();
   }
 
+  void SetGetDetachednessCallback(
+      v8::HeapProfiler::GetDetachednessCallback callback, void* data);
+  bool HasGetDetachednessCallback() const {
+    return get_detachedness_callback_.first != nullptr;
+  }
+  v8::EmbedderGraph::Node::Detachedness GetDetachedness(
+      const v8::Local<v8::Value> v8_value, uint16_t class_id);
+
   bool is_tracking_object_moves() const { return is_tracking_object_moves_; }
 
   Handle<HeapObject> FindHeapObjectById(SnapshotObjectId id);
@@ -99,6 +107,8 @@ class HeapProfiler : public HeapObjectAllocationTracker {
   std::unique_ptr<SamplingHeapProfiler> sampling_heap_profiler_;
   std::vector<std::pair<v8::HeapProfiler::BuildEmbedderGraphCallback, void*>>
       build_embedder_graph_callbacks_;
+  std::pair<v8::HeapProfiler::GetDetachednessCallback, void*>
+      get_detachedness_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(HeapProfiler);
 };

test/unittests/heap/unified-heap-snapshot-unittest.cc

@@ -14,6 +14,7 @@
 #include "include/v8-profiler.h"
 #include "src/api/api-inl.h"
 #include "src/heap/cppgc-js/cpp-heap.h"
+#include "src/heap/cppgc/object-allocator.h"
 #include "src/objects/objects-inl.h"
 #include "src/profiler/heap-snapshot-generator-inl.h"
 #include "src/profiler/heap-snapshot-generator.h"
@@ -275,24 +276,52 @@ class GCedWithJSRef : public cppgc::GarbageCollected<GCedWithJSRef> {
     v8_object_.SetWrapperClassId(class_id);
   }
 
+  uint16_t WrapperClassId() const { return v8_object_.WrapperClassId(); }
+
+  TracedReference<v8::Object>& wrapper() { return v8_object_; }
+
  private:
   TracedReference<v8::Object> v8_object_;
 };
 constexpr const char GCedWithJSRef::kExpectedName[];
 
+class JsTestingScope {
+ public:
+  explicit JsTestingScope(v8::Isolate* isolate)
+      : isolate_(isolate),
+        handle_scope_(isolate),
+        context_(v8::Context::New(isolate)),
+        context_scope_(context_) {}
+
+  v8::Isolate* isolate() const { return isolate_; }
+  v8::Local<v8::Context> context() const { return context_; }
+
+ private:
+  v8::Isolate* isolate_;
+  v8::HandleScope handle_scope_;
+  v8::Local<v8::Context> context_;
+  v8::Context::Scope context_scope_;
+};
+
+cppgc::Persistent<GCedWithJSRef> SetupWrapperWrappablePair(
+    JsTestingScope& testing_scope, cppgc::AllocationHandle& allocation_handle,
+    const char* name) {
+  cppgc::Persistent<GCedWithJSRef> gc_w_js_ref =
+      cppgc::MakeGarbageCollected<GCedWithJSRef>(allocation_handle);
+  v8::Local<v8::Object> wrapper_object = WrapperHelper::CreateWrapper(
+      testing_scope.context(), gc_w_js_ref.Get(), name);
+  gc_w_js_ref->SetV8Object(testing_scope.isolate(), wrapper_object);
+  return std::move(gc_w_js_ref);
+}
+
 }  // namespace
 
 TEST_F(UnifiedHeapSnapshotTest, JSReferenceForcesVisibleObject) {
   // Test ensures that a C++->JS reference forces an object to be visible in the
   // snapshot.
-  cppgc::Persistent<GCedWithJSRef> gc_w_js_ref =
-      cppgc::MakeGarbageCollected<GCedWithJSRef>(allocation_handle());
-  v8::HandleScope scope(v8_isolate());
-  v8::Local<v8::Context> context = v8::Context::New(v8_isolate());
-  v8::Context::Scope context_scope(context);
-  v8::Local<v8::Object> api_object =
-      WrapperHelper::CreateWrapper(context, gc_w_js_ref.Get(), "LeafJSObject");
-  gc_w_js_ref->SetV8Object(v8_isolate(), api_object);
+  JsTestingScope testing_scope(v8_isolate());
+  cppgc::Persistent<GCedWithJSRef> gc_w_js_ref = SetupWrapperWrappablePair(
+      testing_scope, allocation_handle(), "LeafJSObject");
   const v8::HeapSnapshot* snapshot = TakeHeapSnapshot();
   EXPECT_TRUE(IsValidSnapshot(snapshot));
   EXPECT_TRUE(
@@ -308,22 +337,19 @@ TEST_F(UnifiedHeapSnapshotTest, MergedWrapperNode) {
   // Test ensures that the snapshot sets a wrapper node for C++->JS references
   // that have a class id set and that object nodes are merged into the C++
   // node, i.e., the directly reachable JS object is merged into the C++ object.
-  cppgc::Persistent<GCedWithJSRef> gc_w_js_ref =
-      cppgc::MakeGarbageCollected<GCedWithJSRef>(allocation_handle());
-  v8::HandleScope scope(v8_isolate());
-  v8::Local<v8::Context> context = v8::Context::New(v8_isolate());
-  v8::Context::Scope context_scope(context);
-  v8::Local<v8::Object> wrapper_object =
-      WrapperHelper::CreateWrapper(context, gc_w_js_ref.Get(), "MergedObject");
-  gc_w_js_ref->SetV8Object(v8_isolate(), wrapper_object);
+  JsTestingScope testing_scope(v8_isolate());
+  cppgc::Persistent<GCedWithJSRef> gc_w_js_ref = SetupWrapperWrappablePair(
+      testing_scope, allocation_handle(), "MergedObject");
   gc_w_js_ref->SetWrapperClassId(1);  // Any class id will do.
+  v8::Local<v8::Object> next_object = WrapperHelper::CreateWrapper(
+      testing_scope.context(), nullptr, "NextObject");
+  v8::Local<v8::Object> wrapper_object =
+      gc_w_js_ref->wrapper().Get(v8_isolate());
   // Chain another object to `wrapper_object`. Since `wrapper_object` should be
   // merged into `GCedWithJSRef`, the additional object must show up as direct
   // child from `GCedWithJSRef`.
-  v8::Local<v8::Object> next_object =
-      WrapperHelper::CreateWrapper(context, nullptr, "NextObject");
   wrapper_object
-      ->Set(context,
+      ->Set(testing_scope.context(),
             v8::String::NewFromUtf8(v8::Isolate::GetCurrent(), "link")
                 .ToLocalChecked(),
             next_object)
@@ -340,5 +366,126 @@ TEST_F(UnifiedHeapSnapshotTest, MergedWrapperNode) {
                             }));
 }
 
+namespace {
+
+constexpr uint16_t kClassIdForAttachedState = 0xAAAA;
+
+class DetachednessHandler {
+ public:
+  static size_t callback_count;
+
+  static v8::EmbedderGraph::Node::Detachedness GetDetachedness(
+      v8::Isolate* isolate, const v8::Local<v8::Value>& v8_value,
+      uint16_t class_id, void* data) {
+    callback_count++;
+    return class_id == kClassIdForAttachedState
+               ? v8::EmbedderGraph::Node::Detachedness::kAttached
+               : v8::EmbedderGraph::Node::Detachedness::kDetached;
+  }
+
+  static void Reset() { callback_count = 0; }
+};
+// static
+size_t DetachednessHandler::callback_count = 0;
+
+template <typename Callback>
+void ForEachEntryWithName(const v8::HeapSnapshot* snapshot, const char* needle,
+                          Callback callback) {
+  const HeapSnapshot* heap_snapshot =
+      reinterpret_cast<const HeapSnapshot*>(snapshot);
+  for (const HeapEntry& entry : heap_snapshot->entries()) {
+    if (strcmp(entry.name(), needle) == 0) {
+      callback(entry);
+    }
+  }
+}
+
+constexpr uint8_t kExpectedDetachedValueForUnknown =
+    static_cast<uint8_t>(v8::EmbedderGraph::Node::Detachedness::kUnknown);
+constexpr uint8_t kExpectedDetachedValueForAttached =
+    static_cast<uint8_t>(v8::EmbedderGraph::Node::Detachedness::kAttached);
+constexpr uint8_t kExpectedDetachedValueForDetached =
+    static_cast<uint8_t>(v8::EmbedderGraph::Node::Detachedness::kDetached);
+
+}  // namespace
+
+TEST_F(UnifiedHeapSnapshotTest, NoTriggerForClassIdZero) {
+  // Test ensures that objects with JS references that have no class id set do
+  // not have their detachedness state queried.
+  JsTestingScope testing_scope(v8_isolate());
+  cppgc::Persistent<GCedWithJSRef> gc_w_js_ref = SetupWrapperWrappablePair(
+      testing_scope, allocation_handle(), "MergedObject");
+  DetachednessHandler::Reset();
+  v8_isolate()->GetHeapProfiler()->SetGetDetachednessCallback(
+      DetachednessHandler::GetDetachedness, nullptr);
+  gc_w_js_ref->SetWrapperClassId(0);
+  EXPECT_EQ(0u, gc_w_js_ref->WrapperClassId());
+  const v8::HeapSnapshot* snapshot = TakeHeapSnapshot();
+  EXPECT_EQ(0u, DetachednessHandler::callback_count);
+  EXPECT_TRUE(IsValidSnapshot(snapshot));
+  EXPECT_TRUE(
+      ContainsRetainingPath(*snapshot,
+                            {
+                                kExpectedCppRootsName,             // NOLINT
+                                GetExpectedName<GCedWithJSRef>(),  // NOLINT
+                            }));
+  ForEachEntryWithName(
+      snapshot, GetExpectedName<GCedWithJSRef>(), [](const HeapEntry& entry) {
+        EXPECT_EQ(kExpectedDetachedValueForUnknown, entry.detachedness());
+      });
+}
+
+TEST_F(UnifiedHeapSnapshotTest, TriggerDetachednessCallbackSettingAttached) {
+  // Test ensures that objects with JS references that have a non-zero class id
+  // set do  have their detachedness state queried and set (attached version).
+  JsTestingScope testing_scope(v8_isolate());
+  cppgc::Persistent<GCedWithJSRef> gc_w_js_ref = SetupWrapperWrappablePair(
+      testing_scope, allocation_handle(), "MergedObject");
+  DetachednessHandler::Reset();
+  v8_isolate()->GetHeapProfiler()->SetGetDetachednessCallback(
+      DetachednessHandler::GetDetachedness, nullptr);
+  gc_w_js_ref->SetWrapperClassId(kClassIdForAttachedState);
+  EXPECT_NE(0u, gc_w_js_ref->WrapperClassId());
+  const v8::HeapSnapshot* snapshot = TakeHeapSnapshot();
+  EXPECT_EQ(1u, DetachednessHandler::callback_count);
+  EXPECT_TRUE(IsValidSnapshot(snapshot));
+  EXPECT_TRUE(
+      ContainsRetainingPath(*snapshot,
+                            {
+                                kExpectedCppRootsName,             // NOLINT
+                                GetExpectedName<GCedWithJSRef>(),  // NOLINT
+                            }));
+  ForEachEntryWithName(
+      snapshot, GetExpectedName<GCedWithJSRef>(), [](const HeapEntry& entry) {
+        EXPECT_EQ(kExpectedDetachedValueForAttached, entry.detachedness());
+      });
+}
+
+TEST_F(UnifiedHeapSnapshotTest, TriggerDetachednessCallbackSettingDetached) {
+  // Test ensures that objects with JS references that have a non-zero class id
+  // set do  have their detachedness state queried and set (detached version).
+  JsTestingScope testing_scope(v8_isolate());
+  cppgc::Persistent<GCedWithJSRef> gc_w_js_ref = SetupWrapperWrappablePair(
+      testing_scope, allocation_handle(), "MergedObject");
+  DetachednessHandler::Reset();
+  v8_isolate()->GetHeapProfiler()->SetGetDetachednessCallback(
+      DetachednessHandler::GetDetachedness, nullptr);
+  gc_w_js_ref->SetWrapperClassId(kClassIdForAttachedState - 1);
+  EXPECT_NE(0u, gc_w_js_ref->WrapperClassId());
+  const v8::HeapSnapshot* snapshot = TakeHeapSnapshot();
+  EXPECT_EQ(1u, DetachednessHandler::callback_count);
+  EXPECT_TRUE(IsValidSnapshot(snapshot));
+  EXPECT_TRUE(
+      ContainsRetainingPath(*snapshot,
+                            {
+                                kExpectedCppRootsName,             // NOLINT
+                                GetExpectedName<GCedWithJSRef>(),  // NOLINT
+                            }));
+  ForEachEntryWithName(
+      snapshot, GetExpectedName<GCedWithJSRef>(), [](const HeapEntry& entry) {
+        EXPECT_EQ(kExpectedDetachedValueForDetached, entry.detachedness());
+      });
+}
+
 }  // namespace internal
 }  // namespace v8