heap: Migrate HeapListHashSet to GarbageCollected

Bug: 1056170
Change-Id: I0c3420dfb6d0fd978a25aee28c95b412d91a3bd3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2572844
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Auto-Submit: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#833780}

third_party/blink/renderer/platform/heap/BUILD.gn

@@ -57,6 +57,7 @@ blink_platform_sources("heap") {
     "collection_support/heap_hash_table_backing.h",
     "collection_support/heap_linked_hash_set.h",
     "collection_support/heap_linked_stack.h",
+    "collection_support/heap_list_hash_set.h",
     "collection_support/heap_vector_backing.h",
     "disallow_new_wrapper.h",
     "garbage_collected.h",

third_party/blink/renderer/platform/heap/collection_support/heap_list_hash_set.h

+// Copyright 2020 The Chromium 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 THIRD_PARTY_BLINK_RENDERER_PLATFORM_HEAP_COLLECTION_SUPPORT_HEAP_LIST_HASH_SET_H_
+#define THIRD_PARTY_BLINK_RENDERER_PLATFORM_HEAP_COLLECTION_SUPPORT_HEAP_LIST_HASH_SET_H_
+
+#include "third_party/blink/renderer/platform/heap/heap.h"
+#include "third_party/blink/renderer/platform/wtf/list_hash_set.h"
+#include "third_party/blink/renderer/platform/wtf/type_traits.h"
+
+namespace blink {
+
+class HeapListHashSetAllocator;
+template <typename ValueArg>
+class HeapListHashSetNode;
+
+template <typename ValueArg>
+class HeapListHashSetNode final
+    : public GarbageCollected<HeapListHashSetNode<ValueArg>> {
+ public:
+  using NodeAllocator = HeapListHashSetAllocator;
+  using PointerType = Member<HeapListHashSetNode>;
+  using Value = ValueArg;
+
+  template <typename U>
+  static HeapListHashSetNode* Create(NodeAllocator* allocator, U&& value) {
+    return MakeGarbageCollected<HeapListHashSetNode>(std::forward<U>(value));
+  }
+
+  template <typename U>
+  explicit HeapListHashSetNode(U&& value) noexcept
+      : value_(std::forward<U>(value)) {
+    static_assert(std::is_trivially_destructible<Value>::value,
+                  "Garbage collected types used in ListHashSet must be "
+                  "trivially destructible");
+  }
+
+  HeapListHashSetNode() = delete;
+  HeapListHashSetNode(const HeapListHashSetNode&) = delete;
+  HeapListHashSetNode& operator=(const HeapListHashSetNode&) = delete;
+
+  void Destroy(NodeAllocator* allocator) {}
+
+  HeapListHashSetNode* Next() const { return next_; }
+  HeapListHashSetNode* Prev() const { return prev_; }
+
+  void Trace(Visitor* visitor) const {
+    visitor->Trace(prev_);
+    visitor->Trace(next_);
+    visitor->Trace(value_);
+  }
+
+  ValueArg value_;
+  PointerType prev_ = nullptr;
+  PointerType next_ = nullptr;
+};
+
+// Empty allocator as HeapListHashSetNode directly allocates using
+// MakeGarbageCollected().
+class HeapListHashSetAllocator final {
+  DISALLOW_NEW();
+
+ public:
+  using TableAllocator = HeapAllocator;
+
+  static constexpr bool kIsGarbageCollected = true;
+
+  struct AllocatorProvider final {
+    void CreateAllocatorIfNeeded() {}
+    HeapListHashSetAllocator* Get() { return nullptr; }
+    void Swap(AllocatorProvider& other) {}
+  };
+};
+
+template <typename ValueArg,
+          wtf_size_t inlineCapacity = 0,  // The inlineCapacity is just a dummy
+                                          // to match ListHashSet (off-heap).
+          typename HashArg = typename DefaultHash<ValueArg>::Hash>
+class HeapListHashSet final
+    : public GarbageCollected<
+          HeapListHashSet<ValueArg, inlineCapacity, HashArg>>,
+      public ListHashSet<ValueArg,
+                         inlineCapacity,
+                         HashArg,
+                         HeapListHashSetAllocator> {
+ public:
+  HeapListHashSet() = default;
+
+  void Trace(Visitor* v) const {
+    CheckType();
+    ListHashSet<ValueArg, inlineCapacity, HashArg,
+                HeapListHashSetAllocator>::Trace(v);
+  }
+
+ private:
+  static void CheckType() {
+    static_assert(WTF::IsMemberOrWeakMemberType<ValueArg>::value,
+                  "HeapListHashSet supports only Member and WeakMember.");
+    static_assert(std::is_trivially_destructible<HeapListHashSet>::value,
+                  "HeapListHashSet must be trivially destructible.");
+    static_assert(WTF::IsTraceable<ValueArg>::value,
+                  "For sets without traceable elements, use ListHashSet<> "
+                  "instead of HeapListHashSet<>.");
+  }
+};
+
+}  // namespace blink
+
+namespace WTF {
+
+template <typename Value, wtf_size_t inlineCapacity>
+struct ListHashSetTraits<Value, inlineCapacity, blink::HeapListHashSetAllocator>
+    : public HashTraits<blink::Member<blink::HeapListHashSetNode<Value>>> {
+  using Allocator = blink::HeapListHashSetAllocator;
+  using Node = blink::HeapListHashSetNode<Value>;
+
+  static constexpr bool kCanTraceConcurrently =
+      HashTraits<Value>::kCanTraceConcurrently;
+};
+
+}  // namespace WTF
+
+#endif  // THIRD_PARTY_BLINK_RENDERER_PLATFORM_HEAP_COLLECTION_SUPPORT_HEAP_LIST_HASH_SET_H_

third_party/blink/renderer/platform/heap/impl/heap_allocator.h

@@ -10,6 +10,7 @@
 #include "build/build_config.h"
 #include "third_party/blink/renderer/platform/heap/collection_support/heap_hash_table_backing.h"
 #include "third_party/blink/renderer/platform/heap/collection_support/heap_linked_hash_set.h"
+#include "third_party/blink/renderer/platform/heap/collection_support/heap_list_hash_set.h"
 #include "third_party/blink/renderer/platform/heap/collection_support/heap_vector_backing.h"
 #include "third_party/blink/renderer/platform/heap/heap.h"
 #include "third_party/blink/renderer/platform/heap/heap_buildflags.h"
@@ -25,17 +26,11 @@
 #include "third_party/blink/renderer/platform/wtf/hash_map.h"
 #include "third_party/blink/renderer/platform/wtf/hash_set.h"
 #include "third_party/blink/renderer/platform/wtf/hash_table.h"
-#include "third_party/blink/renderer/platform/wtf/linked_hash_set.h"
-#include "third_party/blink/renderer/platform/wtf/list_hash_set.h"
 #include "third_party/blink/renderer/platform/wtf/type_traits.h"
 #include "third_party/blink/renderer/platform/wtf/vector.h"
 
 namespace blink {
 
-class HeapListHashSetAllocator;
-template <typename ValueArg>
-class HeapListHashSetNode;
-
 namespace internal {
 
 template <typename T>
@@ -44,7 +39,7 @@ constexpr bool IsMember = WTF::IsSubclassOfTemplate<T, Member>::value;
 }  // namespace internal
 
 // This is a static-only class used as a trait on collections to make them heap
-// allocated.  However see also HeapListHashSetAllocator.
+// allocated.
 class PLATFORM_EXPORT HeapAllocator {
   STATIC_ONLY(HeapAllocator);
 
@@ -264,113 +259,6 @@ template <typename T, typename U, typename V>
 struct GCInfoTrait<HeapHashSet<T, U, V>>
     : public GCInfoTrait<HashSet<T, U, V, HeapAllocator>> {};
 
-}  // namespace blink
-
-namespace WTF {
-
-template <typename Value, wtf_size_t inlineCapacity>
-struct ListHashSetTraits<Value, inlineCapacity, blink::HeapListHashSetAllocator>
-    : public HashTraits<blink::Member<blink::HeapListHashSetNode<Value>>> {
-  using Allocator = blink::HeapListHashSetAllocator;
-  using Node = blink::HeapListHashSetNode<Value>;
-
-  static constexpr bool kCanTraceConcurrently =
-      HashTraits<Value>::kCanTraceConcurrently;
-};
-
-}  // namespace WTF
-
-namespace blink {
-
-template <typename ValueArg>
-class HeapListHashSetNode final
-    : public GarbageCollected<HeapListHashSetNode<ValueArg>> {
- public:
-  using NodeAllocator = HeapListHashSetAllocator;
-  using PointerType = Member<HeapListHashSetNode>;
-  using Value = ValueArg;
-
-  template <typename U>
-  static HeapListHashSetNode* Create(NodeAllocator* allocator, U&& value) {
-    return MakeGarbageCollected<HeapListHashSetNode>(std::forward<U>(value));
-  }
-
-  template <typename U>
-  explicit HeapListHashSetNode(U&& value) : value_(std::forward<U>(value)) {
-    static_assert(std::is_trivially_destructible<Value>::value,
-                  "Garbage collected types used in ListHashSet must be "
-                  "trivially destructible");
-  }
-
-  void Destroy(NodeAllocator* allocator) {}
-
-  HeapListHashSetNode* Next() const { return next_; }
-  HeapListHashSetNode* Prev() const { return prev_; }
-
-  void Trace(Visitor* visitor) const {
-    visitor->Trace(prev_);
-    visitor->Trace(next_);
-    visitor->Trace(value_);
-  }
-
-  ValueArg value_;
-  PointerType prev_;
-  PointerType next_;
-};
-
-// Empty allocator as HeapListHashSetNode directly allocates using
-// MakeGarbageCollected().
-class HeapListHashSetAllocator {
-  DISALLOW_NEW();
-
- public:
-  using TableAllocator = HeapAllocator;
-
-  static constexpr bool kIsGarbageCollected = true;
-
-  struct AllocatorProvider final {
-    void CreateAllocatorIfNeeded() {}
-    HeapListHashSetAllocator* Get() { return nullptr; }
-    void Swap(AllocatorProvider& other) {}
-  };
-};
-
-template <typename ValueArg,
-          wtf_size_t inlineCapacity = 0,  // The inlineCapacity is just a dummy
-                                          // to match ListHashSet (off-heap).
-          typename HashArg = typename DefaultHash<ValueArg>::Hash>
-class HeapListHashSet : public ListHashSet<ValueArg,
-                                           inlineCapacity,
-                                           HashArg,
-                                           HeapListHashSetAllocator> {
-  IS_GARBAGE_COLLECTED_CONTAINER_TYPE();
-  DISALLOW_NEW();
-
-  static void CheckType() {
-    static_assert(WTF::IsMemberOrWeakMemberType<ValueArg>::value,
-                  "HeapListHashSet supports only Member and WeakMember.");
-    static_assert(std::is_trivially_destructible<HeapListHashSet>::value,
-                  "HeapListHashSet must be trivially destructible.");
-    static_assert(WTF::IsTraceable<ValueArg>::value,
-                  "For sets without traceable elements, use ListHashSet<> "
-                  "instead of HeapListHashSet<>.");
-  }
-
- public:
-  template <typename>
-  static void* AllocateObject(size_t size) {
-    return ThreadHeap::Allocate<
-        HeapListHashSet<ValueArg, inlineCapacity, HashArg>>(size);
-  }
-
-  HeapListHashSet() { CheckType(); }
-};
-
-template <typename T, wtf_size_t inlineCapacity, typename U>
-struct GCInfoTrait<HeapListHashSet<T, inlineCapacity, U>>
-    : public GCInfoTrait<
-          ListHashSet<T, inlineCapacity, U, HeapListHashSetAllocator>> {};
-
 template <typename Value,
           typename HashFunctions = typename DefaultHash<Value>::Hash,
           typename Traits = HashTraits<Value>>

third_party/blink/renderer/platform/heap/test/concurrent_marking_test.cc

@@ -230,31 +230,28 @@ TEST_F(ConcurrentMarkingTest, SwapLinkedHashSet) {
 // HeapListHashSet
 
 template <typename T>
-class HeapListHashSetAdapter : public HeapListHashSet<T> {
- public:
-  ALWAYS_INLINE void swap(HeapListHashSetAdapter<T>& other) {
-    HeapListHashSet<T>::Swap(other);
-  }
+struct MethodAdapter<HeapListHashSet<T>>
+    : public MethodAdapterBase<HeapListHashSet<T>> {
+  static void Swap(HeapListHashSet<T>& a, HeapListHashSet<T>& b) { a.Swap(b); }
 };
 
 TEST_F(ConcurrentMarkingTest, AddToListHashSet) {
-  AddToCollection<HeapListHashSetAdapter<Member<IntegerObject>>>();
+  AddToCollection<HeapListHashSet<Member<IntegerObject>>>();
 }
 TEST_F(ConcurrentMarkingTest, RemoveFromBeginningOfListHashSet) {
-  RemoveFromBeginningOfCollection<
-      HeapListHashSetAdapter<Member<IntegerObject>>>();
+  RemoveFromBeginningOfCollection<HeapListHashSet<Member<IntegerObject>>>();
 }
 TEST_F(ConcurrentMarkingTest, RemoveFromMiddleOfListHashSet) {
-  RemoveFromMiddleOfCollection<HeapListHashSetAdapter<Member<IntegerObject>>>();
+  RemoveFromMiddleOfCollection<HeapListHashSet<Member<IntegerObject>>>();
 }
 TEST_F(ConcurrentMarkingTest, RemoveFromEndOfListHashSet) {
-  RemoveFromEndOfCollection<HeapListHashSetAdapter<Member<IntegerObject>>>();
+  RemoveFromEndOfCollection<HeapListHashSet<Member<IntegerObject>>>();
 }
 TEST_F(ConcurrentMarkingTest, ClearListHashSet) {
-  ClearCollection<HeapListHashSetAdapter<Member<IntegerObject>>>();
+  ClearCollection<HeapListHashSet<Member<IntegerObject>>>();
 }
 TEST_F(ConcurrentMarkingTest, SwapListHashSet) {
-  SwapCollections<HeapListHashSetAdapter<Member<IntegerObject>>>();
+  SwapCollections<HeapListHashSet<Member<IntegerObject>>>();
 }
 
 // HeapHashCountedSet

third_party/blink/renderer/platform/wtf/list_hash_set.h

@@ -101,6 +101,10 @@ class MallocedListHashSetNode {
   explicit MallocedListHashSetNode(U&& value)
       : value_(std::forward<U>(value)) {}
 
+  MallocedListHashSetNode() = delete;
+  MallocedListHashSetNode(const MallocedListHashSetNode&) = delete;
+  MallocedListHashSetNode& operator=(const MallocedListHashSetNode&) = delete;
+
   void Destroy(NodeAllocator* allocator) {
     this->~MallocedListHashSetNode();
     allocator->Deallocate(this);