Oilpan: Backing slots are not registered for some weak HashTables

Weak HashTable's backing slots went unregistered, when it gains a backing after it has been traced.
We need to register the backing slot from HashTable::Trace even when there is no backing yet.

Bug: 864425
Change-Id: I09a0f6031e457242a4374429dde5360777bebc21
Reviewed-on: https://chromium-review.googlesource.com/c/1312189Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Keishi Hattori <keishi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604875}

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

@@ -315,7 +315,8 @@ HeapCompact::HeapCompact(ThreadHeap* heap)
       do_compact_(false),
       gc_count_since_last_compaction_(0),
       free_list_size_(0),
-      compactable_arenas_(0u) {
+      compactable_arenas_(0u),
+      last_fixup_count_for_testing_(0) {
   // The heap compaction implementation assumes the contiguous range,
   //
   //   [Vector1ArenaIndex, HashTableArenaIndex]
@@ -489,9 +490,12 @@ void HeapCompact::StartThreadCompaction() {
     return;
 
   // The mapping between the slots and the backing stores are created
+  last_fixup_count_for_testing_ = 0;
   for (auto** slot : traced_slots_) {
-    if (*slot)
+    if (*slot) {
       Fixups().Add(slot);
+      last_fixup_count_for_testing_++;
+    }
   }
   traced_slots_.clear();
 }

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

@@ -137,6 +137,10 @@ class PLATFORM_EXPORT HeapCompact final {
     return false;
   }
 
+  size_t last_fixup_count_for_testing() {
+    return last_fixup_count_for_testing_;
+  }
+
  private:
   class MovableObjectFixups;
 
@@ -180,6 +184,8 @@ class PLATFORM_EXPORT HeapCompact final {
   // created at the atomic pause phase.
   HashSet<MovableReference*> traced_slots_;
 
+  size_t last_fixup_count_for_testing_;
+
   static bool force_compaction_gc_;
 };
 

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

@@ -1637,9 +1637,9 @@ class IncrementalMarkingTestDriver {
     thread_state_->CompleteSweep();
   }
 
-  HashSet<MovableReference*>& GetTracedSlot() {
+  size_t GetHeapCompactLastFixupCount() {
     HeapCompact* compaction = ThreadState::Current()->Heap().Compaction();
-    return compaction->traced_slots_;
+    return compaction->last_fixup_count_for_testing();
   }
 
  private:
@@ -1754,12 +1754,32 @@ TEST(IncrementalMarkingTest, HasInlineCapacityCollectionWithHeapCompaction) {
   HeapCompact::ScheduleCompactionGCForTesting(true);
   persistent->push_back(Object::Create());
   driver.Start();
-  driver.FinishSteps();
+  driver.FinishGC();
 
   // Should collect also slots that has only inline buffer and nullptr
   // references.
-  EXPECT_EQ(driver.GetTracedSlot().size(), 2u);
+#if defined(ANNOTATE_CONTIGUOUS_CONTAINER)
+  // When ANNOTATE_CONTIGUOUS_CONTAINER is defined, inline capacity is ignored.
+  EXPECT_EQ(driver.GetHeapCompactLastFixupCount(), 1u);
+#else
+  EXPECT_EQ(driver.GetHeapCompactLastFixupCount(), 2u);
+#endif
+}
+
+TEST(IncrementalMarkingTest, WeakHashMapHeapCompaction) {
+  using Store = HeapHashCountedSet<WeakMember<Object>>;
+
+  Persistent<Store> persistent(new Store());
+
+  IncrementalMarkingTestDriver driver(ThreadState::Current());
+  HeapCompact::ScheduleCompactionGCForTesting(true);
+  driver.Start();
+  driver.FinishSteps();
+  persistent->insert(Object::Create());
   driver.FinishGC();
+
+  // Weak caallback should register the slot.
+  EXPECT_EQ(driver.GetHeapCompactLastFixupCount(), 1u);
 }
 
 }  // namespace incremental_marking_test

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

@@ -135,6 +135,9 @@ class PLATFORM_EXPORT MarkingVisitor : public Visitor {
                                TraceDescriptor desc,
                                WeakCallback callback,
                                void* parameter) final {
+    RegisterBackingStoreReference(object_slot);
+    if (!object)
+      return;
     RegisterWeakCallback(parameter, callback);
   }
 

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

@@ -134,8 +134,6 @@ class PLATFORM_EXPORT Visitor {
     static_assert(IsGarbageCollectedType<T>::value,
                   "T needs to be a garbage collected object");
 
-    if (!backing_store)
-      return;
     VisitBackingStoreWeakly(reinterpret_cast<void*>(backing_store),
                             reinterpret_cast<void**>(backing_store_slot),
                             TraceTrait<T>::GetTraceDescriptor(