Remove begin/end subseq. display items, and store on PaintController instead.

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

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "platform/graphics/paint/PaintController.h"
 
 #include "platform/RuntimeEnabledFeatures.h"
 #include "platform/graphics/GraphicsContext.h"
 #include "platform/graphics/paint/ClipPathDisplayItem.h"
 #include "platform/graphics/paint/ClipPathRecorder.h"
@@ skipping 1065 matching lines @@
              FloatRect(100, 200, 100, 100));
     drawRect(context, content2, backgroundDrawingType,
              FloatRect(100, 200, 50, 200));
     drawRect(context, content2, foregroundDrawingType,
              FloatRect(100, 200, 50, 200));
     drawRect(context, container2, foregroundDrawingType,
              FloatRect(100, 200, 100, 100));
   }
   getPaintController().commitNewDisplayItems();
 
-  EXPECT_DISPLAY_LIST(
-      getPaintController().getDisplayItemList(), 12,
-      TestDisplayItem(container1, DisplayItem::kSubsequence),
-      TestDisplayItem(container1, backgroundDrawingType),
-      TestDisplayItem(content1, backgroundDrawingType),
-      TestDisplayItem(content1, foregroundDrawingType),
-      TestDisplayItem(container1, foregroundDrawingType),
-      TestDisplayItem(container1, DisplayItem::kEndSubsequence),
+  EXPECT_DISPLAY_LIST(getPaintController().getDisplayItemList(), 8,
+                      TestDisplayItem(container1, backgroundDrawingType),
+                      TestDisplayItem(content1, backgroundDrawingType),
+                      TestDisplayItem(content1, foregroundDrawingType),
+                      TestDisplayItem(container1, foregroundDrawingType),
 
-      TestDisplayItem(container2, DisplayItem::kSubsequence),
-      TestDisplayItem(container2, backgroundDrawingType),
-      TestDisplayItem(content2, backgroundDrawingType),
-      TestDisplayItem(content2, foregroundDrawingType),
-      TestDisplayItem(container2, foregroundDrawingType),
-      TestDisplayItem(container2, DisplayItem::kEndSubsequence));
+                      TestDisplayItem(container2, backgroundDrawingType),
+                      TestDisplayItem(content2, backgroundDrawingType),
+                      TestDisplayItem(content2, foregroundDrawingType),
+                      TestDisplayItem(container2, foregroundDrawingType));
+
+  PaintController::SubsequenceMarkers* markers =
+      getPaintController().getSubsequenceMarkers(container1);
+  CHECK(markers);
+  EXPECT_EQ(0ull, markers->start);
+  EXPECT_EQ(3ull, markers->end);
+
+  markers = getPaintController().getSubsequenceMarkers(container2);
+  CHECK(markers);
+  EXPECT_EQ(4ull, markers->start);
+  EXPECT_EQ(7ull, markers->end);
 
   if (RuntimeEnabledFeatures::slimmingPaintV2Enabled()) {
     EXPECT_EQ(2u, getPaintController().paintChunks().size());
     EXPECT_EQ(PaintChunk::Id(container1, backgroundDrawingType),
               getPaintController().paintChunks()[0].id);
     EXPECT_EQ(PaintChunk::Id(container2, backgroundDrawingType),
               getPaintController().paintChunks()[1].id);
     EXPECT_THAT(getPaintController().paintChunks()[0].rasterInvalidationRects,
                 UnorderedElementsAre(FloatRect(LayoutRect::infiniteIntRect())));
     EXPECT_THAT(getPaintController().paintChunks()[1].rasterInvalidationRects,
@@ skipping 42 matching lines @@
       drawRect(context, container1, foregroundDrawingType,
                FloatRect(100, 100, 100, 100));
     }
   } else {
     EXPECT_TRUE(SubsequenceRecorder::useCachedSubsequenceIfPossible(
         context, container2));
     EXPECT_TRUE(SubsequenceRecorder::useCachedSubsequenceIfPossible(
         context, container1));
   }
 
-  EXPECT_EQ(12, numCachedNewItems());
+  EXPECT_EQ(8, numCachedNewItems());
 #ifndef NDEBUG
-  EXPECT_EQ(1, numSequentialMatches());
-  EXPECT_EQ(1, numOutOfOrderMatches());
-  EXPECT_EQ(5, numIndexedItems());
+  EXPECT_EQ(0, numSequentialMatches());

[chrishtr, 2017/04/04 18:12:48]

I'm not quite sure these are correct. Could you check?

[Xianzhu, 2017/04/05 22:39:17]

These look good if stat of subsequences are excluded. These are to ensure we use
the best performance path when possible (sequential match), and match
out-of-order (using HashMap::find) and index items (using HashMap::add) only
when needed.

+  EXPECT_EQ(0, numOutOfOrderMatches());
+  EXPECT_EQ(0, numIndexedItems());
 #endif
 
   getPaintController().commitNewDisplayItems();
 
-  EXPECT_DISPLAY_LIST(
-      getPaintController().getDisplayItemList(), 12,
-      TestDisplayItem(container2, DisplayItem::kSubsequence),
-      TestDisplayItem(container2, backgroundDrawingType),
-      TestDisplayItem(content2, backgroundDrawingType),
-      TestDisplayItem(content2, foregroundDrawingType),
-      TestDisplayItem(container2, foregroundDrawingType),
-      TestDisplayItem(container2, DisplayItem::kEndSubsequence),
+  EXPECT_DISPLAY_LIST(getPaintController().getDisplayItemList(), 8,
+                      TestDisplayItem(container2, backgroundDrawingType),
+                      TestDisplayItem(content2, backgroundDrawingType),
+                      TestDisplayItem(content2, foregroundDrawingType),
+                      TestDisplayItem(container2, foregroundDrawingType),
+                      TestDisplayItem(container1, backgroundDrawingType),
+                      TestDisplayItem(content1, backgroundDrawingType),
+                      TestDisplayItem(content1, foregroundDrawingType),
+                      TestDisplayItem(container1, foregroundDrawingType));
 
-      TestDisplayItem(container1, DisplayItem::kSubsequence),
-      TestDisplayItem(container1, backgroundDrawingType),
-      TestDisplayItem(content1, backgroundDrawingType),
-      TestDisplayItem(content1, foregroundDrawingType),
-      TestDisplayItem(container1, foregroundDrawingType),
-      TestDisplayItem(container1, DisplayItem::kEndSubsequence));
+  markers = getPaintController().getSubsequenceMarkers(container2);
+  CHECK(markers);
+  EXPECT_EQ(0ull, markers->start);
+  EXPECT_EQ(3ull, markers->end);
+
+  markers = getPaintController().getSubsequenceMarkers(container1);
+  CHECK(markers);
+  EXPECT_EQ(4ull, markers->start);
+  EXPECT_EQ(7ull, markers->end);
 
 #if CHECK_DISPLAY_ITEM_CLIENT_ALIVENESS
   DisplayItemClient::endShouldKeepAliveAllClients();
 #endif
 
   if (RuntimeEnabledFeatures::slimmingPaintV2Enabled()) {
     EXPECT_EQ(2u, getPaintController().paintChunks().size());
     EXPECT_EQ(PaintChunk::Id(container2, backgroundDrawingType),
               getPaintController().paintChunks()[0].id);
     EXPECT_EQ(PaintChunk::Id(container1, backgroundDrawingType),
@@ skipping 222 matching lines @@
         getPaintController().updateCurrentPaintChunkProperties(
             &id, content2Properties);
       }
       SubsequenceRecorder r(context, content2);
       drawRect(context, content2, backgroundDrawingType,
                FloatRect(100, 200, 50, 200));
     }
   }
   getPaintController().commitNewDisplayItems();
 
-  EXPECT_DISPLAY_LIST(
-      getPaintController().getDisplayItemList(), 14,
-      TestDisplayItem(container1, DisplayItem::kSubsequence),
-      TestDisplayItem(container1, backgroundDrawingType),
-      TestDisplayItem(content1, DisplayItem::kSubsequence),
-      TestDisplayItem(content1, backgroundDrawingType),
-      TestDisplayItem(content1, foregroundDrawingType),
-      TestDisplayItem(content1, DisplayItem::kEndSubsequence),
-      TestDisplayItem(container1, foregroundDrawingType),
-      TestDisplayItem(container1, DisplayItem::kEndSubsequence),
+  EXPECT_DISPLAY_LIST(getPaintController().getDisplayItemList(), 6,
+                      TestDisplayItem(container1, backgroundDrawingType),
+                      TestDisplayItem(content1, backgroundDrawingType),
+                      TestDisplayItem(content1, foregroundDrawingType),
+                      TestDisplayItem(container1, foregroundDrawingType),
+                      TestDisplayItem(container2, backgroundDrawingType),
+                      TestDisplayItem(content2, backgroundDrawingType));
 
-      TestDisplayItem(container2, DisplayItem::kSubsequence),
-      TestDisplayItem(container2, backgroundDrawingType),
-      TestDisplayItem(content2, DisplayItem::kSubsequence),
-      TestDisplayItem(content2, backgroundDrawingType),
-      TestDisplayItem(content2, DisplayItem::kEndSubsequence),
-      TestDisplayItem(container2, DisplayItem::kEndSubsequence));
+  PaintController::SubsequenceMarkers* markers =
+      getPaintController().getSubsequenceMarkers(container1);
+  CHECK(markers);
+  EXPECT_EQ(0ull, markers->start);
+  EXPECT_EQ(3ull, markers->end);
+
+  markers = getPaintController().getSubsequenceMarkers(content1);
+  CHECK(markers);
+  EXPECT_EQ(1ull, markers->start);
+  EXPECT_EQ(2ull, markers->end);
+
+  markers = getPaintController().getSubsequenceMarkers(container2);
+  CHECK(markers);
+  EXPECT_EQ(4ull, markers->start);
+  EXPECT_EQ(5ull, markers->end);
+
+  markers = getPaintController().getSubsequenceMarkers(content2);
+  CHECK(markers);
+  EXPECT_EQ(5ull, markers->start);
+  EXPECT_EQ(5ull, markers->end);
 
   if (RuntimeEnabledFeatures::slimmingPaintV2Enabled()) {
     EXPECT_EQ(5u, getPaintController().paintChunks().size());
     EXPECT_EQ(PaintChunk::Id(container1, backgroundDrawingType),
               getPaintController().paintChunks()[0].id);
     EXPECT_EQ(PaintChunk::Id(content1, backgroundDrawingType),
               getPaintController().paintChunks()[1].id);
     EXPECT_EQ(PaintChunk::Id(container1, foregroundDrawingType),
               getPaintController().paintChunks()[2].id);
     EXPECT_EQ(PaintChunk::Id(container2, backgroundDrawingType),
@@ skipping 63 matching lines @@
     }
     if (RuntimeEnabledFeatures::slimmingPaintV2Enabled()) {
       PaintChunk::Id id(container1, foregroundDrawingType);
       getPaintController().updateCurrentPaintChunkProperties(
           &id, container1ForegroundProperties);
     }
     drawRect(context, container1, foregroundDrawingType,
              FloatRect(100, 100, 100, 100));
   }
 
-  EXPECT_EQ(4, numCachedNewItems());
+  EXPECT_EQ(2, numCachedNewItems());
 #ifndef NDEBUG
-  EXPECT_EQ(1, numSequentialMatches());
+  EXPECT_EQ(0, numSequentialMatches());

[chrishtr, 2017/04/04 18:12:48]

I'm not quite sure these are correct. Could you check?

   EXPECT_EQ(0, numOutOfOrderMatches());
-  EXPECT_EQ(2, numIndexedItems());
+  EXPECT_EQ(0, numIndexedItems());
 #endif
 
   getPaintController().commitNewDisplayItems();
 
-  EXPECT_DISPLAY_LIST(
-      getPaintController().getDisplayItemList(), 10,
-      TestDisplayItem(content2, DisplayItem::kSubsequence),
-      TestDisplayItem(content2, foregroundDrawingType),
-      TestDisplayItem(content2, DisplayItem::kEndSubsequence),
+  EXPECT_DISPLAY_LIST(getPaintController().getDisplayItemList(), 4,
+                      TestDisplayItem(content2, foregroundDrawingType),
+                      TestDisplayItem(content1, backgroundDrawingType),
+                      TestDisplayItem(content1, foregroundDrawingType),
+                      TestDisplayItem(container1, foregroundDrawingType));
 
-      TestDisplayItem(container1, DisplayItem::kSubsequence),
-      TestDisplayItem(content1, DisplayItem::kSubsequence),
-      TestDisplayItem(content1, backgroundDrawingType),
-      TestDisplayItem(content1, foregroundDrawingType),
-      TestDisplayItem(content1, DisplayItem::kEndSubsequence),
-      TestDisplayItem(container1, foregroundDrawingType),
-      TestDisplayItem(container1, DisplayItem::kEndSubsequence));
+  markers = getPaintController().getSubsequenceMarkers(content2);
+  CHECK(markers);
+  EXPECT_EQ(0ull, markers->start);
+  EXPECT_EQ(0ull, markers->end);
+
+  markers = getPaintController().getSubsequenceMarkers(container1);
+  CHECK(markers);
+  EXPECT_EQ(1ull, markers->start);
+  EXPECT_EQ(3ull, markers->end);
+
+  markers = getPaintController().getSubsequenceMarkers(content1);
+  CHECK(markers);
+  EXPECT_EQ(1ull, markers->start);
+  EXPECT_EQ(2ull, markers->end);
 
   if (RuntimeEnabledFeatures::slimmingPaintV2Enabled()) {
     EXPECT_EQ(3u, getPaintController().paintChunks().size());
     EXPECT_EQ(PaintChunk::Id(content2, foregroundDrawingType),
               getPaintController().paintChunks()[0].id);
     EXPECT_EQ(PaintChunk::Id(content1, backgroundDrawingType),
               getPaintController().paintChunks()[1].id);
     EXPECT_EQ(PaintChunk::Id(container1, foregroundDrawingType),
               getPaintController().paintChunks()[2].id);
     // This is a new chunk.
@@ skipping 30 matching lines @@
 
   drawRect(context, multicol, backgroundDrawingType,
            FloatRect(100, 200, 100, 100));
 
   getPaintController().beginSkippingCache();
   drawRect(context, content, foregroundDrawingType, rect1);
   drawRect(context, content, foregroundDrawingType, rect2);
   getPaintController().endSkippingCache();
 
   getPaintController().commitNewDisplayItems();
-
   EXPECT_DISPLAY_LIST(getPaintController().getDisplayItemList(), 3,
                       TestDisplayItem(multicol, backgroundDrawingType),
                       TestDisplayItem(content, foregroundDrawingType),
                       TestDisplayItem(content, foregroundDrawingType));
   sk_sp<const PaintRecord> record1 =
       static_cast<const DrawingDisplayItem&>(
           getPaintController().getDisplayItemList()[1])
           .GetPaintRecord();
   sk_sp<const PaintRecord> record2 =
       static_cast<const DrawingDisplayItem&>(
@@ skipping 622 matching lines @@
     FakeDisplayItemClient content("content");
     GraphicsContext context(getPaintController());
 
     {
       SubsequenceRecorder subsequence(context, container);
       CompositingRecorder compositing(context, content, SkBlendMode::kSrc, 0.5);
       drawRect(context, content, backgroundDrawingType,
                FloatRect(100, 100, 300, 300));
     }
     getPaintController().commitNewDisplayItems();
-    EXPECT_EQ(3u,
+    EXPECT_EQ(1u,
               getPaintController().paintArtifact().getDisplayItemList().size());
 
     {
       EXPECT_FALSE(SubsequenceRecorder::useCachedSubsequenceIfPossible(
           context, container));
       SubsequenceRecorder subsequence(context, container);
       CompositingRecorder compositing(context, content, SkBlendMode::kSrc, 0.5);
       drawRect(context, content, backgroundDrawingType,
                FloatRect(100, 100, 300, 300));
     }
     getPaintController().commitNewDisplayItems();
-    EXPECT_EQ(3u,
+    EXPECT_EQ(1u,
               getPaintController().paintArtifact().getDisplayItemList().size());
 
 #if CHECK_DISPLAY_ITEM_CLIENT_ALIVENESS
     DisplayItemClient::endShouldKeepAliveAllClients();
 #endif
   }
 };
 
 TEST_F(PaintControllerUnderInvalidationTest, ChangeDrawing) {
   EXPECT_DEATH(testChangeDrawing(), "under-invalidation: display item changed");
@@ skipping 18 matching lines @@
 
 TEST_F(PaintControllerUnderInvalidationTest, ChangeDrawingInSubsequence) {
   EXPECT_DEATH(testChangeDrawingInSubsequence(),
                "\"\\(In cached subsequence of first\\)\" under-invalidation: "
                "display item changed");
 }
 
 TEST_F(PaintControllerUnderInvalidationTest, MoreDrawingInSubsequence) {
   EXPECT_DEATH(testMoreDrawingInSubsequence(),
                "\"\\(In cached subsequence of first\\)\" under-invalidation: "
-               "display item changed");
+               "new subsequence wrong length");
 }
 
 TEST_F(PaintControllerUnderInvalidationTest, LessDrawingInSubsequence) {
+  // We allow invalidated display item clients as long as they would produce the
+  // same display items. The cases of changed display items are tested by other
+  // test cases.
   EXPECT_DEATH(testLessDrawingInSubsequence(),
                "\"\\(In cached subsequence of first\\)\" under-invalidation: "
-               "display item changed");
+               "new subsequence wrong length");
 }
 
 TEST_F(PaintControllerUnderInvalidationTest, ChangeNonCacheableInSubsequence) {
   EXPECT_DEATH(testChangeNonCacheableInSubsequence(),
                "\"\\(In cached subsequence of container\\)\" "
                "under-invalidation: display item changed");
 }
 
 TEST_F(PaintControllerUnderInvalidationTest, InvalidationInSubsequence) {
   // We allow invalidated display item clients as long as they would produce the
   // same display items. The cases of changed display items are tested by other
   // test cases.
   testInvalidationInSubsequence();
 }
 
 TEST_F(PaintControllerUnderInvalidationTest,
        FoldCompositingDrawingInSubsequence) {
   testFoldCompositingDrawingInSubsequence();
 }
 
 #endif  // defined(GTEST_HAS_DEATH_TEST) && !OS(ANDROID)
 
 }  // namespace blink