cppgc: Handle ephemerons with Mixin keys.

This was causing DevTools to crush whenever I took a heap snapshot.

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

include/cppgc/visitor.h

@@ -164,16 +164,21 @@ class V8_EXPORT Visitor {
    * \param key WeakMember reference weakly retaining a key object.
    * \param value Member reference weakly retaining a value object.
    */
-  template <typename K, typename V>
-  void TraceEphemeron(const WeakMember<K>& key, const V* value) {
-    const K* k = key.GetRawAtomic();
+  template <typename KeyType, typename ValueType>
+  void TraceEphemeron(const WeakMember<KeyType>& key, const ValueType* value) {
+    const KeyType* k = key.GetRawAtomic();
     if (!k) return;
-    TraceDescriptor value_desc = TraceTrait<V>::GetTraceDescriptor(value);
+    TraceDescriptor value_desc =
+        TraceTrait<ValueType>::GetTraceDescriptor(value);
+    // KeyType might be a GarbageCollectedMixin.
+    const void* key_base_object_payload =
+        TraceTrait<KeyType>::GetTraceDescriptor(k).base_object_payload;
+    CPPGC_DCHECK(key_base_object_payload);
     // `value` must always be non-null. `value_desc.base_object_payload` may be
     // null in the case that value is not a garbage-collected object but only
     // traceable.
     CPPGC_DCHECK(value);
-    VisitEphemeron(key, value, value_desc);
+    VisitEphemeron(key_base_object_payload, value, value_desc);
   }
 
   /**

test/unittests/heap/cppgc/ephemeron-pair-unittest.cc

@@ -5,6 +5,7 @@
 #include "include/cppgc/ephemeron-pair.h"
 
 #include "include/cppgc/allocation.h"
+#include "include/cppgc/garbage-collected.h"
 #include "include/cppgc/persistent.h"
 #include "src/heap/cppgc/heap-object-header.h"
 #include "src/heap/cppgc/marking-visitor.h"
@@ -21,7 +22,7 @@ class GCed : public GarbageCollected<GCed> {
   void Trace(cppgc::Visitor*) const {}
 };
 
-class EphemeronHolder : public GarbageCollected<GCed> {
+class EphemeronHolder : public GarbageCollected<EphemeronHolder> {
  public:
   EphemeronHolder(GCed* key, GCed* value) : ephemeron_pair_(key, value) {}
   void Trace(cppgc::Visitor* visitor) const { visitor->Trace(ephemeron_pair_); }
@@ -168,5 +169,62 @@ TEST_F(EphemeronPairGCTest, EphemeronPairValueIsCleared) {
   EXPECT_EQ(nullptr, holder->ephemeron_pair().value.Get());
 }
 
+namespace {
+
+class Mixin : public GarbageCollectedMixin {
+ public:
+  void Trace(Visitor* v) const override {}
+};
+
+class OtherMixin : public GarbageCollectedMixin {
+ public:
+  void Trace(Visitor* v) const override {}
+};
+
+class GCedWithMixin : public GarbageCollected<GCedWithMixin>,
+                      public OtherMixin,
+                      public Mixin {
+ public:
+  void Trace(Visitor* v) const override {
+    OtherMixin::Trace(v);
+    Mixin::Trace(v);
+  }
+};
+
+class EphemeronHolderWithMixins
+    : public GarbageCollected<EphemeronHolderWithMixins> {
+ public:
+  EphemeronHolderWithMixins(Mixin* key, Mixin* value)
+      : ephemeron_pair_(key, value) {}
+  void Trace(cppgc::Visitor* visitor) const { visitor->Trace(ephemeron_pair_); }
+
+  const EphemeronPair<Mixin, Mixin>& ephemeron_pair() const {
+    return ephemeron_pair_;
+  }
+
+ private:
+  EphemeronPair<Mixin, Mixin> ephemeron_pair_;
+};
+
+}  // namespace
+
+TEST_F(EphemeronPairTest, EphemeronPairWithMixinKey) {
+  GCedWithMixin* key =
+      MakeGarbageCollected<GCedWithMixin>(GetAllocationHandle());
+  GCedWithMixin* value =
+      MakeGarbageCollected<GCedWithMixin>(GetAllocationHandle());
+  Persistent<EphemeronHolderWithMixins> holder =
+      MakeGarbageCollected<EphemeronHolderWithMixins>(GetAllocationHandle(),
+                                                      key, value);
+  EXPECT_NE(static_cast<void*>(key), holder->ephemeron_pair().key.Get());
+  EXPECT_NE(static_cast<void*>(value), holder->ephemeron_pair().value.Get());
+  InitializeMarker(*Heap::From(GetHeap()), GetPlatformHandle().get());
+  FinishSteps();
+  EXPECT_FALSE(HeapObjectHeader::FromPayload(value).IsMarked());
+  EXPECT_TRUE(HeapObjectHeader::FromPayload(key).TryMarkAtomic());
+  FinishMarking();
+  EXPECT_TRUE(HeapObjectHeader::FromPayload(value).IsMarked());
+}
+
 }  // namespace internal
 }  // namespace cppgc