cppgc: Refactor prefinalizers

- Add a comment on the macro that registers a prefinalizer.
- Refactor the API to avoid exposing internal types needlessly.

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

BUILD.bazel

@@ -409,7 +409,6 @@ filegroup(
         "include/cppgc/internal/name-trait.h",
         "include/cppgc/internal/persistent-node.h",
         "include/cppgc/internal/pointer-policies.h",
-        "include/cppgc/internal/prefinalizer-handler.h",
         "include/cppgc/internal/write-barrier.h",
         "include/cppgc/liveness-broker.h",
         "include/cppgc/macros.h",

BUILD.gn

@@ -5529,7 +5529,6 @@ v8_header_set("cppgc_headers") {
     "include/cppgc/internal/name-trait.h",
     "include/cppgc/internal/persistent-node.h",
     "include/cppgc/internal/pointer-policies.h",
-    "include/cppgc/internal/prefinalizer-handler.h",
     "include/cppgc/internal/write-barrier.h",
     "include/cppgc/liveness-broker.h",
     "include/cppgc/macros.h",

include/cppgc/internal/prefinalizer-handler.h

@@ -1,30 +0,0 @@
-// Copyright 2020 the V8 project authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef INCLUDE_CPPGC_INTERNAL_PREFINALIZER_HANDLER_H_
-#define INCLUDE_CPPGC_INTERNAL_PREFINALIZER_HANDLER_H_
-
-#include "cppgc/heap.h"
-#include "cppgc/liveness-broker.h"
-
-namespace cppgc {
-namespace internal {
-
-class V8_EXPORT PreFinalizerRegistrationDispatcher final {
- public:
-  using PreFinalizerCallback = bool (*)(const LivenessBroker&, void*);
-  struct PreFinalizer {
-    void* object;
-    PreFinalizerCallback callback;
-
-    bool operator==(const PreFinalizer& other) const;
-  };
-
-  static void RegisterPrefinalizer(PreFinalizer pre_finalizer);
-};
-
-}  // namespace internal
-}  // namespace cppgc
-
-#endif  // INCLUDE_CPPGC_INTERNAL_PREFINALIZER_HANDLER_H_

include/cppgc/prefinalizer.h

@@ -6,23 +6,17 @@
 #define INCLUDE_CPPGC_PREFINALIZER_H_
 
 #include "cppgc/internal/compiler-specific.h"
-#include "cppgc/internal/prefinalizer-handler.h"
 #include "cppgc/liveness-broker.h"
 
 namespace cppgc {
 
 namespace internal {
 
-template <typename T>
-class PrefinalizerRegistration final {
+class V8_EXPORT PrefinalizerRegistration final {
  public:
-  explicit PrefinalizerRegistration(T* self) {
-    static_assert(sizeof(&T::InvokePreFinalizer) > 0,
-                  "USING_PRE_FINALIZER(T) must be defined.");
+  using Callback = bool (*)(const cppgc::LivenessBroker&, void*);
 
-    cppgc::internal::PreFinalizerRegistrationDispatcher::RegisterPrefinalizer(
-        {self, T::InvokePreFinalizer});
-  }
+  PrefinalizerRegistration(void*, Callback);
 
   void* operator new(size_t, void* location) = delete;
   void* operator new(size_t) = delete;
@@ -30,6 +24,35 @@ class PrefinalizerRegistration final {
 
 }  // namespace internal
 
+/**
+ * Macro must be used in the private section of `Class` and registers a
+ * prefinalization callback `void Class::PreFinalizer()`. The callback is
+ * invoked on garbage collection after the collector has found an object to be
+ * dead.
+ *
+ * Callback properties:
+ * - The callback is invoked before a possible destructor for the corresponding
+ *   object.
+ * - The callback may access the whole object graph, irrespective of whether
+ *   objects are considered dead or alive.
+ * - The callback is invoked on the same thread as the object was created on.
+ *
+ * Example:
+ * \code
+ * class WithPrefinalizer : public GarbageCollected<WithPrefinalizer> {
+ *   CPPGC_USING_PRE_FINALIZER(WithPrefinalizer, Dispose);
+ *
+ *  public:
+ *   void Trace(Visitor*) const {}
+ *   void Dispose() { prefinalizer_called = true; }
+ *   ~WithPrefinalizer() {
+ *     // prefinalizer_called == true
+ *   }
+ *  private:
+ *   bool prefinalizer_called = false;
+ * };
+ * \endcode
+ */
 #define CPPGC_USING_PRE_FINALIZER(Class, PreFinalizer)                         \
  public:                                                                       \
   static bool InvokePreFinalizer(const cppgc::LivenessBroker& liveness_broker, \
@@ -43,8 +66,8 @@ class PrefinalizerRegistration final {
   }                                                                            \
                                                                                \
  private:                                                                      \
-  CPPGC_NO_UNIQUE_ADDRESS cppgc::internal::PrefinalizerRegistration<Class>     \
-      prefinalizer_dummy_{this};                                               \
+  CPPGC_NO_UNIQUE_ADDRESS cppgc::internal::PrefinalizerRegistration            \
+      prefinalizer_dummy_{this, Class::InvokePreFinalizer};                    \
   static_assert(true, "Force semicolon.")
 
 }  // namespace cppgc

src/heap/cppgc/prefinalizer-handler.cc

@@ -16,16 +16,14 @@
 namespace cppgc {
 namespace internal {
 
-// static
-void PreFinalizerRegistrationDispatcher::RegisterPrefinalizer(
-    PreFinalizer pre_finalizer) {
-  auto* page = BasePage::FromPayload(pre_finalizer.object);
+PrefinalizerRegistration::PrefinalizerRegistration(void* object,
+                                                   Callback callback) {
+  auto* page = BasePage::FromPayload(object);
   DCHECK(!page->space().is_compactable());
-  page->heap().prefinalizer_handler()->RegisterPrefinalizer(pre_finalizer);
+  page->heap().prefinalizer_handler()->RegisterPrefinalizer({object, callback});
 }
 
-bool PreFinalizerRegistrationDispatcher::PreFinalizer::operator==(
-    const PreFinalizer& other) const {
+bool PreFinalizer::operator==(const PreFinalizer& other) const {
   return (object == other.object) && (callback == other.callback);
 }
 
@@ -35,7 +33,7 @@ PreFinalizerHandler::PreFinalizerHandler(HeapBase& heap)
 #ifdef DEBUG
       ,
       creation_thread_id_(v8::base::OS::GetCurrentThreadId())
-#endif
+#endif  // DEBUG
 {
 }
 

src/heap/cppgc/prefinalizer-handler.h

@@ -15,11 +15,17 @@ namespace internal {
 
 class HeapBase;
 
+struct PreFinalizer final {
+  using Callback = PrefinalizerRegistration::Callback;
+
+  void* object;
+  Callback callback;
+
+  bool operator==(const PreFinalizer& other) const;
+};
+
 class PreFinalizerHandler final {
  public:
-  using PreFinalizer =
-      cppgc::internal::PreFinalizerRegistrationDispatcher::PreFinalizer;
-
   explicit PreFinalizerHandler(HeapBase& heap);
 
   void RegisterPrefinalizer(PreFinalizer pre_finalizer);