cppgc: Provide Visitor::Trace for WeakMember

Provides the infrastructure to register weak callbacks for
WeakMember<T> through visitor. The WeakCallbackInfo broker is used to
query objects for liveness. In a future CL the same broker object is
passed to custom weak callbacks.

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

include/cppgc/visitor.h

@@ -6,6 +6,8 @@
 #define INCLUDE_CPPGC_VISITOR_H_
 
 #include "include/cppgc/garbage-collected.h"
+#include "include/cppgc/internal/pointer-policies.h"
+#include "include/cppgc/liveness-broker.h"
 #include "include/cppgc/member.h"
 #include "include/cppgc/trace-trait.h"
 
@@ -14,6 +16,14 @@ namespace internal {
 class VisitorBase;
 }  // namespace internal
 
+class Visitor;
+
+using WeakCallback = void (*)(const LivenessBroker&, const void*);
+
+/**
+ * Visitor passed to trace methods. All managed pointers must have called the
+ * visitor's trace method on them.
+ */
 class Visitor {
  public:
   template <typename T>
@@ -24,10 +34,43 @@ class Visitor {
     Trace(value);
   }
 
+  template <typename T>
+  void Trace(const WeakMember<T>& weak_member) {
+    static_assert(sizeof(T), "T must be fully defined");
+    static_assert(internal::IsGarbageCollectedType<T>::value,
+                  "T must be GarabgeCollected or GarbageCollectedMixin type");
+
+    const T* value = weak_member.GetRawAtomic();
+
+    // Bailout assumes that WeakMember emits write barrier.
+    if (!value) {
+      return;
+    }
+
+    // TODO(chromium:1056170): DCHECK (or similar) for deleted values as they
+    // should come in at a different path.
+    VisitWeak(value, TraceTrait<T>::GetTraceDescriptor(value),
+              &HandleWeakMember<T>, &weak_member);
+  }
+
  protected:
-  virtual void Visit(const void*, TraceDescriptor) {}
+  virtual void Visit(const void* self, TraceDescriptor) {}
+  virtual void VisitWeak(const void* self, TraceDescriptor, WeakCallback,
+                         const void* weak_member) {}
 
  private:
+  template <typename T>
+  static void HandleWeakMember(const LivenessBroker& info, const void* object) {
+    const WeakMember<T>* weak_member =
+        reinterpret_cast<const WeakMember<T>*>(object);
+    if (!info.IsHeapObjectAlive(*weak_member)) {
+      // Object is passed down through the marker as const. Alternatives are
+      // - non-const Trace method;
+      // - mutable pointer in MemberBase;
+      const_cast<WeakMember<T>*>(weak_member)->Clear();
+    }
+  }
+
   Visitor() = default;
 
   template <typename T>

src/heap/cppgc/visitor.h

@@ -10,7 +10,8 @@
 namespace cppgc {
 namespace internal {
 
-// Base visitor that is allowed to create a public cppgc::Visitor object.
+// Base visitor that is allowed to create a public cppgc::Visitor object and
+// use its internals.
 class VisitorBase : public cppgc::Visitor {
  public:
   VisitorBase() = default;

test/unittests/heap/cppgc/visitor_unittest.cc

@@ -3,10 +3,12 @@
 // found in the LICENSE file.
 
 #include "src/heap/cppgc/visitor.h"
+
 #include "include/cppgc/allocation.h"
 #include "include/cppgc/garbage-collected.h"
 #include "include/cppgc/trace-trait.h"
 #include "src/base/macros.h"
+#include "src/heap/cppgc/heap.h"
 #include "test/unittests/heap/cppgc/tests.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -60,6 +62,14 @@ class DispatchingVisitor final : public VisitorBase {
     desc.callback(this, desc.base_object_payload);
   }
 
+  void VisitWeak(const void* t, TraceDescriptor desc, WeakCallback callback,
+                 const void* weak_member) final {
+    EXPECT_EQ(object_, t);
+    EXPECT_EQ(payload_, desc.base_object_payload);
+    LivenessBroker broker = LivenessBrokerFactory::Create();
+    callback(broker, weak_member);
+  }
+
  private:
   const void* object_;
   const void* payload_;
@@ -133,5 +143,25 @@ TEST_F(VisitorTest, DispatchTraceGCedMixin) {
   EXPECT_EQ(1u, GCed::trace_callcount);
 }
 
+TEST_F(VisitorTest, DispatchTraceWeakGCed) {
+  WeakMember<GCed> ref = MakeGarbageCollected<GCed>(GetHeap());
+  DispatchingVisitor visitor(ref, ref);
+  visitor.Trace(ref);
+  // No marking, so reference should be cleared.
+  EXPECT_EQ(nullptr, ref.Get());
+}
+
+TEST_F(VisitorTest, DispatchTraceWeakGCedMixin) {
+  auto* gced_mixin_app = MakeGarbageCollected<GCedMixinApplication>(GetHeap());
+  auto* gced_mixin = static_cast<GCedMixin*>(gced_mixin_app);
+  // Ensure that we indeed test dispatching an inner object.
+  EXPECT_NE(static_cast<void*>(gced_mixin_app), static_cast<void*>(gced_mixin));
+  WeakMember<GCedMixin> ref = gced_mixin;
+  DispatchingVisitor visitor(gced_mixin, gced_mixin_app);
+  visitor.Trace(ref);
+  // No marking, so reference should be cleared.
+  EXPECT_EQ(nullptr, ref.Get());
+}
+
 }  // namespace internal
 }  // namespace cppgc