heap: Fix collections initializations

- Fix various operator new invocations.
- Disallow nesting of HeapLinkedHashSet which depends on the fact that
  the collection itself does not move.

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

third_party/blink/renderer/core/animation/scroll_timeline.cc

@@ -22,7 +22,7 @@ namespace {
 using ActiveScrollTimelineSet = HeapHashCountedSet<WeakMember<Node>>;
 ActiveScrollTimelineSet& GetActiveScrollTimelineSet() {
   DEFINE_STATIC_LOCAL(Persistent<ActiveScrollTimelineSet>, set,
-                      (new ActiveScrollTimelineSet));
+                      (MakeGarbageCollected<ActiveScrollTimelineSet>()));
   return *set;
 }
 

third_party/blink/renderer/core/frame/local_dom_window.cc

@@ -131,14 +131,15 @@ using DOMWindowSet = HeapHashCountedSet<WeakMember<LocalDOMWindow>>;
 
 static DOMWindowSet& WindowsWithUnloadEventListeners() {
   DEFINE_STATIC_LOCAL(Persistent<DOMWindowSet>,
-                      windows_with_unload_event_listeners, (new DOMWindowSet));
+                      windows_with_unload_event_listeners,
+                      (MakeGarbageCollected<DOMWindowSet>()));
   return *windows_with_unload_event_listeners;
 }
 
 static DOMWindowSet& WindowsWithBeforeUnloadEventListeners() {
   DEFINE_STATIC_LOCAL(Persistent<DOMWindowSet>,
                       windows_with_before_unload_event_listeners,
-                      (new DOMWindowSet));
+                      (MakeGarbageCollected<DOMWindowSet>()));
   return *windows_with_before_unload_event_listeners;
 }
 

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

@@ -2304,9 +2304,9 @@ TEST(HeapTest, LargeVector) {
 
   // Try to allocate a HeapVectors larger than kMaxHeapObjectSize
   // (crbug.com/597953).
-  wtf_size_t size = kMaxHeapObjectSize / sizeof(int);
-  Persistent<HeapVector<int>> vector =
-      MakeGarbageCollected<HeapVector<int>>(size);
+  const wtf_size_t size = kMaxHeapObjectSize / sizeof(Member<IntWrapper>);
+  Persistent<HeapVector<Member<IntWrapper>>> vector =
+      MakeGarbageCollected<HeapVector<Member<IntWrapper>>>(size);
   EXPECT_LE(size, vector->capacity());
 }
 
@@ -2529,7 +2529,7 @@ TEST(HeapTest, HeapCollectionTypes) {
       MakeGarbageCollected<PrimitiveMember>();
   Persistent<MemberSet> set = MakeGarbageCollected<MemberSet>();
   Persistent<MemberSet> set2 = MakeGarbageCollected<MemberSet>();
-  Persistent<MemberCountedSet> set3 = new MemberCountedSet();
+  Persistent<MemberCountedSet> set3 = MakeGarbageCollected<MemberCountedSet>();
   Persistent<MemberVector> vector = MakeGarbageCollected<MemberVector>();
   Persistent<MemberVector> vector2 = MakeGarbageCollected<MemberVector>();
   Persistent<VectorWU> vector_wu = MakeGarbageCollected<VectorWU>();
@@ -3107,7 +3107,8 @@ TEST(HeapTest, HeapWeakCollectionSimple) {
   Persistent<StrongWeak> strong_weak = MakeGarbageCollected<StrongWeak>();
   Persistent<WeakWeak> weak_weak = MakeGarbageCollected<WeakWeak>();
   Persistent<WeakSet> weak_set = MakeGarbageCollected<WeakSet>();
-  Persistent<WeakCountedSet> weak_counted_set = new WeakCountedSet();
+  Persistent<WeakCountedSet> weak_counted_set =
+      MakeGarbageCollected<WeakCountedSet>();
 
   Persistent<IntWrapper> two = MakeGarbageCollected<IntWrapper>(2);
 
@@ -5982,58 +5983,6 @@ TEST(HeapTest, HeapHashMapCallsDestructor) {
   EXPECT_TRUE(string.Impl()->HasOneRef());
 }
 
-class DoublyLinkedListNodeImpl
-    : public GarbageCollectedFinalized<DoublyLinkedListNodeImpl>,
-      public DoublyLinkedListNode<DoublyLinkedListNodeImpl> {
- public:
-  DoublyLinkedListNodeImpl() = default;
-
-  static int destructor_calls_;
-  ~DoublyLinkedListNodeImpl() { ++destructor_calls_; }
-
-  void Trace(Visitor* visitor) {
-    visitor->Trace(prev_);
-    visitor->Trace(next_);
-  }
-
- private:
-  friend class WTF::DoublyLinkedListNode<DoublyLinkedListNodeImpl>;
-  Member<DoublyLinkedListNodeImpl> prev_;
-  Member<DoublyLinkedListNodeImpl> next_;
-};
-
-int DoublyLinkedListNodeImpl::destructor_calls_ = 0;
-
-template <typename T>
-class HeapDoublyLinkedListContainer
-    : public GarbageCollected<HeapDoublyLinkedListContainer<T>> {
- public:
-  HeapDoublyLinkedListContainer<T>() = default;
-  HeapDoublyLinkedList<T> list_;
-  void Trace(Visitor* visitor) { visitor->Trace(list_); }
-};
-
-TEST(HeapTest, HeapDoublyLinkedList) {
-  Persistent<HeapDoublyLinkedListContainer<DoublyLinkedListNodeImpl>>
-      container = MakeGarbageCollected<
-          HeapDoublyLinkedListContainer<DoublyLinkedListNodeImpl>>();
-  DoublyLinkedListNodeImpl::destructor_calls_ = 0;
-
-  container->list_.Append(MakeGarbageCollected<DoublyLinkedListNodeImpl>());
-  container->list_.Append(MakeGarbageCollected<DoublyLinkedListNodeImpl>());
-
-  PreciselyCollectGarbage();
-  EXPECT_EQ(DoublyLinkedListNodeImpl::destructor_calls_, 0);
-
-  container->list_.RemoveHead();
-  PreciselyCollectGarbage();
-  EXPECT_EQ(DoublyLinkedListNodeImpl::destructor_calls_, 1);
-
-  container->list_.RemoveHead();
-  PreciselyCollectGarbage();
-  EXPECT_EQ(DoublyLinkedListNodeImpl::destructor_calls_, 2);
-}
-
 TEST(HeapTest, PromptlyFreeStackAllocatedHeapVector) {
   NormalPageArena* normal_arena;
   Address before;
@@ -6151,6 +6100,9 @@ TEST(HeapTest, GarbageCollectedMixinIsAliveDuringConstruction) {
   using O = ObjectWithMixinWithCallbackBeforeInitializer<IntWrapper>;
   MakeGarbageCollected<O>(base::BindOnce(
       [](O::Mixin* thiz) { CHECK(ThreadHeap::IsHeapObjectAlive(thiz)); }));
+
+  using P = HeapVector<Member<HeapLinkedHashSet<Member<IntWrapper>>>>;
+  MakeGarbageCollected<P>();
 }
 
 }  // namespace blink

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

@@ -658,57 +658,6 @@ TEST(IncrementalMarkingTest, HeapVectorEagerTracingStopsAtMember) {
   }
 }
 
-// =============================================================================
-// HeapDoublyLinkedList support. ===============================================
-// =============================================================================
-
-namespace {
-
-class ObjectNode : public GarbageCollected<ObjectNode>,
-                   public DoublyLinkedListNode<ObjectNode> {
- public:
-  explicit ObjectNode(Object* obj) : obj_(obj) {}
-
-  void Trace(Visitor* visitor) {
-    visitor->Trace(obj_);
-    visitor->Trace(prev_);
-    visitor->Trace(next_);
-  }
-
- private:
-  friend class WTF::DoublyLinkedListNode<ObjectNode>;
-
-  Member<Object> obj_;
-  Member<ObjectNode> prev_;
-  Member<ObjectNode> next_;
-};
-
-}  // namespace
-
-TEST(IncrementalMarkingTest, HeapDoublyLinkedListPush) {
-  auto* obj = MakeGarbageCollected<Object>();
-  ObjectNode* obj_node = MakeGarbageCollected<ObjectNode>(obj);
-  HeapDoublyLinkedList<ObjectNode> list;
-  {
-    ExpectWriteBarrierFires scope(ThreadState::Current(), {obj_node});
-    list.Push(obj_node);
-    // |obj| will be marked once |obj_node| gets processed.
-    EXPECT_FALSE(obj->IsMarked());
-  }
-}
-
-TEST(IncrementalMarkingTest, HeapDoublyLinkedListAppend) {
-  auto* obj = MakeGarbageCollected<Object>();
-  ObjectNode* obj_node = MakeGarbageCollected<ObjectNode>(obj);
-  HeapDoublyLinkedList<ObjectNode> list;
-  {
-    ExpectWriteBarrierFires scope(ThreadState::Current(), {obj_node});
-    list.Append(obj_node);
-    // |obj| will be marked once |obj_node| gets processed.
-    EXPECT_FALSE(obj->IsMarked());
-  }
-}
-
 // =============================================================================
 // HeapDeque support. ==========================================================
 // =============================================================================
@@ -1602,7 +1551,7 @@ TEST(IncrementalMarkingTest, DropBackingStore) {
   // Regression test: https://crbug.com/828537
   using WeakStore = HeapHashCountedSet<WeakMember<Object>>;
 
-  Persistent<WeakStore> persistent(new WeakStore);
+  Persistent<WeakStore> persistent(MakeGarbageCollected<WeakStore>());
   persistent->insert(MakeGarbageCollected<Object>());
   IncrementalMarkingTestDriver driver(ThreadState::Current());
   driver.Start();
@@ -1619,7 +1568,7 @@ TEST(IncrementalMarkingTest, WeakCallbackDoesNotReviveDeletedValue) {
   // std::pair avoids treating the hashset backing as weak backing.
   using WeakStore = HeapHashCountedSet<std::pair<WeakMember<Object>, size_t>>;
 
-  Persistent<WeakStore> persistent(new WeakStore);
+  Persistent<WeakStore> persistent(MakeGarbageCollected<WeakStore>());
   // Create at least two entries to avoid completely emptying out the data
   // structure. The values for .second are chosen to be non-null as they
   // would otherwise count as empty and be skipped during iteration after the
@@ -1654,7 +1603,7 @@ TEST(IncrementalMarkingTest, NoBackingFreeDuringIncrementalMarking) {
   // Only reproduces in ASAN configurations.
   using WeakStore = HeapHashCountedSet<std::pair<WeakMember<Object>, size_t>>;
 
-  Persistent<WeakStore> persistent(new WeakStore);
+  Persistent<WeakStore> persistent(MakeGarbageCollected<WeakStore>());
   // Prefill the collection to grow backing store. A new backing store allocaton
   // would trigger the write barrier, mitigating the bug where a backing store
   // is promptly freed.
@@ -1674,7 +1623,7 @@ TEST(IncrementalMarkingTest, NoBackingFreeDuringIncrementalMarking) {
 TEST(IncrementalMarkingTest, DropReferenceWithHeapCompaction) {
   using Store = HeapHashCountedSet<Member<Object>>;
 
-  Persistent<Store> persistent(new Store());
+  Persistent<Store> persistent(MakeGarbageCollected<Store>());
   persistent->insert(MakeGarbageCollected<Object>());
   IncrementalMarkingTestDriver driver(ThreadState::Current());
   ThreadState::Current()->EnableCompactionForNextGCForTesting();
@@ -1711,7 +1660,7 @@ TEST(IncrementalMarkingTest, HasInlineCapacityCollectionWithHeapCompaction) {
 TEST(IncrementalMarkingTest, WeakHashMapHeapCompaction) {
   using Store = HeapHashCountedSet<WeakMember<Object>>;
 
-  Persistent<Store> persistent(new Store());
+  Persistent<Store> persistent(MakeGarbageCollected<Store>());
 
   IncrementalMarkingTestDriver driver(ThreadState::Current());
   ThreadState::Current()->EnableCompactionForNextGCForTesting();

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

@@ -128,11 +128,9 @@ class DoublyLinkedList {
 template <typename T, typename PointerType>
 inline DoublyLinkedList<T, PointerType>::DoublyLinkedList()
     : head_(nullptr), tail_(nullptr) {
-  static_assert(
-      !IsGarbageCollectedType<T>::value ||
-          !std::is_same<PointerType, T*>::value,
-      "Cannot use DoublyLinkedList<> with garbage collected types, use "
-      "HeapDoublyLinkedList<> instead.");
+  static_assert(!IsGarbageCollectedType<T>::value ||
+                    !std::is_same<PointerType, T*>::value,
+                "Cannot use DoublyLinkedList<> with garbage collected types.");
 }
 
 template <typename T, typename PointerType>