heap: Trivial code health cleanups around ThreadState

- Move trivial initializers to declaration site
- Move Visit* methods to private section
- Remove unused parameter around Persistent APIs

Bug: 982754
Change-Id: Iec4102f1466a8b65251aa69892d041f450ac069e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1743636
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#685265}

third_party/blink/renderer/platform/heap/persistent.h

@@ -220,10 +220,11 @@ class PersistentBase {
   // clean LSan leak reports or to register a thread-local persistent
   // needing to be cleared out before the thread is terminated.
   PersistentBase* RegisterAsStaticReference() {
-    CHECK_EQ(weaknessConfiguration, kNonWeakPersistentConfiguration);
+    static_assert(weaknessConfiguration == kNonWeakPersistentConfiguration,
+                  "Can only register non-weak Persistent references as static "
+                  "references.");
     if (PersistentNode* node = persistent_node_.Get()) {
-      DCHECK(ThreadState::Current());
-      ThreadState::Current()->RegisterStaticPersistentNode(node, nullptr);
+      ThreadState::Current()->RegisterStaticPersistentNode(node);
       LEAK_SANITIZER_IGNORE_OBJECT(this);
     }
     return this;

third_party/blink/renderer/platform/heap/persistent_node.cc

@@ -29,7 +29,7 @@ PersistentRegion::~PersistentRegion() {
 }
 
 int PersistentRegion::NumberOfPersistents() {
-  int persistent_count = 0;
+  size_t persistent_count = 0;
   for (PersistentNodeSlots* slots = slots_; slots; slots = slots->next_) {
     for (int i = 0; i < PersistentNodeSlots::kSlotCount; ++i) {
       if (!slots->slot_[i].IsUnused())
@@ -56,17 +56,10 @@ void PersistentRegion::EnsurePersistentNodeSlots(void* self,
   slots_ = slots;
 }
 
-void PersistentRegion::ReleasePersistentNode(
-    PersistentNode* persistent_node,
-    ThreadState::PersistentClearCallback callback) {
+void PersistentRegion::ReleasePersistentNode(PersistentNode* persistent_node) {
   DCHECK(!persistent_node->IsUnused());
   // 'self' is in use, containing the persistent wrapper object.
   void* self = persistent_node->Self();
-  if (callback) {
-    (*callback)(self);
-    DCHECK(persistent_node->IsUnused());
-    return;
-  }
   Persistent<DummyGCBase>* persistent =
       reinterpret_cast<Persistent<DummyGCBase>*>(self);
   persistent->Clear();
@@ -80,7 +73,7 @@ void PersistentRegion::ReleasePersistentNode(
 void PersistentRegion::TracePersistentNodes(Visitor* visitor,
                                             ShouldTraceCallback should_trace) {
   free_list_head_ = nullptr;
-  int persistent_count = 0;
+  size_t persistent_count = 0;
   PersistentNodeSlots** prev_next = &slots_;
   PersistentNodeSlots* slots = slots_;
   while (slots) {
@@ -142,7 +135,7 @@ void PersistentRegion::PrepareForThreadStateTermination() {
     slots = slots->next_;
   }
 #if DCHECK_IS_ON()
-  DCHECK_EQ(persistent_count_, 0);
+  DCHECK_EQ(persistent_count_, 0u);
 #endif
 }
 
@@ -197,7 +190,7 @@ void CrossThreadPersistentRegion::UnpoisonCrossThreadPersistents() {
 #if DCHECK_IS_ON()
   ProcessHeap::CrossThreadPersistentMutex().AssertAcquired();
 #endif
-  int persistent_count = 0;
+  size_t persistent_count = 0;
   for (PersistentNodeSlots* slots = persistent_region_.slots_; slots;
        slots = slots->next_) {
     for (int i = 0; i < PersistentNodeSlots::kSlotCount; ++i) {

third_party/blink/renderer/platform/heap/persistent_node.h

@@ -161,15 +161,7 @@ class PLATFORM_EXPORT PersistentRegion final {
   USING_FAST_MALLOC(PersistentRegion);
 
  public:
-  PersistentRegion()
-      : free_list_head_(nullptr),
-        slots_(nullptr)
-#if DCHECK_IS_ON()
-        ,
-        persistent_count_(0)
-#endif
-  {
-  }
+  PersistentRegion() = default;
   ~PersistentRegion();
 
   PersistentNode* AllocatePersistentNode(void* self, TraceCallback trace) {
@@ -188,7 +180,7 @@ class PLATFORM_EXPORT PersistentRegion final {
 
   void FreePersistentNode(PersistentNode* persistent_node) {
 #if DCHECK_IS_ON()
-    DCHECK_GT(persistent_count_, 0);
+    DCHECK_GT(persistent_count_, 0u);
 #endif
     persistent_node->SetFreeListNext(free_list_head_);
     free_list_head_ = persistent_node;
@@ -201,8 +193,7 @@ class PLATFORM_EXPORT PersistentRegion final {
     return true;
   }
 
-  void ReleasePersistentNode(PersistentNode*,
-                             ThreadState::PersistentClearCallback);
+  void ReleasePersistentNode(PersistentNode*);
   using ShouldTraceCallback = bool (*)(Visitor*, PersistentNode*);
   void TracePersistentNodes(
       Visitor*,
@@ -215,10 +206,10 @@ class PLATFORM_EXPORT PersistentRegion final {
 
   void EnsurePersistentNodeSlots(void*, TraceCallback);
 
-  PersistentNode* free_list_head_;
-  PersistentNodeSlots* slots_;
+  PersistentNode* free_list_head_ = nullptr;
+  PersistentNodeSlots* slots_ = nullptr;
 #if DCHECK_IS_ON()
-  int persistent_count_;
+  size_t persistent_count_ = 0;
 #endif
 };
 
@@ -272,7 +263,6 @@ class PLATFORM_EXPORT CrossThreadPersistentRegion final {
 #endif
 
  private:
-
   // We don't make CrossThreadPersistentRegion inherit from PersistentRegion
   // because we don't want to virtualize performance-sensitive methods
   // such as PersistentRegion::allocate/freePersistentNode.

third_party/blink/renderer/platform/heap/thread_state.cc

@@ -130,13 +130,9 @@ ThreadState::ThreadState()
       persistent_region_(std::make_unique<PersistentRegion>()),
       weak_persistent_region_(std::make_unique<PersistentRegion>()),
       start_of_stack_(reinterpret_cast<Address*>(WTF::GetStackStart())),
-      gc_state_(kNoGCScheduled),
-      gc_phase_(GCPhase::kNone),
-      reason_for_scheduled_gc_(BlinkGC::GCReason::kForcedGCForTesting),
 #if defined(ADDRESS_SANITIZER)
       asan_fake_stack_(__asan_get_current_fake_stack()),
 #endif
-      reported_memory_to_v8_(0),
       sweeper_scheduler_(base::MakeRefCounted<WorkerPoolTaskRunner>()) {
   DCHECK(CheckThread());
   DCHECK(!**thread_specific_);
@@ -1173,24 +1169,21 @@ void ThreadState::LeaveStaticReferenceRegistrationDisabledScope() {
   static_persistent_registration_disabled_count_--;
 }
 
-void ThreadState::RegisterStaticPersistentNode(
-    PersistentNode* node,
-    PersistentClearCallback callback) {
+void ThreadState::RegisterStaticPersistentNode(PersistentNode* node) {
   if (static_persistent_registration_disabled_count_)
     return;
 
   DCHECK(!static_persistents_.Contains(node));
-  static_persistents_.insert(node, callback);
+  static_persistents_.insert(node);
 }
 
 void ThreadState::ReleaseStaticPersistentNodes() {
-  HashMap<PersistentNode*, ThreadState::PersistentClearCallback>
-      static_persistents;
+  HashSet<PersistentNode*> static_persistents;
   static_persistents.swap(static_persistents_);
 
   PersistentRegion* persistent_region = GetPersistentRegion();
-  for (const auto& it : static_persistents)
-    persistent_region->ReleasePersistentNode(it.key, it.value);
+  for (PersistentNode* it : static_persistents)
+    persistent_region->ReleasePersistentNode(it);
 }
 
 void ThreadState::FreePersistentNode(PersistentRegion* persistent_region,

third_party/blink/renderer/platform/heap/thread_state.h

@@ -338,15 +338,6 @@ class PLATFORM_EXPORT ThreadState final : private RAILModeObserver {
     return weak_persistent_region_.get();
   }
 
-  // Visit all non-weak persistents allocated on this thread.
-  void VisitPersistents(Visitor*);
-
-  // Visit all weak persistents allocated on this thread.
-  void VisitWeakPersistents(Visitor*);
-
-  // Visit all DOM wrappers allocatd on this thread.
-  void VisitDOMWrappers(Visitor*);
-
   struct GCSnapshotInfo {
     STACK_ALLOCATED();
 
@@ -360,12 +351,9 @@ class PLATFORM_EXPORT ThreadState final : private RAILModeObserver {
     Vector<size_t> dead_size;
   };
 
-  void FreePersistentNode(PersistentRegion*, PersistentNode*);
-
-  using PersistentClearCallback = void (*)(void*);
-
-  void RegisterStaticPersistentNode(PersistentNode*, PersistentClearCallback);
+  void RegisterStaticPersistentNode(PersistentNode*);
   void ReleaseStaticPersistentNodes();
+  void FreePersistentNode(PersistentRegion*, PersistentNode*);
 
   v8::Isolate* GetIsolate() const { return isolate_; }
 
@@ -520,6 +508,15 @@ class PLATFORM_EXPORT ThreadState final : private RAILModeObserver {
                                     Address*,
                                     Address*);
 
+  // Visit all non-weak persistents allocated on this thread.
+  void VisitPersistents(Visitor*);
+
+  // Visit all weak persistents allocated on this thread.
+  void VisitWeakPersistents(Visitor*);
+
+  // Visit all DOM wrappers allocatd on this thread.
+  void VisitDOMWrappers(Visitor*);
+
   // ShouldForceConservativeGC
   // implements the heuristics that are used to determine when to collect
   // garbage.
@@ -602,9 +599,10 @@ class PLATFORM_EXPORT ThreadState final : private RAILModeObserver {
   base::TimeDelta next_incremental_marking_step_duration_;
   base::TimeDelta previous_incremental_marking_time_left_;
 
-  GCState gc_state_;
-  GCPhase gc_phase_;
-  BlinkGC::GCReason reason_for_scheduled_gc_;
+  GCState gc_state_ = GCState::kNoGCScheduled;
+  GCPhase gc_phase_ = GCPhase::kNone;
+  BlinkGC::GCReason reason_for_scheduled_gc_ =
+      BlinkGC::GCReason::kForcedGCForTesting;
 
   using PreFinalizerCallback = bool (*)(void*);
   using PreFinalizer = std::pair<void*, PreFinalizerCallback>;
@@ -630,10 +628,9 @@ class PLATFORM_EXPORT ThreadState final : private RAILModeObserver {
   // references that either have to be cleared upon the thread
   // detaching from Oilpan and shutting down or references we
   // have to clear before initiating LSan's leak detection.
-  HashMap<PersistentNode*, PersistentClearCallback> static_persistents_;
-
-  size_t reported_memory_to_v8_;
+  HashSet<PersistentNode*> static_persistents_;
 
+  size_t reported_memory_to_v8_ = 0;
   int gc_age_ = 0;
 
   struct GCData {