[api] Remove TracedGlobal<>

Remove deprecated TracedGlobal<>, greatly simplifying handling of
traced references in general.

Also saves a word per v8::TracedReference as there's no need to keep a
possible callback around.

Bug: v8:12603
Change-Id: Ice35d7906775b912d02e97a27a722b3e1cec28d9
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3532251Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#79589}

include/v8-embedder-heap.h

@@ -34,29 +34,22 @@ class V8_EXPORT EmbedderRootsHandler {
   virtual ~EmbedderRootsHandler() = default;
 
   /**
-   * Returns true if the TracedGlobal handle should be considered as root for
-   * the currently running non-tracing garbage collection and false otherwise.
-   * The default implementation will keep all TracedGlobal references as roots.
+   * Returns true if the |TracedReference| handle should be considered as root
+   * for the currently running non-tracing garbage collection and false
+   * otherwise. The default implementation will keep all |TracedReference|
+   * references as roots.
    *
    * If this returns false, then V8 may decide that the object referred to by
-   * such a handle is reclaimed. In that case:
-   * - No action is required if handles are used with destructors, i.e., by just
-   *   using |TracedGlobal|.
-   * - When run without destructors, i.e., by using |TracedReference|, V8 calls
-   *  |ResetRoot|.
+   * such a handle is reclaimed. In that case, V8 calls |ResetRoot()| for the
+   * |TracedReference|.
    *
-   * Note that the |handle| is different from the handle that the embedder holds
+   * Note that the `handle` is different from the handle that the embedder holds
    * for retaining the object. The embedder may use |WrapperClassId()| to
    * distinguish cases where it wants handles to be treated as roots from not
    * being treated as roots.
    */
   virtual bool IsRoot(const v8::TracedReference<v8::Value>& handle) = 0;
 
-  V8_DEPRECATED("See v8::TracedGlobal class comment.")
-  virtual bool IsRoot(const v8::TracedGlobal<v8::Value>& handle) {
-    return true;
-  }
-
   /**
    * Used in combination with |IsRoot|. Called by V8 when an
    * object that is backed by a handle is reclaimed by a non-tracing garbage
@@ -87,13 +80,11 @@ class V8_EXPORT EmbedderHeapTracer {
   };
 
   /**
-   * Interface for iterating through TracedGlobal handles.
+   * Interface for iterating through |TracedReference| handles.
    */
   class V8_EXPORT TracedGlobalHandleVisitor {
    public:
     virtual ~TracedGlobalHandleVisitor() = default;
-    V8_DEPRECATED("See v8::TracedGlobal class comment.")
-    virtual void VisitTracedGlobalHandle(const TracedGlobal<Value>& handle) {}
     virtual void VisitTracedReference(const TracedReference<Value>& handle) {}
   };
 
@@ -118,8 +109,8 @@ class V8_EXPORT EmbedderHeapTracer {
   virtual ~EmbedderHeapTracer() = default;
 
   /**
-   * Iterates all TracedGlobal handles created for the v8::Isolate the tracer is
-   * attached to.
+   * Iterates all |TracedReference| handles created for the |v8::Isolate| the
+   * tracer is attached to.
    */
   void IterateTracedGlobalHandles(TracedGlobalHandleVisitor* visitor);
 
@@ -194,8 +185,6 @@ class V8_EXPORT EmbedderHeapTracer {
    */
   virtual bool IsRootForNonTracingGC(
       const v8::TracedReference<v8::Value>& handle);
-  V8_DEPRECATED("See v8::TracedGlobal class comment.")
-  virtual bool IsRootForNonTracingGC(const v8::TracedGlobal<v8::Value>& handle);
 
   /**
    * See documentation on EmbedderRootsHandler.

include/v8-local-handle.h

@@ -46,8 +46,6 @@ class String;
 template <class F>
 class Traced;
 template <class F>
-class TracedGlobal;
-template <class F>
 class TracedReference;
 class TracedReferenceBase;
 class Utils;
@@ -312,8 +310,6 @@ class Local {
   template <class F>
   friend class Traced;
   template <class F>
-  friend class TracedGlobal;
-  template <class F>
   friend class BasicTracedReference;
   template <class F>
   friend class TracedReference;

include/v8-object.h

@@ -493,7 +493,7 @@ class V8_EXPORT Object : public Value {
     return object.val_->GetAlignedPointerFromInternalField(index);
   }
 
-  /** Same as above, but works for TracedGlobal. */
+  /** Same as above, but works for TracedReference. */
   V8_INLINE static void* GetAlignedPointerFromInternalField(
       const BasicTracedReference<Object>& object, int index) {
     return object->GetAlignedPointerFromInternalField(index);

src/api/api.cc

@@ -807,17 +807,16 @@ void ResourceConstraints::ConfigureDefaults(uint64_t physical_memory,
 
 namespace internal {
 
-i::Address* GlobalizeTracedReference(
-    i::Isolate* isolate, i::Address* obj, internal::Address* slot,
-    GlobalHandleDestructionMode destruction_mode,
-    GlobalHandleStoreMode store_mode) {
+i::Address* GlobalizeTracedReference(i::Isolate* isolate, i::Address* obj,
+                                     internal::Address* slot,
+                                     GlobalHandleStoreMode store_mode) {
   LOG_API(isolate, TracedGlobal, New);
 #ifdef DEBUG
   Utils::ApiCheck((slot != nullptr), "v8::GlobalizeTracedReference",
                   "the address slot must be not null");
 #endif
-  i::Handle<i::Object> result = isolate->global_handles()->CreateTraced(
-      *obj, slot, destruction_mode, store_mode);
+  i::Handle<i::Object> result =
+      isolate->global_handles()->CreateTraced(*obj, slot, store_mode);
 #ifdef VERIFY_HEAP
   if (i::FLAG_verify_heap) {
     i::Object(*obj).ObjectVerify(isolate);
@@ -826,24 +825,17 @@ i::Address* GlobalizeTracedReference(
   return result.location();
 }
 
-void MoveTracedGlobalReference(internal::Address** from,
-                               internal::Address** to) {
-  GlobalHandles::MoveTracedGlobal(from, to);
+void MoveTracedReference(internal::Address** from, internal::Address** to) {
+  GlobalHandles::MoveTracedReference(from, to);
 }
 
-void CopyTracedGlobalReference(const internal::Address* const* from,
-                               internal::Address** to) {
-  GlobalHandles::CopyTracedGlobal(from, to);
+void CopyTracedReference(const internal::Address* const* from,
+                         internal::Address** to) {
+  GlobalHandles::CopyTracedReference(from, to);
 }
 
-void DisposeTracedGlobal(internal::Address* location) {
-  GlobalHandles::DestroyTraced(location);
-}
-
-void SetFinalizationCallbackTraced(internal::Address* location, void* parameter,
-                                   WeakCallbackInfo<void>::Callback callback) {
-  GlobalHandles::SetFinalizationCallbackForTraced(location, parameter,
-                                                  callback);
+void DisposeTracedReference(internal::Address* location) {
+  GlobalHandles::DestroyTracedReference(location);
 }
 
 }  // namespace internal
@@ -10277,11 +10269,6 @@ bool EmbedderHeapTracer::IsRootForNonTracingGC(
   return true;
 }
 
-bool EmbedderHeapTracer::IsRootForNonTracingGC(
-    const v8::TracedGlobal<v8::Value>& handle) {
-  return true;
-}
-
 void EmbedderHeapTracer::ResetHandleInNonTracingGC(
     const v8::TracedReference<v8::Value>& handle) {
   UNREACHABLE();

src/handles/global-handles.h

@@ -90,12 +90,9 @@ class V8_EXPORT_PRIVATE GlobalHandles final {
   // API for traced handles.
   //
 
-  static void MoveTracedGlobal(Address** from, Address** to);
-  static void CopyTracedGlobal(const Address* const* from, Address** to);
-  static void DestroyTraced(Address* location);
-  static void SetFinalizationCallbackForTraced(
-      Address* location, void* parameter,
-      WeakCallbackInfo<void>::Callback callback);
+  static void MoveTracedReference(Address** from, Address** to);
+  static void CopyTracedReference(const Address* const* from, Address** to);
+  static void DestroyTracedReference(Address* location);
   static void MarkTraced(Address* location);
 
   explicit GlobalHandles(Isolate* isolate);
@@ -109,14 +106,11 @@ class V8_EXPORT_PRIVATE GlobalHandles final {
   inline Handle<T> Create(T value);
 
   Handle<Object> CreateTraced(Object value, Address* slot,
-                              GlobalHandleDestructionMode destruction_mode,
                               GlobalHandleStoreMode store_mode,
                               bool is_on_stack);
   Handle<Object> CreateTraced(Object value, Address* slot,
-                              GlobalHandleDestructionMode destruction_mode,
                               GlobalHandleStoreMode store_mode);
   Handle<Object> CreateTraced(Address value, Address* slot,
-                              GlobalHandleDestructionMode destruction_mode,
                               GlobalHandleStoreMode store_mode);
 
   void RecordStats(HeapStats* stats);

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

@@ -64,10 +64,6 @@ class V8ToCppGCReferencesVisitor final
         isolate_(isolate),
         wrapper_descriptor_(wrapper_descriptor) {}
 
-  void VisitTracedGlobalHandle(const v8::TracedGlobal<v8::Value>&) final {
-    UNREACHABLE();
-  }
-
   void VisitTracedReference(const v8::TracedReference<v8::Value>& value) final {
     VisitHandle(value, value.WrapperClassId());
   }

src/heap/embedder-tracing.cc

@@ -6,7 +6,6 @@
 
 #include "include/v8-cppgc.h"
 #include "src/base/logging.h"
-#include "src/common/allow-deprecated.h"
 #include "src/handles/global-handles.h"
 #include "src/heap/embedder-tracing-inl.h"
 #include "src/heap/gc-tracer.h"
@@ -211,13 +210,6 @@ bool DefaultEmbedderRootsHandler::IsRoot(
   return !tracer_ || tracer_->IsRootForNonTracingGC(handle);
 }
 
-START_ALLOW_USE_DEPRECATED()
-bool DefaultEmbedderRootsHandler::IsRoot(
-    const v8::TracedGlobal<v8::Value>& handle) {
-  return !tracer_ || tracer_->IsRootForNonTracingGC(handle);
-}
-END_ALLOW_USE_DEPRECATED()
-
 void DefaultEmbedderRootsHandler::ResetRoot(
     const v8::TracedReference<v8::Value>& handle) {
   // Resetting is only called when IsRoot() returns false which

src/heap/embedder-tracing.h

@@ -27,10 +27,6 @@ class V8_EXPORT_PRIVATE DefaultEmbedderRootsHandler final
  public:
   bool IsRoot(const v8::TracedReference<v8::Value>& handle) final;
 
-  START_ALLOW_USE_DEPRECATED()
-  bool IsRoot(const v8::TracedGlobal<v8::Value>& handle) final;
-  END_ALLOW_USE_DEPRECATED()
-
   void ResetRoot(const v8::TracedReference<v8::Value>& handle) final;
 
   void SetTracer(EmbedderHeapTracer* tracer) { tracer_ = tracer; }

test/cctest/cctest.status

@@ -1032,22 +1032,15 @@
   'test-debug/TerminateOnResumeFromOtherThread': [SKIP],
   'test-debug/TerminateOnResumeRunJavaScriptAtBreakpoint': [SKIP],
   'test-debug/TerminateOnResumeRunMicrotaskAtBreakpoint': [SKIP],
+  'test-embedder-tracing/BasicTracedReference': [SKIP],
   'test-embedder-tracing/GarbageCollectionForTesting': [SKIP],
   'test-embedder-tracing/NotifyEmptyStack': [SKIP],
-  'test-embedder-tracing/TracedGlobalCopy': [SKIP],
-  'test-embedder-tracing/TracedGlobalCopyNoDestructor': [SKIP],
-  'test-embedder-tracing/TracedGlobalCopyWithDestructor': [SKIP],
-  'test-embedder-tracing/TracedGlobalDestructor': [SKIP],
-  'test-embedder-tracing/TracedGlobalInStdUnorderedMap': [SKIP],
-  'test-embedder-tracing/TracedGlobalInStdVector': [SKIP],
-  'test-embedder-tracing/TracedGlobalMove': [SKIP],
-  'test-embedder-tracing/TracedGlobalNoDestructor': [SKIP],
-  'test-embedder-tracing/TracedGlobalSetFinalizationCallbackMarkSweep': [SKIP],
-  'test-embedder-tracing/TracedGlobalToUnmodifiedJSObjectDiesOnMarkSweep': [SKIP],
+  'test-embedder-tracing/TracedReferenceCopyReferences': [SKIP],
   'test-embedder-tracing/TracedReferenceCopy': [SKIP],
   'test-embedder-tracing/TracedReferenceHandlesDoNotLeak': [SKIP],
   'test-embedder-tracing/TracedReferenceHandlesMarking': [SKIP],
   'test-embedder-tracing/TracedReferenceMove': [SKIP],
+  'test-embedder-tracing/TracedReferenceToUnmodifiedJSObjectDiesOnMarkSweep': [SKIP],
   'test-embedder-tracing/TracingInEphemerons': [SKIP],
   'test-embedder-tracing/TracingInRevivedSubgraph': [SKIP],
   'test-embedder-tracing/V8RegisteringEmbedderReference': [SKIP],

test/cctest/test-global-handles.cc

@@ -44,6 +44,10 @@ namespace internal {
 
 namespace {
 
+struct TracedReferenceWrapper {
+  v8::TracedReference<v8::Object> handle;
+};
+
 // Empty v8::EmbedderHeapTracer that never keeps objects alive on Scavenge. See
 // |IsRootForNonTracingGC|.
 class NonRootingEmbedderHeapTracer final : public v8::EmbedderHeapTracer {
@@ -58,9 +62,26 @@ class NonRootingEmbedderHeapTracer final : public v8::EmbedderHeapTracer {
   void TraceEpilogue(TraceSummary*) final {}
   void EnterFinalPause(EmbedderStackState) final {}
 
-  bool IsRootForNonTracingGC(const v8::TracedGlobal<v8::Value>& handle) final {
+  bool IsRootForNonTracingGC(
+      const v8::TracedReference<v8::Value>& handle) final {
     return false;
   }
+
+  void ResetHandleInNonTracingGC(
+      const v8::TracedReference<v8::Value>& handle) final {
+    for (auto* wrapper : wrappers_) {
+      if (wrapper->handle == handle) {
+        wrapper->handle.Reset();
+      }
+    }
+  }
+
+  void Register(TracedReferenceWrapper* wrapper) {
+    wrappers_.push_back(wrapper);
+  }
+
+ private:
+  std::vector<TracedReferenceWrapper*> wrappers_;
 };
 
 void InvokeScavenge() { CcTest::CollectGarbage(i::NEW_SPACE); }
@@ -76,10 +97,6 @@ struct FlagAndGlobal {
   v8::Global<v8::Object> handle;
 };
 
-struct TracedGlobalWrapper {
-  v8::TracedGlobal<v8::Object> handle;
-};
-
 void ResetHandleAndSetFlag(const v8::WeakCallbackInfo<FlagAndGlobal>& data) {
   data.GetParameter()->handle.Reset();
   data.GetParameter()->flag = true;
@@ -104,12 +121,12 @@ void ConstructJSObject(v8::Isolate* isolate, v8::Global<v8::Object>* global) {
 }
 
 void ConstructJSObject(v8::Isolate* isolate,
-                       v8::TracedGlobal<v8::Object>* traced) {
+                       v8::TracedReference<v8::Object>* handle) {
   v8::HandleScope scope(isolate);
   v8::Local<v8::Object> object(v8::Object::New(isolate));
   CHECK(!object.IsEmpty());
-  *traced = v8::TracedGlobal<v8::Object>(isolate, object);
-  CHECK(!traced->IsEmpty());
+  *handle = v8::TracedReference<v8::Object>(isolate, object);
+  CHECK(!handle->IsEmpty());
 }
 
 template <typename HandleContainer>
@@ -150,12 +167,11 @@ void WeakHandleTest(v8::Isolate* isolate, ConstructFunction construct_function,
   CHECK_IMPLIES(survives == SurvivalMode::kDies, fp.flag);
 }
 
-template <typename ConstructFunction, typename ModifierFunction,
-          typename GCFunction>
-void TracedGlobalTest(v8::Isolate* isolate,
-                      ConstructFunction construct_function,
-                      ModifierFunction modifier_function,
-                      GCFunction gc_function, SurvivalMode survives) {
+template <typename ConstructFunction, typename ModifierFunction>
+void TracedReferenceTestWithScavenge(v8::Isolate* isolate,
+                                     ConstructFunction construct_function,
+                                     ModifierFunction modifier_function,
+                                     SurvivalMode survives) {
   v8::HandleScope scope(isolate);
   v8::Local<v8::Context> context = v8::Context::New(isolate);
   v8::Context::Scope context_scope(context);
@@ -163,11 +179,14 @@ void TracedGlobalTest(v8::Isolate* isolate,
   NonRootingEmbedderHeapTracer tracer;
   heap::TemporaryEmbedderHeapTracerScope tracer_scope(isolate, &tracer);
 
-  auto fp = std::make_unique<TracedGlobalWrapper>();
+  auto fp = std::make_unique<TracedReferenceWrapper>();
+  tracer.Register(fp.get());
   construct_function(isolate, context, fp.get());
   CHECK(heap::InCorrectGeneration(isolate, fp->handle));
   modifier_function(fp.get());
-  gc_function();
+  InvokeScavenge();
+  // Scavenge clear properly resets the original handle, so we can check the
+  // handle directly here.
   CHECK_IMPLIES(survives == SurvivalMode::kSurvives, !fp->handle.IsEmpty());
   CHECK_IMPLIES(survives == SurvivalMode::kDies, fp->handle.IsEmpty());
 }
@@ -343,14 +362,13 @@ TEST(WeakHandleToUnmodifiedJSObjectDiesOnScavenge) {
       SurvivalMode::kDies);
 }
 
-TEST(TracedGlobalToUnmodifiedJSObjectSurvivesScavenge) {
+TEST(TracedReferenceToUnmodifiedJSObjectSurvivesScavenge) {
   if (FLAG_single_generation) return;
   ManualGCScope manual_gc;
   CcTest::InitializeVM();
-  TracedGlobalTest(
-      CcTest::isolate(), &ConstructJSObject<TracedGlobalWrapper>,
-      [](TracedGlobalWrapper* fp) {}, []() { InvokeScavenge(); },
-      SurvivalMode::kSurvives);
+  TracedReferenceTestWithScavenge(
+      CcTest::isolate(), &ConstructJSObject<TracedReferenceWrapper>,
+      [](TracedReferenceWrapper* fp) {}, SurvivalMode::kSurvives);
 }
 
 TEST(WeakHandleToUnmodifiedJSObjectDiesOnMarkCompact) {
@@ -382,17 +400,16 @@ TEST(WeakHandleToUnmodifiedJSApiObjectDiesOnScavenge) {
       SurvivalMode::kDies);
 }
 
-TEST(TracedGlobalToUnmodifiedJSApiObjectDiesOnScavenge) {
+TEST(TracedReferenceToUnmodifiedJSApiObjectDiesOnScavenge) {
   if (FLAG_single_generation) return;
   ManualGCScope manual_gc;
   CcTest::InitializeVM();
-  TracedGlobalTest(
-      CcTest::isolate(), &ConstructJSApiObject<TracedGlobalWrapper>,
-      [](TracedGlobalWrapper* fp) {}, []() { InvokeScavenge(); },
-      SurvivalMode::kDies);
+  TracedReferenceTestWithScavenge(
+      CcTest::isolate(), &ConstructJSApiObject<TracedReferenceWrapper>,
+      [](TracedReferenceWrapper* fp) {}, SurvivalMode::kDies);
 }
 
-TEST(TracedGlobalToJSApiObjectWithIdentityHashSurvivesScavenge) {
+TEST(TracedReferenceToJSApiObjectWithIdentityHashSurvivesScavenge) {
   if (FLAG_single_generation) return;
 
   ManualGCScope manual_gc;
@@ -401,9 +418,9 @@ TEST(TracedGlobalToJSApiObjectWithIdentityHashSurvivesScavenge) {
   HandleScope scope(i_isolate);
   Handle<JSWeakMap> weakmap = i_isolate->factory()->NewJSWeakMap();
 
-  TracedGlobalTest(
-      CcTest::isolate(), &ConstructJSApiObject<TracedGlobalWrapper>,
-      [&weakmap, i_isolate](TracedGlobalWrapper* fp) {
+  TracedReferenceTestWithScavenge(
+      CcTest::isolate(), &ConstructJSApiObject<TracedReferenceWrapper>,
+      [&weakmap, i_isolate](TracedReferenceWrapper* fp) {
         v8::HandleScope scope(CcTest::isolate());
         Handle<JSReceiver> key =
             Utils::OpenHandle(*fp->handle.Get(CcTest::isolate()));
@@ -411,7 +428,7 @@ TEST(TracedGlobalToJSApiObjectWithIdentityHashSurvivesScavenge) {
         int32_t hash = key->GetOrCreateHash(i_isolate).value();
         JSWeakCollection::Set(weakmap, key, smi, hash);
       },
-      []() { InvokeScavenge(); }, SurvivalMode::kSurvives);
+      SurvivalMode::kSurvives);
 }
 
 TEST(WeakHandleToUnmodifiedJSApiObjectSurvivesScavengeWhenInHandle) {
@@ -447,13 +464,13 @@ TEST(WeakHandleToUnmodifiedJSApiObjectSurvivesMarkCompactWhenInHandle) {
       []() { InvokeMarkSweep(); }, SurvivalMode::kSurvives);
 }
 
-TEST(TracedGlobalToJSApiObjectWithModifiedMapSurvivesScavenge) {
+TEST(TracedReferenceToJSApiObjectWithModifiedMapSurvivesScavenge) {
   if (FLAG_single_generation) return;
   CcTest::InitializeVM();
   v8::Isolate* isolate = CcTest::isolate();
   LocalContext context;
 
-  TracedGlobal<v8::Object> handle;
+  TracedReference<v8::Object> handle;
   {
     v8::HandleScope scope(isolate);
     // Create an API object which does not have the same map as constructor.
@@ -469,13 +486,13 @@ TEST(TracedGlobalToJSApiObjectWithModifiedMapSurvivesScavenge) {
   CHECK(!handle.IsEmpty());
 }
 
-TEST(TracedGlobalTOJsApiObjectWithElementsSurvivesScavenge) {
+TEST(TracedReferenceTOJsApiObjectWithElementsSurvivesScavenge) {
   if (FLAG_single_generation) return;
   CcTest::InitializeVM();
   v8::Isolate* isolate = CcTest::isolate();
   LocalContext context;
 
-  TracedGlobal<v8::Object> handle;
+  TracedReference<v8::Object> handle;
   {
     v8::HandleScope scope(isolate);
 
@@ -717,13 +734,14 @@ TEST(TotalSizeTracedNode) {
   Isolate* i_isolate = CcTest::i_isolate();
   v8::HandleScope scope(isolate);
 
-  v8::TracedGlobal<v8::Object>* global = new TracedGlobal<v8::Object>();
+  v8::TracedReference<v8::Object>* handle = new TracedReference<v8::Object>();
   CHECK_EQ(i_isolate->global_handles()->TotalSize(), 0);
   CHECK_EQ(i_isolate->global_handles()->UsedSize(), 0);
-  ConstructJSObject(isolate, global);
+  ConstructJSObject(isolate, handle);
   CHECK_GT(i_isolate->global_handles()->TotalSize(), 0);
   CHECK_GT(i_isolate->global_handles()->UsedSize(), 0);
-  delete global;
+  delete handle;
+  InvokeMarkSweep();
   CHECK_GT(i_isolate->global_handles()->TotalSize(), 0);
   CHECK_EQ(i_isolate->global_handles()->UsedSize(), 0);
 }