[PE] Fix a bug that PaintChunker used previous id for a forced chunk

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I0e9a46a9ec873c68ef8fd0d6fc4218a5a0c3eca2
Reviewed-on: https://chromium-review.googlesource.com/1017317Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552416}

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

@@ -49,6 +49,12 @@ bool PaintChunker::IncrementDisplayItemIndex(const DisplayItem& item) {
     // new chunk id will be treated as having no id to avoid the chunk from
     // using the same id as the chunk before the forced chunk.
     current_chunk_id_ = WTF::nullopt;
+  } else if (force_new_chunk_ && data_.chunks.size() &&
+             data_.chunks.back().id == current_chunk_id_) {
+    // For other forced_new_chunk_ reasons (e.g. subsequences), use the first
+    // display items' id if the client didn't specify an id for the forced new
+    // chunk (i.e. |current_chunk_id_| has been used by the previous chunk).
+    current_chunk_id_ = WTF::nullopt;
   }
 
   size_t new_chunk_begin_index;

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

@@ -281,7 +281,7 @@ TEST_F(PaintChunkerTest, CreatesSeparateChunksWhenRequested) {
           PaintChunk(5, 6, i3.GetId(), DefaultPaintChunkProperties())));
 }
 
-TEST_F(PaintChunkerTest, ForceNewChunk) {
+TEST_F(PaintChunkerTest, ForceNewChunkWithNewId) {
   PaintChunker chunker;
   PaintChunk::Id id0(client_, DisplayItemType(0));
   chunker.UpdateCurrentPaintChunkProperties(id0, DefaultPaintChunkProperties());
@@ -294,9 +294,9 @@ TEST_F(PaintChunkerTest, ForceNewChunk) {
   chunker.IncrementDisplayItemIndex(TestChunkerDisplayItem(client_));
   chunker.IncrementDisplayItemIndex(TestChunkerDisplayItem(client_));
 
+  chunker.ForceNewChunk();
   PaintChunk::Id id2(client_, DisplayItemType(20));
   chunker.UpdateCurrentPaintChunkProperties(id2, DefaultPaintChunkProperties());
-  chunker.ForceNewChunk();
   chunker.IncrementDisplayItemIndex(TestChunkerDisplayItem(client_));
   chunker.IncrementDisplayItemIndex(TestChunkerDisplayItem(client_));
 
@@ -308,6 +308,37 @@ TEST_F(PaintChunkerTest, ForceNewChunk) {
                   PaintChunk(4, 6, id2, DefaultPaintChunkProperties())));
 }
 
+TEST_F(PaintChunkerTest, ForceNewChunkWithoutNewId) {
+  PaintChunker chunker;
+  PaintChunk::Id id0(client_, DisplayItemType(0));
+  chunker.UpdateCurrentPaintChunkProperties(WTF::nullopt,
+                                            DefaultPaintChunkProperties());
+  chunker.IncrementDisplayItemIndex(
+      TestChunkerDisplayItem(id0.client, id0.type));
+  chunker.IncrementDisplayItemIndex(TestChunkerDisplayItem(client_));
+
+  chunker.ForceNewChunk();
+  PaintChunk::Id id1(client_, DisplayItemType(10));
+  chunker.IncrementDisplayItemIndex(
+      TestChunkerDisplayItem(id1.client, id1.type));
+  chunker.IncrementDisplayItemIndex(
+      TestChunkerDisplayItem(client_, DisplayItemType(11)));
+
+  chunker.ForceNewChunk();
+  PaintChunk::Id id2(client_, DisplayItemType(20));
+  chunker.IncrementDisplayItemIndex(
+      TestChunkerDisplayItem(id2.client, id2.type));
+  chunker.IncrementDisplayItemIndex(
+      TestChunkerDisplayItem(client_, DisplayItemType(21)));
+
+  Vector<PaintChunk> chunks = chunker.PaintChunks();
+  EXPECT_THAT(
+      chunks,
+      ElementsAre(PaintChunk(0, 2, id0, DefaultPaintChunkProperties()),
+                  PaintChunk(2, 4, id1, DefaultPaintChunkProperties()),
+                  PaintChunk(4, 6, id2, DefaultPaintChunkProperties())));
+}
+
 class TestScrollHitTestRequiringSeparateChunk : public TestChunkerDisplayItem {
  public:
   TestScrollHitTestRequiringSeparateChunk(const DisplayItemClient& client)

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

@@ -851,6 +851,16 @@ TEST_P(PaintControllerTest, CachedSubsequenceForcePaintChunk) {
 
   GetPaintController().CommitNewDisplayItems();
 
+  // Even though the paint properties match, |container| should receive its
+  // own PaintChunk because it created a subsequence.
+  EXPECT_EQ(3u, GetPaintController().GetPaintArtifact().PaintChunks().size());
+  EXPECT_EQ(root,
+            GetPaintController().GetPaintArtifact().PaintChunks()[0].id.client);
+  EXPECT_EQ(container,
+            GetPaintController().GetPaintArtifact().PaintChunks()[1].id.client);
+  EXPECT_EQ(root,
+            GetPaintController().GetPaintArtifact().PaintChunks()[2].id.client);
+
   root_properties.backface_hidden = true;
   // This time, record the fist chunk with backface_hidden == true
   GetPaintController().UpdateCurrentPaintChunkProperties(root_id,
@@ -860,8 +870,8 @@ TEST_P(PaintControllerTest, CachedSubsequenceForcePaintChunk) {
   DrawRect(context, root, kForegroundType, FloatRect(100, 100, 100, 100));
   GetPaintController().CommitNewDisplayItems();
 
-  // Even though the paint properties match, |container| should receive its
-  // own PaintChunk because it is a cached subsequence.
+  // |container| should still receive its own PaintChunk because it is a cached
+  // subsequence.
   EXPECT_EQ(3u, GetPaintController().GetPaintArtifact().PaintChunks().size());
   EXPECT_EQ(root,
             GetPaintController().GetPaintArtifact().PaintChunks()[0].id.client);