From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Michael Lippautz <mlippautz@chromium.org>
Date: Fri, 3 Sep 2021 14:17:59 +0200
Subject: cppgc-js: Support eager traced value in ephemeron pairs

Before this patch, both key and value of an ephemeron pair was always
considered to be GarbageCollected objects.

This patch adjusts the snapshotting mechanism to accomodate that
values may not be GarbageCollected objects and must thus be eagerly
traced for visibility and edge creation.

In practice this only shows up in Blink when associating an existing
wrappable with a wrapper in a non-main world, e.g., through an
extension. In this case, DOMWrapperMap keeps the wrapper value through
a TracedReference in the ephemeron map with the existing wrappable as
key. The semantics are intended to be general ephemeron semantics,
i.e., value needs to be kept alive when the key is alive.  This is
visualized in DevTools as the main wrapper/wrappable pair (which is
merged into a single node for the snapshot) retaining the non-main
world wrapper.

Bug: chromium:1245894
Change-Id: Ibfa6722f20c76f94c310f9a040f0d3d4b9083bbb
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3140601
Reviewed-by: Omer Katz <omerkatz@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#76658}

diff --git a/src/heap/cppgc-js/cpp-snapshot.cc b/src/heap/cppgc-js/cpp-snapshot.cc
index dc55753ff625a135b6e494344ee49105eb59121e..9b20b5c0a7831ea026819f90ea80c10eb2324282 100644
--- a/src/heap/cppgc-js/cpp-snapshot.cc
+++ b/src/heap/cppgc-js/cpp-snapshot.cc
@@ -264,6 +264,10 @@ class State final : public StateBase {
     ephemeron_edges_.insert(&value);
   }
 
+  void AddEagerEphemeronEdge(const void* value, cppgc::TraceCallback callback) {
+    eager_ephemeron_edges_.insert({value, callback});
+  }
+
   template <typename Callback>
   void ForAllEphemeronEdges(Callback callback) {
     for (const HeapObjectHeader* value : ephemeron_edges_) {
@@ -271,10 +275,20 @@ class State final : public StateBase {
     }
   }
 
+  template <typename Callback>
+  void ForAllEagerEphemeronEdges(Callback callback) {
+    for (const auto& pair : eager_ephemeron_edges_) {
+      callback(pair.first, pair.second);
+    }
+  }
+
  private:
   bool is_weak_container_ = false;
   // Values that are held alive through ephemerons by this particular key.
   std::unordered_set<const HeapObjectHeader*> ephemeron_edges_;
+  // Values that are eagerly traced and held alive through ephemerons by this
+  // particular key.
+  std::unordered_map<const void*, cppgc::TraceCallback> eager_ephemeron_edges_;
 };
 
 // Root states are similar to regular states with the difference that they are
@@ -404,6 +418,9 @@ class CppGraphBuilderImpl final {
   void VisitForVisibility(State& parent, const TracedReferenceBase&);
   void VisitEphemeronForVisibility(const HeapObjectHeader& key,
                                    const HeapObjectHeader& value);
+  void VisitEphemeronWithNonGarbageCollectedValueForVisibility(
+      const HeapObjectHeader& key, const void* value,
+      cppgc::TraceDescriptor value_desc);
   void VisitWeakContainerForVisibility(const HeapObjectHeader&);
   void VisitRootForGraphBuilding(RootState&, const HeapObjectHeader&,
                                  const cppgc::SourceLocation&);
@@ -421,7 +438,7 @@ class CppGraphBuilderImpl final {
   }
 
   void AddEdge(State& parent, const HeapObjectHeader& header,
-               const std::string& edge_name = {}) {
+               const std::string& edge_name) {
     DCHECK(parent.IsVisibleNotDependent());
     auto& current = states_.GetExistingState(header);
     if (!current.IsVisibleNotDependent()) return;
@@ -443,7 +460,8 @@ class CppGraphBuilderImpl final {
     }
   }
 
-  void AddEdge(State& parent, const TracedReferenceBase& ref) {
+  void AddEdge(State& parent, const TracedReferenceBase& ref,
+               const std::string& edge_name) {
     DCHECK(parent.IsVisibleNotDependent());
     v8::Local<v8::Value> v8_value = ref.Get(cpp_heap_.isolate());
     if (!v8_value.IsEmpty()) {
@@ -451,12 +469,19 @@ class CppGraphBuilderImpl final {
         parent.set_node(AddNode(*parent.header()));
       }
       auto* v8_node = graph_.V8Node(v8_value);
-      graph_.AddEdge(parent.get_node(), v8_node);
+      if (!edge_name.empty()) {
+        graph_.AddEdge(parent.get_node(), v8_node,
+                       parent.get_node()->InternalizeEdgeName(edge_name));
+      } else {
+        graph_.AddEdge(parent.get_node(), v8_node);
+      }
 
       // References that have a class id set may have their internal fields
       // pointing back to the object. Set up a wrapper node for the graph so
       // that the snapshot generator  can merge the nodes appropriately.
-      if (!ref.WrapperClassId()) return;
+      // Even with a set class id, do not set up a wrapper node when the edge
+      // has a specific name.
+      if (!ref.WrapperClassId() || !edge_name.empty()) return;
 
       void* back_reference_object = ExtractEmbedderDataBackref(
           reinterpret_cast<v8::internal::Isolate*>(cpp_heap_.isolate()),
@@ -598,8 +623,18 @@ class WeakVisitor : public JSVisitor {
   void VisitEphemeron(const void* key, const void* value,
                       cppgc::TraceDescriptor value_desc) final {
     // For ephemerons, the key retains the value.
+    // Key always must be a GarbageCollected object.
+    auto& key_header = HeapObjectHeader::FromObject(key);
+    if (!value_desc.base_object_payload) {
+      // Value does not represent an actual GarbageCollected object but rather
+      // should be traced eagerly.
+      graph_builder_.VisitEphemeronWithNonGarbageCollectedValueForVisibility(
+          key_header, value, value_desc);
+      return;
+    }
+    // Regular path where both key and value are GarbageCollected objects.
     graph_builder_.VisitEphemeronForVisibility(
-        HeapObjectHeader::FromObject(key), HeapObjectHeader::FromObject(value));
+        key_header, HeapObjectHeader::FromObject(value));
   }
 
  protected:
@@ -645,7 +680,7 @@ class GraphBuildingVisitor final : public JSVisitor {
   void Visit(const void*, cppgc::TraceDescriptor desc) final {
     graph_builder_.AddEdge(
         parent_scope_.ParentAsRegularState(),
-        HeapObjectHeader::FromObject(desc.base_object_payload));
+        HeapObjectHeader::FromObject(desc.base_object_payload), edge_name_);
   }
   void VisitWeakContainer(const void* object,
                           cppgc::TraceDescriptor strong_desc,
@@ -655,7 +690,8 @@ class GraphBuildingVisitor final : public JSVisitor {
     // container itself.
     graph_builder_.AddEdge(
         parent_scope_.ParentAsRegularState(),
-        HeapObjectHeader::FromObject(strong_desc.base_object_payload));
+        HeapObjectHeader::FromObject(strong_desc.base_object_payload),
+        edge_name_);
   }
   void VisitRoot(const void*, cppgc::TraceDescriptor desc,
                  const cppgc::SourceLocation& loc) final {
@@ -667,12 +703,18 @@ class GraphBuildingVisitor final : public JSVisitor {
                      const void*, const cppgc::SourceLocation&) final {}
   // JS handling.
   void Visit(const TracedReferenceBase& ref) final {
-    graph_builder_.AddEdge(parent_scope_.ParentAsRegularState(), ref);
+    graph_builder_.AddEdge(parent_scope_.ParentAsRegularState(), ref,
+                           edge_name_);
+  }
+
+  void set_edge_name(std::string edge_name) {
+    edge_name_ = std::move(edge_name);
   }
 
  private:
   CppGraphBuilderImpl& graph_builder_;
   const ParentScope& parent_scope_;
+  std::string edge_name_;
 };
 
 // Base class for transforming recursion into iteration. Items are processed
@@ -765,6 +807,19 @@ void CppGraphBuilderImpl::VisitForVisibility(State* parent,
   }
 }
 
+void CppGraphBuilderImpl::
+    VisitEphemeronWithNonGarbageCollectedValueForVisibility(
+        const HeapObjectHeader& key, const void* value,
+        cppgc::TraceDescriptor value_desc) {
+  auto& key_state = states_.GetOrCreateState(key);
+  // Eagerly trace the value here, effectively marking key as visible and
+  // queuing processing for all reachable values.
+  ParentScope parent_scope(key_state);
+  VisiblityVisitor visitor(*this, parent_scope);
+  value_desc.callback(&visitor, value);
+  key_state.AddEagerEphemeronEdge(value, value_desc.callback);
+}
+
 void CppGraphBuilderImpl::VisitEphemeronForVisibility(
     const HeapObjectHeader& key, const HeapObjectHeader& value) {
   auto& key_state = states_.GetOrCreateState(key);
@@ -820,6 +875,12 @@ void CppGraphBuilderImpl::Run() {
     state.ForAllEphemeronEdges([this, &state](const HeapObjectHeader& value) {
       AddEdge(state, value, "part of key -> value pair in ephemeron table");
     });
+    object_visitor.set_edge_name(
+        "part of key -> value pair in ephemeron table");
+    state.ForAllEagerEphemeronEdges(
+        [&object_visitor](const void* value, cppgc::TraceCallback callback) {
+          callback(&object_visitor, value);
+        });
   });
   // Add roots.
   {
