[PE] Allow duplicated id for uncached display items in PaintController

For now we don't have any bug about this. I found the issue when I was
experimenting some code change, e.g. using uncached display item client
for some display items.

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

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

@@ -255,6 +255,7 @@ class PLATFORM_EXPORT DisplayItem {
   bool IsScrollHitTest() const { return type_ == kScrollHitTest; }
 
   bool IsCacheable() const { return is_cacheable_; }
+  void SetUncacheable() { is_cacheable_ = false; }
 
   virtual bool Equals(const DisplayItem& other) const {
     // Failure of this DCHECK would cause bad casts in subclasses.

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

@@ -200,17 +200,13 @@ void PaintController::EndSubsequence(const DisplayItemClient& client,
   new_cached_subsequences_.insert(&client, SubsequenceMarkers(start, end));
 }
 
-const DisplayItem* PaintController::LastDisplayItem(unsigned offset) {
-  if (offset < new_display_item_list_.size())
-    return &new_display_item_list_[new_display_item_list_.size() - offset - 1];
-  return nullptr;
-}
-
 void PaintController::ProcessNewItem(DisplayItem& display_item) {
   DCHECK(!construction_disabled_);
 
-  if (IsSkippingCache() && usage_ == kMultiplePaints)
+  if (IsSkippingCache() && usage_ == kMultiplePaints) {
     display_item.Client().Invalidate(PaintInvalidationReason::kUncacheable);
+    display_item.SetUncacheable();
+  }
 
   bool chunk_added = new_paint_chunks_.IncrementDisplayItemIndex(display_item);
 
@@ -221,7 +217,7 @@ void PaintController::ProcessNewItem(DisplayItem& display_item) {
                             new_paint_chunk_indices_by_client_);
   }
 
-  if (usage_ == kMultiplePaints && !IsSkippingCache()) {
+  if (usage_ == kMultiplePaints && display_item.IsCacheable()) {
     size_t index = FindMatchingItemFromIndex(
         display_item.GetId(), new_display_item_indices_by_client_,
         new_display_item_list_);

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

@@ -144,8 +144,6 @@ class PLATFORM_EXPORT PaintController {
   // BeginSubsequence().
   void EndSubsequence(const DisplayItemClient&, size_t start);
 
-  const DisplayItem* LastDisplayItem(unsigned offset);
-
   void BeginSkippingCache() {
     if (usage_ == kTransient)
       return;

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

@@ -1567,6 +1567,45 @@ TEST_P(PaintControllerTest, TransientPaintControllerIncompleteCycle) {
   paint_controller = nullptr;
 }
 
+TEST_P(PaintControllerTest, AllowDuplicatedIdForUncacheableItem) {
+  if (RuntimeEnabledFeatures::PaintUnderInvalidationCheckingEnabled())
+    return;
+
+  LayoutRect r(100, 100, 300, 300);
+  FakeDisplayItemClient cacheable("cacheable", r);
+  FakeDisplayItemClient uncacheable("uncacheable", r);
+  GraphicsContext context(GetPaintController());
+
+  uncacheable.Invalidate(PaintInvalidationReason::kUncacheable);
+  EXPECT_TRUE(cacheable.IsCacheable());
+  EXPECT_FALSE(uncacheable.IsCacheable());
+
+  InitRootChunk();
+  {
+    SubsequenceRecorder recorder(context, cacheable);
+    DrawRect(context, cacheable, kBackgroundType, FloatRect(r));
+    DrawRect(context, uncacheable, kBackgroundType, FloatRect(r));
+    // This should not trigger the duplicated id assert.
+    DrawRect(context, uncacheable, kBackgroundType, FloatRect(r));
+  }
+
+  CommitAndFinishCycle();
+  EXPECT_TRUE(GetPaintController().GetDisplayItemList()[0].IsCacheable());
+  EXPECT_FALSE(GetPaintController().GetDisplayItemList()[1].IsCacheable());
+  EXPECT_FALSE(GetPaintController().GetDisplayItemList()[2].IsCacheable());
+  EXPECT_TRUE(cacheable.IsCacheable());
+  EXPECT_FALSE(uncacheable.IsCacheable());
+
+  InitRootChunk();
+  EXPECT_TRUE(GetPaintController().UseCachedSubsequenceIfPossible(cacheable));
+  CommitAndFinishCycle();
+  EXPECT_TRUE(GetPaintController().GetDisplayItemList()[0].IsCacheable());
+  EXPECT_FALSE(GetPaintController().GetDisplayItemList()[1].IsCacheable());
+  EXPECT_FALSE(GetPaintController().GetDisplayItemList()[2].IsCacheable());
+  EXPECT_TRUE(cacheable.IsCacheable());
+  EXPECT_FALSE(uncacheable.IsCacheable());
+}
+
 // Death tests don't work properly on Android.
 #if defined(GTEST_HAS_DEATH_TEST) && !defined(OS_ANDROID)