Use chunk indices for subsequence markers in PaintController

This prepares for allowing paint chunks that don't contain any display
items. Previously we used display item indices which couldn't accurately
indicate whether an empty paint chunk belonged to a subsequence.

This avoids the lookup of a paint chunk from display item index.

Also adds under-invalidation checking for paint chunks in cached
subsequences.

Will introduce empty paint chunks in crrev.com/c/937573.

Change-Id: I7c18a0f6b176b535c04414db28a244491e3023af
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2076411
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#745108}

third_party/blink/renderer/core/paint/paint_controller_paint_test.h

@@ -144,17 +144,6 @@ const DisplayItem::Type kScrollingContentsBackgroundChunkType =
     DisplayItem::PaintPhaseToClipType(
         PaintPhase::kDescendantBlockBackgroundsOnly);
 
-#define EXPECT_SUBSEQUENCE(client, expected_start, expected_end)        \
-  do {                                                                  \
-    auto* subsequence = GetSubsequenceMarkers(client);                  \
-    ASSERT_NE(nullptr, subsequence);                                    \
-    EXPECT_EQ(static_cast<size_t>(expected_start), subsequence->start); \
-    EXPECT_EQ(static_cast<size_t>(expected_end), subsequence->end);     \
-  } while (false)
-
-#define EXPECT_NO_SUBSEQUENCE(client) \
-  EXPECT_EQ(nullptr, GetSubsequenceMarkers(client)
-
 }  // namespace blink
 
 #endif  // THIRD_PARTY_BLINK_RENDERER_CORE_PAINT_PAINT_CONTROLLER_PAINT_TEST_H_

third_party/blink/renderer/core/paint/paint_layer_painter_test.cc

@@ -493,7 +493,8 @@ TEST_P(PaintLayerPainterTest, CachedSubsequenceRetainsPreviousPaintResult) {
                     IsSameId(content1, kBackgroundType),
                     IsSameId(content2, kBackgroundType)));
     // |target| still created subsequence (repainted).
-    EXPECT_SUBSEQUENCE(*target_layer, 2, 4);
+    EXPECT_SUBSEQUENCE(*target_layer, 2, 3);
+    EXPECT_EQ(2u, RootPaintController().PaintChunks()[2].size());
   } else {
     EXPECT_EQ(CullRect(IntRect(0, 0, 800, 7600)),
               target_layer->PreviousCullRect());
@@ -503,7 +504,8 @@ TEST_P(PaintLayerPainterTest, CachedSubsequenceRetainsPreviousPaintResult) {
                             IsSameId(content1, kBackgroundType),
                             IsSameId(content2, kBackgroundType)));
     // |target| still created subsequence (repainted).
-    EXPECT_SUBSEQUENCE(*target_layer, 1, 3);
+    EXPECT_SUBSEQUENCE(*target_layer, 1, 2);
+    EXPECT_EQ(2u, RootPaintController().PaintChunks()[1].size());
   }
 }
 

third_party/blink/renderer/platform/graphics/compositing/content_layer_client_impl.cc

@@ -73,7 +73,7 @@ scoped_refptr<cc::PictureLayer> ContentLayerClientImpl::UpdateCcPictureLayer(
     auto json = std::make_unique<JSONObject>();
     json->SetString("data", chunk.ToString());
     json->SetArray("displayItems",
-                   paint_artifact->GetDisplayItemList().SubsequenceAsJSON(
+                   paint_artifact->GetDisplayItemList().DisplayItemsAsJSON(
                        chunk.begin_index, chunk.end_index,
                        DisplayItemList::kShowOnlyDisplayItemTypes));
     paint_chunk_debug_data_->PushObject(std::move(json));

third_party/blink/renderer/platform/graphics/compositing/paint_artifact_compositor.cc

@@ -296,7 +296,6 @@ PaintArtifactCompositor::CompositedLayerForPendingLayer(
       paint_artifact->GetPaintChunkSubset(pending_layer.paint_chunk_indices);
   DCHECK(paint_chunks.size());
   const PaintChunk& first_paint_chunk = paint_chunks[0];
-  DCHECK(first_paint_chunk.size());
 
   // If the paint chunk is a foreign layer, just return that layer.
   if (scoped_refptr<cc::Layer> foreign_layer = ForeignLayerForPaintChunk(
@@ -844,11 +843,14 @@ void PaintArtifactCompositor::LayerizeGroup(
     const auto& chunk_effect = chunk_it->properties.Effect().Unalias();
     if (&chunk_effect == &unaliased_group) {
       // Case A: The next chunk belongs to the current group but no subgroup.
-      const auto& first_display_item =
-          paint_artifact.GetDisplayItemList()[chunk_it->begin_index];
-      bool requires_own_layer = first_display_item.IsForeignLayer() ||
-                                IsCompositedScrollHitTest(first_display_item) ||
-                                IsCompositedScrollbar(first_display_item);
+      bool requires_own_layer = false;
+      if (chunk_it->size()) {
+        const auto& first_display_item =
+            paint_artifact.GetDisplayItemList()[chunk_it->begin_index];
+        requires_own_layer = first_display_item.IsForeignLayer() ||
+                             IsCompositedScrollHitTest(first_display_item) ||
+                             IsCompositedScrollbar(first_display_item);
+      }
       DCHECK(!requires_own_layer || chunk_it->size() == 1u);
 
       pending_layers_.emplace_back(

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

@@ -24,7 +24,7 @@ DisplayItemList::ItemsInPaintChunk(const PaintChunk& paint_chunk) const {
 
 #if DCHECK_IS_ON()
 
-std::unique_ptr<JSONArray> DisplayItemList::SubsequenceAsJSON(
+std::unique_ptr<JSONArray> DisplayItemList::DisplayItemsAsJSON(
     wtf_size_t begin_index,
     wtf_size_t end_index,
     JsonFlags flags) const {

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

@@ -91,9 +91,9 @@ class PLATFORM_EXPORT DisplayItemList
   };
   typedef unsigned JsonFlags;
 
-  std::unique_ptr<JSONArray> SubsequenceAsJSON(wtf_size_t begin_index,
-                                               wtf_size_t end_index,
-                                               JsonFlags) const;
+  std::unique_ptr<JSONArray> DisplayItemsAsJSON(wtf_size_t begin_index,
+                                                wtf_size_t end_index,
+                                                JsonFlags) const;
   void AppendSubsequenceAsJSON(wtf_size_t begin_index,
                                wtf_size_t end_index,
                                JsonFlags,

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

@@ -44,15 +44,12 @@ static SkColor DisplayItemBackgroundColor(const DisplayItem& item) {
 
 void ComputeChunkDerivedData(const DisplayItemList& display_items,
                              PaintChunk& chunk) {
+  if (!chunk.size())
+    return;
+
   SkRegion known_to_be_opaque_region;
   auto items = display_items.ItemsInPaintChunk(chunk);
   for (const DisplayItem& item : items) {
-    chunk.bounds.Unite(item.VisualRect());
-    if (item.DrawsContent())
-      chunk.drawable_bounds.Unite(item.VisualRect());
-    chunk.outset_for_raster_effects = std::max(chunk.outset_for_raster_effects,
-                                               item.OutsetForRasterEffects());
-
     if (RuntimeEnabledFeatures::CompositeAfterPaintEnabled() &&
         item.IsDrawing()) {
       const auto& drawing = static_cast<const DrawingDisplayItem&>(item);
@@ -110,15 +107,9 @@ PaintArtifact::PaintArtifact(DisplayItemList display_items,
   for (auto& chunk : chunks_) {
     if (chunk.is_moved_from_cached_subsequence) {
 #if DCHECK_IS_ON()
-      auto old_bounds = chunk.bounds;
-      auto old_drawable_bounds = chunk.drawable_bounds;
-      auto old_outset_for_raster_effects = chunk.outset_for_raster_effects;
       auto old_hit_test_data = std::move(chunk.hit_test_data);
       auto old_known_to_be_opaque = chunk.known_to_be_opaque;
       ComputeChunkDerivedData(display_item_list_, chunk);
-      DCHECK_EQ(old_bounds, chunk.bounds);
-      DCHECK_EQ(old_drawable_bounds, chunk.drawable_bounds);
-      DCHECK_EQ(old_outset_for_raster_effects, chunk.outset_for_raster_effects);
       DCHECK((!old_hit_test_data && !chunk.hit_test_data) ||
              (old_hit_test_data && chunk.hit_test_data &&
               *old_hit_test_data == *chunk.hit_test_data));

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

@@ -61,14 +61,6 @@ class PLATFORM_EXPORT PaintArtifact final : public RefCounted<PaintArtifact> {
     return PaintChunkSubset(PaintChunks(), subset_indices);
   }
 
-  Vector<PaintChunk>::const_iterator FindChunkByDisplayItemIndex(
-      wtf_size_t index) const {
-    return FindChunkInVectorByDisplayItemIndex(PaintChunks(), index);
-  }
-  Vector<PaintChunk>::iterator FindChunkByDisplayItemIndex(wtf_size_t index) {
-    return FindChunkInVectorByDisplayItemIndex(PaintChunks(), index);
-  }
-
   // Returns the approximate memory usage, excluding memory likely to be
   // shared with the embedder after copying to cc::DisplayItemList.
   size_t ApproximateUnsharedMemoryUsage() const;

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

@@ -24,6 +24,14 @@ struct SameSizeAsPaintChunk {
 static_assert(sizeof(PaintChunk) == sizeof(SameSizeAsPaintChunk),
               "PaintChunk should stay small");
 
+bool PaintChunk::EqualsForUnderInvalidationChecking(
+    const PaintChunk& other) const {
+  return size() == other.size() && id == other.id &&
+         properties == other.properties && bounds == other.bounds &&
+         drawable_bounds == other.drawable_bounds &&
+         outset_for_raster_effects == other.outset_for_raster_effects;
+}
+
 size_t PaintChunk::MemoryUsageInBytes() const {
   size_t total_size = sizeof(*this);
   if (hit_test_data) {

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

@@ -37,9 +37,7 @@ struct PLATFORM_EXPORT PaintChunk {
         id(id),
         properties(props),
         is_cacheable(id.client.IsCacheable()),
-        client_is_just_created(id.client.IsJustCreated()) {
-    DCHECK_GT(end_index, begin_index);
-  }
+        client_is_just_created(id.client.IsJustCreated()) {}
 
   // Move a paint chunk from a cached subsequence.
   PaintChunk(wtf_size_t begin, PaintChunk&& other)
@@ -62,7 +60,7 @@ struct PLATFORM_EXPORT PaintChunk {
   }
 
   wtf_size_t size() const {
-    DCHECK_GT(end_index, begin_index);
+    DCHECK_GE(end_index, begin_index);
     return end_index - begin_index;
   }
 
@@ -85,6 +83,8 @@ struct PLATFORM_EXPORT PaintChunk {
     return !client_is_just_created;
   }
 
+  bool EqualsForUnderInvalidationChecking(const PaintChunk& other) const;
+
   size_t MemoryUsageInBytes() const;
 
   String ToString() const;
@@ -104,9 +104,12 @@ struct PLATFORM_EXPORT PaintChunk {
   // The paint properties which apply to this chunk.
   RefCountedPropertyTreeState properties;
 
-  // The following fields are not initialized when the chunk is created because
-  // they depend on the display items in this chunk. They are updated by the
-  // constructor of PaintArtifact.
+  // The following fields are not initialized when the chunk is created by
+  // PaintChunker because they depend on the display items in this chunk.
+  // They are updated when a display item is added into the chunk, or by the
+  // constructor of PaintArtifact, or when a paint chunk is moved from a cached
+  // subsequence.
+  // TODO(wangxianzhu): Reduce complexity of this.
 
   std::unique_ptr<HitTestData> hit_test_data;
 
@@ -135,27 +138,6 @@ struct PLATFORM_EXPORT PaintChunk {
   bool is_moved_from_cached_subsequence = false;
 };
 
-inline bool ChunkLessThanIndex(const PaintChunk& chunk, wtf_size_t index) {
-  return chunk.end_index <= index;
-}
-
-inline Vector<PaintChunk>::iterator FindChunkInVectorByDisplayItemIndex(
-    Vector<PaintChunk>& chunks,
-    wtf_size_t index) {
-  auto* chunk =
-      std::lower_bound(chunks.begin(), chunks.end(), index, ChunkLessThanIndex);
-  DCHECK(chunk == chunks.end() ||
-         (index >= chunk->begin_index && index < chunk->end_index));
-  return chunk;
-}
-
-inline Vector<PaintChunk>::const_iterator FindChunkInVectorByDisplayItemIndex(
-    const Vector<PaintChunk>& chunks,
-    wtf_size_t index) {
-  return FindChunkInVectorByDisplayItemIndex(
-      const_cast<Vector<PaintChunk>&>(chunks), index);
-}
-
 PLATFORM_EXPORT std::ostream& operator<<(std::ostream&, const PaintChunk&);
 
 }  // namespace blink

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

@@ -39,7 +39,7 @@ void PaintChunker::UpdateCurrentPaintChunkProperties(
 
 void PaintChunker::ForceNewChunk() {
   force_new_chunk_ = true;
-  // Always use a new chunk id for a force chunk which may be for a subsequence
+  // Always use a new chunk id for a forced chunk which may be for a subsequence
   // which needs the chunk id to be independence with previous chunks.
   next_chunk_id_ = base::nullopt;
 }
@@ -50,43 +50,42 @@ void PaintChunker::AppendByMoving(PaintChunk&& chunk) {
   chunks_.emplace_back(next_chunk_begin_index, std::move(chunk));
 }
 
-bool PaintChunker::IncrementDisplayItemIndex(const DisplayItem& item) {
+void PaintChunker::CreateNewChunk() {
 #if DCHECK_IS_ON()
   // If this DCHECKs are hit we are missing a call to update the properties.
   // See: ScopedPaintChunkProperties.
   DCHECK(!IsInInitialState());
   // At this point we should have all of the properties given to us.
   DCHECK(current_properties_.IsInitialized());
+  DCHECK(next_chunk_id_);
 #endif
+  wtf_size_t begin = chunks_.IsEmpty() ? 0 : LastChunk().end_index;
+  chunks_.emplace_back(begin, begin, *next_chunk_id_, current_properties_);
+  next_chunk_id_ = base::nullopt;
+}
 
+bool PaintChunker::IncrementDisplayItemIndex(const DisplayItem& item) {
   bool item_forces_new_chunk =
       item.IsForeignLayer() || item.IsScrollHitTest() || item.IsScrollbar();
-  if (item_forces_new_chunk) {
-    force_new_chunk_ = true;
-    next_chunk_id_.emplace(item.GetId());
-  }
-
-  wtf_size_t new_chunk_begin_index;
-  if (chunks_.IsEmpty()) {
-    new_chunk_begin_index = 0;
-  } else {
-    auto& last_chunk = LastChunk();
-    if (!force_new_chunk_ && current_properties_ == last_chunk.properties) {
-      // Continue the current chunk.
-      last_chunk.end_index++;
-      // We don't create a new chunk when UpdateCurrentPaintChunkProperties()
-      // just changed |next_chunk_id_| but not |current_properties_|. Clear
-      // |next_chunk_id_| which has been ignored.
-      next_chunk_id_ = base::nullopt;
-      return false;
-    }
-    new_chunk_begin_index = last_chunk.end_index;
+  if (item_forces_new_chunk)
+    ForceNewChunk();
+
+  if (!chunks_.IsEmpty() && !force_new_chunk_ &&
+      current_properties_ == LastChunk().properties) {
+    // Continue the last chunk as the current chunk.
+    AddItemToCurrentChunk(item);
+    // We don't create a new chunk when UpdateCurrentPaintChunkProperties()
+    // just changed |next_chunk_id_| but not |current_properties_|. Clear
+    // |next_chunk_id_| which has been ignored.
+    next_chunk_id_ = base::nullopt;
+    return false;
   }
 
-  chunks_.emplace_back(new_chunk_begin_index, new_chunk_begin_index + 1,
-                       next_chunk_id_ ? *next_chunk_id_ : item.GetId(),
-                       current_properties_);
-  next_chunk_id_ = base::nullopt;
+  // Use the first display item's id as the chunk id if it's not set explicitly.
+  if (!next_chunk_id_)
+    next_chunk_id_.emplace(item.GetId());
+  CreateNewChunk();
+  AddItemToCurrentChunk(item);
 
   // When forcing a new chunk, we still need to force new chunk for the next
   // display item. Otherwise reset force_new_chunk_ to false.
@@ -96,6 +95,16 @@ bool PaintChunker::IncrementDisplayItemIndex(const DisplayItem& item) {
   return true;
 }
 
+void PaintChunker::AddItemToCurrentChunk(const DisplayItem& item) {
+  auto& chunk = LastChunk();
+  chunk.bounds.Unite(item.VisualRect());
+  if (item.DrawsContent())
+    chunk.drawable_bounds.Unite(item.VisualRect());
+  chunk.outset_for_raster_effects =
+      std::max(chunk.outset_for_raster_effects, item.OutsetForRasterEffects());
+  chunk.end_index++;
+}
+
 Vector<PaintChunk> PaintChunker::ReleasePaintChunks() {
   next_chunk_id_ = base::nullopt;
   current_properties_ = PropertyTreeState::Uninitialized();

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

@@ -46,23 +46,17 @@ class PLATFORM_EXPORT PaintChunker final {
 
   const Vector<PaintChunk>& PaintChunks() const { return chunks_; }
 
-  PaintChunk& PaintChunkAt(wtf_size_t i) { return chunks_[i]; }
-  wtf_size_t LastChunkIndex() const {
-    return chunks_.IsEmpty() ? kNotFound : chunks_.size() - 1;
-  }
+  wtf_size_t size() const { return chunks_.size(); }
   PaintChunk& LastChunk() { return chunks_.back(); }
 
-  PaintChunk& FindChunkByDisplayItemIndex(wtf_size_t index) {
-    auto* chunk = FindChunkInVectorByDisplayItemIndex(chunks_, index);
-    DCHECK(chunk != chunks_.end());
-    return *chunk;
-  }
-
   // Releases the generated paint chunk list and raster invalidations and
   // resets the state of this object.
   Vector<PaintChunk> ReleasePaintChunks();
 
  private:
+  void CreateNewChunk();
+  void AddItemToCurrentChunk(const DisplayItem&);
+
   wtf_size_t ChunkIndex(const PaintChunk& chunk) const {
     auto index = static_cast<wtf_size_t>(&chunk - &chunks_.front());
     DCHECK_LT(index, chunks_.size());