[PE] Fix DCHECK failure when copying cached subsequence containing fragments

Previously CopyCachedSubsequence() called SetCurrentPaintChunkProperties()
which always reset id.fragment, causing multiple fragments to have the
same id. Now call SetCurrentPaintChunkPropertiesUsingIdWithFragment()
which doesn't reset fragment.

Bug: 823029
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ibf34f7bb5fba798f96a755ba89fddf848c5f8656
Reviewed-on: https://chromium-review.googlesource.com/969961
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544299}

third_party/WebKit/Source/platform/graphics/paint/PaintController.cpp

@@ -510,8 +510,8 @@ void PaintController::CopyCachedSubsequence(size_t begin_index,
     properties_before_subsequence =
         new_paint_chunks_.CurrentPaintChunkProperties();
     new_paint_chunks_.ForceNewChunk();
-    UpdateCurrentPaintChunkProperties(cached_chunk->id,
-                                      cached_chunk->properties);
+    UpdateCurrentPaintChunkPropertiesUsingIdWithFragment(
+        cached_chunk->id, cached_chunk->properties);
   } else {
     // Avoid uninitialized variable error on Windows.
     cached_chunk = current_paint_artifact_.PaintChunks().begin();
@@ -530,8 +530,8 @@ void PaintController::CopyCachedSubsequence(size_t begin_index,
       ++cached_chunk;
       DCHECK(cached_chunk != current_paint_artifact_.PaintChunks().end());
       new_paint_chunks_.ForceNewChunk();
-      UpdateCurrentPaintChunkProperties(cached_chunk->id,
-                                        cached_chunk->properties);
+      UpdateCurrentPaintChunkPropertiesUsingIdWithFragment(
+          cached_chunk->id, cached_chunk->properties);
     }
 
 #if DCHECK_IS_ON()

third_party/WebKit/Source/platform/graphics/paint/PaintController.h

@@ -79,8 +79,8 @@ class PLATFORM_EXPORT PaintController {
       const PaintChunkProperties& properties) {
     if (id) {
       PaintChunk::Id id_with_fragment(*id, current_fragment_);
-      new_paint_chunks_.UpdateCurrentPaintChunkProperties(id_with_fragment,
-                                                          properties);
+      UpdateCurrentPaintChunkPropertiesUsingIdWithFragment(id_with_fragment,
+                                                           properties);
 #if DCHECK_IS_ON()
       CheckDuplicatePaintChunkId(id_with_fragment);
 #endif
@@ -283,6 +283,13 @@ class PLATFORM_EXPORT PaintController {
   size_t FindOutOfOrderCachedItemForward(const DisplayItem::Id&);
   void CopyCachedSubsequence(size_t begin_index, size_t end_index);
 
+  void UpdateCurrentPaintChunkPropertiesUsingIdWithFragment(
+      const PaintChunk::Id& id_with_fragment,
+      const PaintChunkProperties& properties) {
+    new_paint_chunks_.UpdateCurrentPaintChunkProperties(id_with_fragment,
+                                                        properties);
+  }
+
   // Resets the indices (e.g. next_item_to_match_) of
   // current_paint_artifact_.GetDisplayItemList() to their initial values. This
   // should be called when the DisplayItemList in current_paint_artifact_ is

third_party/WebKit/Source/platform/graphics/paint/PaintControllerTest.cpp

@@ -12,6 +12,8 @@
 #include "platform/graphics/paint/DisplayItemCacheSkipper.h"
 #include "platform/graphics/paint/DrawingDisplayItem.h"
 #include "platform/graphics/paint/DrawingRecorder.h"
+#include "platform/graphics/paint/ScopedDisplayItemFragment.h"
+#include "platform/graphics/paint/ScopedPaintChunkProperties.h"
 #include "platform/graphics/paint/SubsequenceRecorder.h"
 #include "platform/runtime_enabled_features.h"
 #include "platform/testing/PaintTestConfigurations.h"
@@ -1145,6 +1147,79 @@ TEST_P(PaintControllerTest, CachedSubsequenceAndDisplayItemsSwapOrder) {
   EXPECT_EQ(4u, markers->end);
 }
 
+TEST_P(PaintControllerTest, CachedSubsequenceWithFragments) {
+  if (!RuntimeEnabledFeatures::SlimmingPaintV175Enabled())
+    return;
+
+  GraphicsContext context(GetPaintController());
+  FakeDisplayItemClient root("root");
+  constexpr size_t kFragmentCount = 3;
+  FakeDisplayItemClient container("container");
+
+  // The first paint.
+  auto paint_container = [this, &context, &container, kFragmentCount]() {
+    SubsequenceRecorder r(context, container);
+    for (size_t i = 0; i < kFragmentCount; ++i) {
+      ScopedDisplayItemFragment scoped_fragment(context, i);
+      ScopedPaintChunkProperties content_chunk_properties(
+          GetPaintController(), testing::DefaultPaintChunkProperties(),
+          container, kBackgroundType);
+      DrawRect(context, container, kBackgroundType,
+               FloatRect(100, 100, 100, 100));
+    }
+  };
+  {
+    ScopedPaintChunkProperties root_chunk_properties(
+        GetPaintController(), testing::DefaultPaintChunkProperties(), root,
+        kBackgroundType);
+    DrawRect(context, root, kBackgroundType, FloatRect(100, 100, 100, 100));
+    paint_container();
+    DrawRect(context, root, kForegroundType, FloatRect(100, 100, 100, 100));
+  }
+  GetPaintController().CommitNewDisplayItems();
+
+  // Check results of the first paint.
+  auto check_paint_results = [this, &root, &container, kFragmentCount]() {
+    const auto& chunks = GetPaintController().PaintChunks();
+    ASSERT_EQ(2u + kFragmentCount, chunks.size());
+    EXPECT_EQ(root, chunks[0].id.client);
+    EXPECT_EQ(kBackgroundType, chunks[0].id.type);
+    EXPECT_EQ(0u, chunks[0].id.fragment);
+    for (size_t i = 0; i < kFragmentCount; ++i) {
+      const auto& id = chunks[i + 1].id;
+      EXPECT_EQ(container, id.client);
+      EXPECT_EQ(kBackgroundType, id.type);
+      EXPECT_EQ(i, id.fragment);
+    }
+    EXPECT_EQ(root, chunks.back().id.client);
+    EXPECT_EQ(kForegroundType, chunks.back().id.type);
+    EXPECT_EQ(0u, chunks.back().id.fragment);
+  };
+  check_paint_results();
+
+  // The second paint.
+  {
+    ScopedPaintChunkProperties root_chunk_properties(
+        GetPaintController(), testing::DefaultPaintChunkProperties(), root,
+        kBackgroundType);
+    DrawRect(context, root, kBackgroundType, FloatRect(100, 100, 100, 100));
+
+    if (RuntimeEnabledFeatures::PaintUnderInvalidationCheckingEnabled()) {
+      EXPECT_FALSE(
+          GetPaintController().UseCachedSubsequenceIfPossible(container));
+      paint_container();
+    } else {
+      EXPECT_TRUE(
+          GetPaintController().UseCachedSubsequenceIfPossible(container));
+    }
+    DrawRect(context, root, kForegroundType, FloatRect(100, 100, 100, 100));
+  }
+  GetPaintController().CommitNewDisplayItems();
+
+  // The second paint should produce the exactly same results.
+  check_paint_results();
+}
+
 TEST_P(PaintControllerTest, UpdateSwapOrderCrossingChunks) {
   FakeDisplayItemClient container1("container1",
                                    LayoutRect(100, 100, 100, 100));