Remove unused ContiguousContainer methods and cleanups

- Remove unused ContiguousContainer methods such as Clear(), Swap(),
  etc.
- Replace RemoveLast() with ReplaceLastByMoving(). This removes the
  logic to deal with removed items, making ContiguousContainer
  appending only.
- Let ContiguousContainer store Buffer directly in the vector instead of
  through unique_ptr.
- Use 'item' instead of 3 different terms 'item', 'element', 'object' in
  ContiguousContainer
- Remove DisplayItem::IsMovedFromCachedSubsequence() because with the
  new PaintChunkSubset we can use
  PaintChunk.is_moved_from_cached_subsequence.
- etc.

Bug: 917911
Change-Id: I8b3004d001ea12a00fed96fc3e031986db1ee005
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2501064Reviewed-by: Stefan Zager <szager@chromium.org>
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#822912}

third_party/blink/renderer/platform/graphics/contiguous_container.cc

@@ -7,167 +7,53 @@
 #include <algorithm>
 #include <memory>
 
-#include "base/macros.h"
-#include "third_party/blink/renderer/platform/wtf/allocator/allocator.h"
-#include "third_party/blink/renderer/platform/wtf/allocator/partitions.h"
-#include "third_party/blink/renderer/platform/wtf/container_annotations.h"
-
 namespace blink {
 
-class ContiguousContainerBase::Buffer {
-  USING_FAST_MALLOC(Buffer);
-
- public:
-  Buffer(wtf_size_t buffer_size, const char* type_name) {
-    capacity_ = WTF::Partitions::BufferActualSize(buffer_size);
-    begin_ = end_ =
-        static_cast<char*>(WTF::Partitions::BufferMalloc(capacity_, type_name));
-    ANNOTATE_NEW_BUFFER(begin_, capacity_, 0);
-  }
-
-  ~Buffer() {
-    ANNOTATE_DELETE_BUFFER(begin_, capacity_, UsedCapacity());
-    WTF::Partitions::BufferFree(begin_);
-  }
-
-  wtf_size_t Capacity() const { return capacity_; }
-  wtf_size_t UsedCapacity() const { return end_ - begin_; }
-  wtf_size_t UnusedCapacity() const { return Capacity() - UsedCapacity(); }
-  bool IsEmpty() const { return UsedCapacity() == 0; }
-
-  void* Allocate(wtf_size_t object_size) {
-    DCHECK_GE(UnusedCapacity(), object_size);
-    ANNOTATE_CHANGE_SIZE(begin_, capacity_, UsedCapacity(),
-                         UsedCapacity() + object_size);
-    void* result = end_;
-    end_ += object_size;
-    return result;
-  }
-
-  void DeallocateLastObject(void* object) {
-    CHECK_LE(begin_, object);
-    CHECK_LT(object, end_);
-    ANNOTATE_CHANGE_SIZE(begin_, capacity_, UsedCapacity(),
-                         static_cast<char*>(object) - begin_);
-    end_ = static_cast<char*>(object);
-  }
-
- private:
-  // begin_ <= end_ <= begin_ + capacity_
-  char* begin_;
-  char* end_;
-  wtf_size_t capacity_;
-
-  DISALLOW_COPY_AND_ASSIGN(Buffer);
-};
-
 ContiguousContainerBase::ContiguousContainerBase(
-    wtf_size_t max_object_size,
+    wtf_size_t max_item_size,
     wtf_size_t initial_capacity_in_bytes)
-    : end_index_(0),
-      max_object_size_(max_object_size),
+    : max_item_size_(max_item_size),
       initial_capacity_in_bytes_(
-          std::max(max_object_size, initial_capacity_in_bytes)) {}
-
-ContiguousContainerBase::ContiguousContainerBase(
-    ContiguousContainerBase&& source)
-    : ContiguousContainerBase(source.max_object_size_,
-                              source.initial_capacity_in_bytes_) {
-  Swap(source);
-}
+          std::max(max_item_size, initial_capacity_in_bytes)) {}
 
 ContiguousContainerBase::~ContiguousContainerBase() = default;
 
-ContiguousContainerBase& ContiguousContainerBase::operator=(
-    ContiguousContainerBase&& source) {
-  Swap(source);
-  return *this;
-}
-
 wtf_size_t ContiguousContainerBase::CapacityInBytes() const {
   wtf_size_t capacity = 0;
   for (const auto& buffer : buffers_)
-    capacity += buffer->Capacity();
+    capacity += buffer.Capacity();
   return capacity;
 }
 
 wtf_size_t ContiguousContainerBase::UsedCapacityInBytes() const {
   wtf_size_t used_capacity = 0;
   for (const auto& buffer : buffers_)
-    used_capacity += buffer->UsedCapacity();
+    used_capacity += buffer.UsedCapacity();
   return used_capacity;
 }
 
 wtf_size_t ContiguousContainerBase::MemoryUsageInBytes() const {
-  return sizeof(*this) + CapacityInBytes() + elements_.CapacityInBytes();
+  return sizeof(*this) + CapacityInBytes() + items_.CapacityInBytes();
 }
 
-void* ContiguousContainerBase::Allocate(wtf_size_t object_size,
+uint8_t* ContiguousContainerBase::Allocate(wtf_size_t item_size,
                                            const char* type_name) {
-  DCHECK_LE(object_size, max_object_size_);
+  DCHECK_LE(item_size, max_item_size_);
 
   Buffer* buffer_for_alloc = nullptr;
-  if (!buffers_.IsEmpty()) {
-    Buffer* end_buffer = buffers_[end_index_].get();
-    if (end_buffer->UnusedCapacity() >= object_size)
-      buffer_for_alloc = end_buffer;
-    else if (end_index_ + 1 < buffers_.size())
-      buffer_for_alloc = buffers_[++end_index_].get();
-  }
+  if (!buffers_.IsEmpty() && buffers_.back().UnusedCapacity() >= item_size)
+    buffer_for_alloc = &buffers_.back();
 
   if (!buffer_for_alloc) {
     wtf_size_t new_buffer_size = buffers_.IsEmpty()
                                      ? initial_capacity_in_bytes_
-                                     : 2 * buffers_.back()->Capacity();
-    buffer_for_alloc =
-        AllocateNewBufferForNextAllocation(new_buffer_size, type_name);
+                                     : 2 * buffers_.back().Capacity();
+    buffer_for_alloc = &buffers_.emplace_back(new_buffer_size, type_name);
   }
 
-  void* element = buffer_for_alloc->Allocate(object_size);
-  elements_.push_back(element);
-  return element;
-}
-
-void ContiguousContainerBase::RemoveLast() {
-  void* object = elements_.back();
-  elements_.pop_back();
-
-  Buffer* end_buffer = buffers_[end_index_].get();
-  end_buffer->DeallocateLastObject(object);
-
-  if (end_buffer->IsEmpty()) {
-    if (end_index_ > 0)
-      end_index_--;
-    if (end_index_ + 2 < buffers_.size())
-      buffers_.pop_back();
-  }
-}
-
-void ContiguousContainerBase::Clear() {
-  elements_.clear();
-  buffers_.clear();
-  end_index_ = 0;
-}
-
-void ContiguousContainerBase::Swap(ContiguousContainerBase& other) {
-  elements_.swap(other.elements_);
-  buffers_.swap(other.buffers_);
-  std::swap(end_index_, other.end_index_);
-  std::swap(max_object_size_, other.max_object_size_);
-  std::swap(initial_capacity_in_bytes_, other.initial_capacity_in_bytes_);
-}
-
-ContiguousContainerBase::Buffer*
-ContiguousContainerBase::AllocateNewBufferForNextAllocation(
-    wtf_size_t buffer_size,
-    const char* type_name) {
-  DCHECK(buffers_.IsEmpty() || end_index_ == buffers_.size() - 1);
-  std::unique_ptr<Buffer> new_buffer =
-      std::make_unique<Buffer>(buffer_size, type_name);
-  Buffer* buffer_to_return = new_buffer.get();
-  buffers_.push_back(std::move(new_buffer));
-  end_index_ = buffers_.size() - 1;
-  return buffer_to_return;
+  uint8_t* item = buffer_for_alloc->Allocate(item_size);
+  items_.push_back(item);
+  return item;
 }
 
 }  // namespace blink

third_party/blink/renderer/platform/graphics/paint/display_item.h

@@ -168,8 +168,7 @@ class PLATFORM_EXPORT DisplayItem {
             static_cast<unsigned>(client.VisualRectOutsetForRasterEffects())),
         draws_content_(draws_content),
         is_cacheable_(client.IsCacheable()),
-        is_tombstone_(false),
-        is_moved_from_cached_subsequence_(false) {
+        is_tombstone_(false) {
     // |derived_size| must fit in |derived_size_|.
     // If it doesn't, enlarge |derived_size_| and fix this assert.
     SECURITY_DCHECK(derived_size == derived_size_);
@@ -258,13 +257,6 @@ class PLATFORM_EXPORT DisplayItem {
   bool IsCacheable() const { return is_cacheable_; }
   void SetUncacheable() { is_cacheable_ = false; }
 
-  bool IsMovedFromCachedSubsequence() const {
-    return is_moved_from_cached_subsequence_;
-  }
-  void SetMovedFromCachedSubsequence(bool b) {
-    is_moved_from_cached_subsequence_ = b;
-  }
-
   virtual bool Equals(const DisplayItem& other) const {
     // Failure of this DCHECK would cause bad casts in subclasses.
     SECURITY_CHECK(!is_tombstone_);
@@ -309,7 +301,6 @@ class PLATFORM_EXPORT DisplayItem {
   unsigned draws_content_ : 1;
   unsigned is_cacheable_ : 1;
   unsigned is_tombstone_ : 1;
-  unsigned is_moved_from_cached_subsequence_ : 1;
 };
 
 inline bool operator==(const DisplayItem::Id& a, const DisplayItem::Id& b) {

third_party/blink/renderer/platform/graphics/paint/display_item_list.h

@@ -37,36 +37,20 @@ class PLATFORM_EXPORT DisplayItemList
                                 ? initial_capacity_in_bytes
                                 : kDefaultCapacityInBytes) {}
 
-  DisplayItemList(DisplayItemList&& source)
-      : ContiguousContainer(std::move(source)) {}
-
-  DisplayItemList& operator=(DisplayItemList&& source) {
-    ContiguousContainer::operator=(std::move(source));
-    return *this;
-  }
-
   DisplayItem& AppendByMoving(DisplayItem& item) {
     SECURITY_CHECK(!item.IsTombstone());
     DisplayItem& result =
         ContiguousContainer::AppendByMoving(item, item.DerivedSize());
-    // ContiguousContainer::AppendByMoving() calls an in-place constructor
-    // on item which replaces it with a tombstone/"dead display item" that
-    // can be safely destructed but should never be used except for debugging
-    // and raster invalidation (see below).
-    DCHECK(item.IsTombstone());
-    // We need |visual_rect_| and |outset_for_raster_effects_| of the old
-    // display item for raster invalidation. Also, the fields that make up the
-    // ID (|client_|, |type_| and |fragment_|) need to match. As their values
-    // were either initialized to default values or were left uninitialized by
-    // DisplayItem's default constructor, now copy their original values back
-    // from |result|.
-    item.client_ = result.client_;
-    item.type_ = result.type_;
-    item.fragment_ = result.fragment_;
-    DCHECK(item.GetId() == result.GetId());
-    item.visual_rect_ = result.visual_rect_;
-    item.raster_effect_outset_ = result.raster_effect_outset_;
-    result.SetMovedFromCachedSubsequence(false);
+    SetupTombstone(item, result);
+    return result;
+  }
+
+  DisplayItem& ReplaceLastByMoving(DisplayItem& item) {
+    SECURITY_CHECK(!item.IsTombstone());
+    DCHECK_EQ(back().DerivedSize(), item.DerivedSize());
+    DisplayItem& result =
+        ContiguousContainer::ReplaceLastByMoving(item, item.DerivedSize());
+    SetupTombstone(item, result);
     return result;
   }
 
@@ -116,6 +100,26 @@ class PLATFORM_EXPORT DisplayItemList
       const Range<const_iterator>& display_items,
       JsonFlags);
 #endif  // DCHECK_IS_ON()
+
+ private:
+  // Called by AppendByMoving() and ReplaceLastByMoving() which created a
+  // tombstone/"dead display item" that can be safely destructed but should
+  // never be used except for debugging and raster invalidation.
+  void SetupTombstone(DisplayItem& item, const DisplayItem& new_item) {
+    DCHECK(item.IsTombstone());
+    // We need |visual_rect_| and |outset_for_raster_effects_| of the old
+    // display item for raster invalidation. Also, the fields that make up the
+    // ID (|client_|, |type_| and |fragment_|) need to match. As their values
+    // were either initialized to default values or were left uninitialized by
+    // DisplayItem's default constructor, now copy their original values back
+    // from |result|.
+    item.client_ = new_item.client_;
+    item.type_ = new_item.type_;
+    item.fragment_ = new_item.fragment_;
+    DCHECK_EQ(item.GetId(), new_item.GetId());
+    item.visual_rect_ = new_item.visual_rect_;
+    item.raster_effect_outset_ = new_item.raster_effect_outset_;
+  }
 };
 
 using DisplayItemIterator = DisplayItemList::const_iterator;

third_party/blink/renderer/platform/graphics/paint/paint_controller.cc

@@ -43,7 +43,7 @@ PaintController::~PaintController() {
 
 void PaintController::EnsureChunk() {
   if (paint_chunker_.EnsureChunk())
-    DidAppendChunk();
+    CheckNewChunk();
 }
 
 void PaintController::RecordHitTestData(const DisplayItemClient& client,
@@ -65,7 +65,7 @@ void PaintController::RecordHitTestData(const DisplayItemClient& client,
   CheckDuplicatePaintChunkId(id);
   if (paint_chunker_.AddHitTestDataToCurrentChunk(id, rect, touch_action,
                                                   blocking_wheel))
-    DidAppendChunk();
+    CheckNewChunk();
 }
 
 void PaintController::RecordScrollHitTestData(
@@ -76,7 +76,7 @@ void PaintController::RecordScrollHitTestData(
   PaintChunk::Id id(client, type, current_fragment_);
   CheckDuplicatePaintChunkId(id);
   paint_chunker_.CreateScrollHitTestChunk(id, scroll_translation, rect);
-  DidAppendChunk();
+  CheckNewChunk();
 }
 
 void PaintController::SetPossibleBackgroundColor(
@@ -87,7 +87,7 @@ void PaintController::SetPossibleBackgroundColor(
                     current_fragment_);
   CheckDuplicatePaintChunkId(id);
   if (paint_chunker_.ProcessBackgroundColorCandidate(id, color, area))
-    DidAppendChunk();
+    CheckNewChunk();
 }
 
 bool PaintController::UseCachedItemIfPossible(const DisplayItemClient& client,
@@ -280,7 +280,7 @@ void PaintController::EndSubsequence(const DisplayItemClient& client,
       &client, SubsequenceMarkers{start_chunk_index, end_chunk_index});
 }
 
-void PaintController::DidAppendItem(DisplayItem& display_item) {
+void PaintController::CheckNewItem(DisplayItem& display_item) {
   if (usage_ == kTransient)
     return;
 
@@ -313,7 +313,7 @@ void PaintController::ProcessNewItem(DisplayItem& display_item) {
   }
 
   if (paint_chunker_.IncrementDisplayItemIndex(display_item))
-    DidAppendChunk();
+    CheckNewChunk();
 
   if (!frame_first_paints_.back().first_painted && display_item.IsDrawing() &&
       // Here we ignore all document-background paintings because we don't
@@ -325,7 +325,7 @@ void PaintController::ProcessNewItem(DisplayItem& display_item) {
     SetFirstPainted();
   }
 
-  DidAppendItem(display_item);
+  CheckNewItem(display_item);
 }
 
 DisplayItem& PaintController::MoveItemFromCurrentListToNewList(
@@ -334,7 +334,7 @@ DisplayItem& PaintController::MoveItemFromCurrentListToNewList(
       current_paint_artifact_->GetDisplayItemList()[index]);
 }
 
-void PaintController::DidAppendChunk() {
+void PaintController::CheckNewChunk() {
 #if DCHECK_IS_ON()
   auto& chunks = new_paint_artifact_->PaintChunks();
   if (chunks.back().is_cacheable) {
@@ -374,7 +374,7 @@ void PaintController::UpdateCurrentPaintChunkProperties(
 void PaintController::AppendChunkByMoving(PaintChunk&& chunk) {
   CheckDuplicatePaintChunkId(chunk.id);
   paint_chunker_.AppendByMoving(std::move(chunk));
-  DidAppendChunk();
+  CheckNewChunk();
 }
 
 bool PaintController::ClientCacheIsValid(
@@ -504,9 +504,7 @@ void PaintController::CopyCachedSubsequence(wtf_size_t start_chunk_index,
       SECURITY_CHECK(!cached_item.IsTombstone());
       DCHECK(!cached_item.IsCacheable() ||
              ClientCacheIsValid(cached_item.Client()));
-      auto& item = MoveItemFromCurrentListToNewList(cached_item_index++);
-      item.SetMovedFromCachedSubsequence(true);
-      DidAppendItem(item);
+      CheckNewItem(MoveItemFromCurrentListToNewList(cached_item_index++));
     }
 
     DCHECK_EQ(cached_item_index, cached_chunk.end_index);
@@ -593,20 +591,20 @@ void PaintController::FinishCycle() {
     chunk.client_is_just_created = false;
     const auto& client = chunk.id.client;
     if (chunk.is_moved_from_cached_subsequence) {
-      DCHECK(!chunk.is_cacheable || ClientCacheIsValid(client));
+      // We don't need to validate the clients of paint chunks and display
+      // items that are moved from a cached subsequence, because they should be
+      // already valid. See http://crbug.com/1050090 for more details.
+#if DCHECK_IS_ON()
+      DCHECK(ClientCacheIsValid(client));
+      for (const auto& item : current_paint_artifact_->DisplayItemsInChunk(i))
+        DCHECK(!item.IsCacheable() || ClientCacheIsValid(item.Client()));
+#endif
       continue;
     }
     if (client.IsCacheable())
       client.Validate();
 
     for (const auto& item : current_paint_artifact_->DisplayItemsInChunk(i)) {
-      if (item.IsMovedFromCachedSubsequence()) {
-        // We don't need to validate the clients of a display item that is
-        // copied from a cached subsequence, because it should be already
-        // valid. See http://crbug.com/1050090 for more details.
-        DCHECK(!item.IsCacheable() || ClientCacheIsValid(item.Client()));
-        continue;
-      }
       item.Client().ClearPartialInvalidationVisualRect();
       if (item.Client().IsCacheable())
         item.Client().Validate();
@@ -716,7 +714,7 @@ void PaintController::CheckUnderInvalidation() {
     return;
   }
 
-  DisplayItem& new_item = new_paint_artifact_->GetDisplayItemList().Last();
+  DisplayItem& new_item = new_paint_artifact_->GetDisplayItemList().back();
   auto old_item_index = under_invalidation_checking_begin_;
   DisplayItem* old_item =
       old_item_index < current_paint_artifact_->GetDisplayItemList().size()
@@ -736,8 +734,8 @@ void PaintController::CheckUnderInvalidation() {
   // non-under-invalidation-checking path to empty the original cached slot,
   // leaving only disappeared or invalidated display items in the old list after
   // painting.
-  new_paint_artifact_->GetDisplayItemList().RemoveLast();
-  MoveItemFromCurrentListToNewList(old_item_index);
+  new_paint_artifact_->GetDisplayItemList().ReplaceLastByMoving(
+      current_paint_artifact_->GetDisplayItemList()[old_item_index]);
 
   ++under_invalidation_checking_begin_;
 }

third_party/blink/renderer/platform/graphics/paint/paint_controller.h

@@ -286,9 +286,9 @@ class PLATFORM_EXPORT PaintController {
   // Set new item state (cache skipping, etc) for the last new display item.
   void ProcessNewItem(DisplayItem&);
 
-  void DidAppendItem(DisplayItem&);
+  void CheckNewItem(DisplayItem&);
   DisplayItem& MoveItemFromCurrentListToNewList(wtf_size_t);
-  void DidAppendChunk();
+  void CheckNewChunk();
 
   struct IdAsHashKey {
     IdAsHashKey() = default;

third_party/blink/renderer/platform/testing/test_paint_artifact.cc

@@ -147,7 +147,7 @@ FakeDisplayItemClient& TestPaintArtifact::Client(wtf_size_t i) const {
 void TestPaintArtifact::DidAddDisplayItem() {
   auto& chunk = paint_artifact_->PaintChunks().back();
   DCHECK_EQ(chunk.end_index, paint_artifact_->GetDisplayItemList().size() - 1);
-  const auto& item = paint_artifact_->GetDisplayItemList().Last();
+  const auto& item = paint_artifact_->GetDisplayItemList().back();
   chunk.bounds.Unite(item.VisualRect());
   if (item.DrawsContent())
     chunk.drawable_bounds.Unite(item.VisualRect());