Add DisplayItemList::newDisplayItems() and cleanup tests

We'll need to access DisplayItemList::newDisplayItems() to update
cacheable flag of BeginSubsequence display items for
https://codereview.chromium.org/1327563003/.

Update layout tests and make some cleanups.

Review URL: https://codereview.chromium.org/1323243011

git-svn-id: svn://svn.chromium.org/blink/trunk@201902 bbb929c8-8fbe-4397-9dbb-9b2b20218538

third_party/WebKit/Source/core/paint/DisplayItemListPaintTest.h

@@ -17,13 +17,11 @@ namespace blink {
 class DisplayItemListPaintTest : public RenderingTest {
 public:
     DisplayItemListPaintTest()
-        : m_layoutView(nullptr)
-        , m_originalSlimmingPaintEnabled(RuntimeEnabledFeatures::slimmingPaintEnabled()) { }
+        : m_originalSlimmingPaintEnabled(RuntimeEnabledFeatures::slimmingPaintEnabled()) { }
 
 protected:
-    LayoutView& layoutView() { return *m_layoutView; }
+    LayoutView& layoutView() { return *document().layoutView(); }
     DisplayItemList& rootDisplayItemList() { return *layoutView().layer()->graphicsLayerBacking()->displayItemList(); }
-    const DisplayItems& newDisplayItemsBeforeUpdate() { return rootDisplayItemList().m_newDisplayItems; }
 
 private:
     void SetUp() override
@@ -32,9 +30,6 @@ private:
 
         RenderingTest::SetUp();
         enableCompositing();
-
-        m_layoutView = document().view()->layoutView();
-        ASSERT_TRUE(m_layoutView);
     }
 
     void TearDown() override
@@ -42,7 +37,6 @@ private:
         RuntimeEnabledFeatures::setSlimmingPaintEnabled(m_originalSlimmingPaintEnabled);
     }
 
-    LayoutView* m_layoutView;
     bool m_originalSlimmingPaintEnabled;
 };
 
@@ -51,13 +45,11 @@ private:
 class DisplayItemListPaintTestForSlimmingPaintV2 : public RenderingTest {
 public:
     DisplayItemListPaintTestForSlimmingPaintV2()
-        : m_layoutView(nullptr)
-        , m_originalSlimmingPaintV2Enabled(RuntimeEnabledFeatures::slimmingPaintV2Enabled()) { }
+        : m_originalSlimmingPaintV2Enabled(RuntimeEnabledFeatures::slimmingPaintV2Enabled()) { }
 
 protected:
-    LayoutView& layoutView() { return *m_layoutView; }
+    LayoutView& layoutView() { return *document().layoutView(); }
     DisplayItemList& rootDisplayItemList() { return *layoutView().layer()->graphicsLayerBacking()->displayItemList(); }
-    const DisplayItems& newDisplayItemsBeforeUpdate() { return rootDisplayItemList().m_newDisplayItems; }
 
     // Expose some document lifecycle steps for checking new display items before commiting.
     void updateLifecyclePhasesToPaintForSlimmingPaintV2Clean()
@@ -76,9 +68,6 @@ private:
 
         RenderingTest::SetUp();
         enableCompositing();
-
-        m_layoutView = document().view()->layoutView();
-        ASSERT_TRUE(m_layoutView);
     }
 
     void TearDown() override
@@ -86,7 +75,6 @@ private:
         RuntimeEnabledFeatures::setSlimmingPaintV2Enabled(m_originalSlimmingPaintV2Enabled);
     }
 
-    LayoutView* m_layoutView;
     bool m_originalSlimmingPaintV2Enabled;
 };
 
@@ -123,12 +111,12 @@ public:
 #define EXPECT_DISPLAY_LIST_WITH_RED_FILL_IN_DEBUG(actual, expectedSizeWithoutFill, ...) \
     EXPECT_DISPLAY_LIST_BASE( \
         actual, expectedSizeWithoutFill + 1, \
-        TestDisplayItem(*document().layoutView()->layer()->graphicsLayerBacking(), DisplayItem::DebugRedFill), \
+        TestDisplayItem(*layoutView().layer()->graphicsLayerBacking(), DisplayItem::DebugRedFill), \
         __VA_ARGS__)
 #define EXPECT_DISPLAY_LIST_WITH_CACHED_RED_FILL_IN_DEBUG(actual, expectedSizeWithoutFill, ...) \
     EXPECT_DISPLAY_LIST_BASE( \
         actual, expectedSizeWithoutFill + 1, \
-        TestDisplayItem(*document().layoutView()->layer()->graphicsLayerBacking(), DisplayItem::drawingTypeToCachedDrawingType(DisplayItem::DebugRedFill)), \
+        TestDisplayItem(*layoutView().layer()->graphicsLayerBacking(), DisplayItem::drawingTypeToCachedDrawingType(DisplayItem::DebugRedFill)), \
         __VA_ARGS__)
 #else
 #define EXPECT_DISPLAY_LIST_WITH_RED_FILL_IN_DEBUG EXPECT_DISPLAY_LIST_BASE

third_party/WebKit/Source/core/paint/LayoutObjectDrawingRecorderTest.cpp

@@ -8,6 +8,7 @@
 #include "core/layout/LayoutTestHelper.h"
 #include "core/layout/LayoutView.h"
 #include "core/paint/DeprecatedPaintLayer.h"
+#include "core/paint/DisplayItemListPaintTest.h"
 #include "platform/graphics/GraphicsContext.h"
 #include "platform/graphics/GraphicsLayer.h"
 #include "platform/graphics/paint/DisplayItemList.h"
@@ -16,73 +17,8 @@
 
 namespace blink {
 
-class LayoutObjectDrawingRecorderTest : public RenderingTest {
-public:
-    LayoutObjectDrawingRecorderTest()
-        : m_layoutView(nullptr)
-        , m_originalSlimmingPaintEnabled(RuntimeEnabledFeatures::slimmingPaintEnabled()) { }
-
-protected:
-    LayoutView& layoutView() { return *m_layoutView; }
-    DisplayItemList& rootDisplayItemList() { return *layoutView().layer()->graphicsLayerBacking()->displayItemList(); }
-    const DisplayItems& newDisplayItemsBeforeUpdate() { return rootDisplayItemList().m_newDisplayItems; }
-
-private:
-    void SetUp() override
-    {
-        RuntimeEnabledFeatures::setSlimmingPaintEnabled(true);
-
-        RenderingTest::SetUp();
-        enableCompositing();
-
-        m_layoutView = document().view()->layoutView();
-        ASSERT_TRUE(m_layoutView);
-    }
-
-    void TearDown() override
-    {
-        RuntimeEnabledFeatures::setSlimmingPaintEnabled(m_originalSlimmingPaintEnabled);
-    }
-
-    LayoutView* m_layoutView;
-    bool m_originalSlimmingPaintEnabled;
-};
-
-class LayoutObjectDrawingRecorderTestForSlimmingPaintV2 : public RenderingTest {
-public:
-    LayoutObjectDrawingRecorderTestForSlimmingPaintV2()
-        : m_layoutView(nullptr)
-        , m_originalSlimmingPaintEnabled(RuntimeEnabledFeatures::slimmingPaintEnabled())
-        , m_originalSlimmingPaintV2Enabled(RuntimeEnabledFeatures::slimmingPaintV2Enabled()) { }
-
-protected:
-    LayoutView& layoutView() { return *m_layoutView; }
-    DisplayItemList& rootDisplayItemList() { return *layoutView().layer()->graphicsLayerBacking()->displayItemList(); }
-    const DisplayItems& newDisplayItemsBeforeUpdate() { return rootDisplayItemList().m_newDisplayItems; }
-
-private:
-    void SetUp() override
-    {
-        RuntimeEnabledFeatures::setSlimmingPaintEnabled(true);
-        RuntimeEnabledFeatures::setSlimmingPaintV2Enabled(true);
-
-        RenderingTest::SetUp();
-        enableCompositing();
-
-        m_layoutView = document().view()->layoutView();
-        ASSERT_TRUE(m_layoutView);
-    }
-
-    void TearDown() override
-    {
-        RuntimeEnabledFeatures::setSlimmingPaintEnabled(m_originalSlimmingPaintV2Enabled);
-        RuntimeEnabledFeatures::setSlimmingPaintEnabled(m_originalSlimmingPaintEnabled);
-    }
-
-    LayoutView* m_layoutView;
-    bool m_originalSlimmingPaintEnabled;
-    bool m_originalSlimmingPaintV2Enabled;
-};
+using LayoutObjectDrawingRecorderTest = DisplayItemListPaintTest;
+using LayoutObjectDrawingRecorderTestForSlimmingPaintV2 = DisplayItemListPaintTestForSlimmingPaintV2;
 
 namespace {
 
@@ -103,16 +39,6 @@ void drawRect(GraphicsContext& context, LayoutView& layoutView, PaintPhase phase
     context.drawRect(rect);
 }
 
-bool isDrawing(const DisplayItem& item)
-{
-    return DisplayItem::isDrawingType(item.type());
-}
-
-bool isCached(const DisplayItem& item)
-{
-    return DisplayItem::isCachedType(item.type());
-}
-
 TEST_F(LayoutObjectDrawingRecorderTest, Nothing)
 {
     GraphicsContext context(&rootDisplayItemList());
@@ -121,10 +47,9 @@ TEST_F(LayoutObjectDrawingRecorderTest, Nothing)
 
     drawNothing(context, layoutView(), PaintPhaseForeground, bound);
     rootDisplayItemList().commitNewDisplayItems();
-    EXPECT_EQ((size_t)1, rootDisplayItemList().displayItems().size());
-    const auto& item = rootDisplayItemList().displayItems()[0];
-    ASSERT_TRUE(isDrawing(item));
-    EXPECT_FALSE(static_cast<const DrawingDisplayItem&>(item).picture());
+    EXPECT_DISPLAY_LIST_BASE(rootDisplayItemList().displayItems(), 1,
+        TestDisplayItem(layoutView(), DisplayItem::paintPhaseToDrawingType(PaintPhaseForeground)));
+    EXPECT_FALSE(static_cast<const DrawingDisplayItem&>(rootDisplayItemList().displayItems()[0]).picture());
 }
 
 TEST_F(LayoutObjectDrawingRecorderTest, Rect)
@@ -133,8 +58,8 @@ TEST_F(LayoutObjectDrawingRecorderTest, Rect)
     LayoutRect bound = layoutView().viewRect();
     drawRect(context, layoutView(), PaintPhaseForeground, bound);
     rootDisplayItemList().commitNewDisplayItems();
-    EXPECT_EQ((size_t)1, rootDisplayItemList().displayItems().size());
-    EXPECT_TRUE(isDrawing(rootDisplayItemList().displayItems()[0]));
+    EXPECT_DISPLAY_LIST_BASE(rootDisplayItemList().displayItems(), 1,
+        TestDisplayItem(layoutView(), DisplayItem::paintPhaseToDrawingType(PaintPhaseForeground)));
 }
 
 TEST_F(LayoutObjectDrawingRecorderTest, Cached)
@@ -144,19 +69,23 @@ TEST_F(LayoutObjectDrawingRecorderTest, Cached)
     drawNothing(context, layoutView(), PaintPhaseBlockBackground, bound);
     drawRect(context, layoutView(), PaintPhaseForeground, bound);
     rootDisplayItemList().commitNewDisplayItems();
-    EXPECT_EQ((size_t)2, rootDisplayItemList().displayItems().size());
-    EXPECT_TRUE(isDrawing(rootDisplayItemList().displayItems()[0]));
-    EXPECT_TRUE(isDrawing(rootDisplayItemList().displayItems()[1]));
+
+    EXPECT_DISPLAY_LIST_BASE(rootDisplayItemList().displayItems(), 2,
+        TestDisplayItem(layoutView(), DisplayItem::paintPhaseToDrawingType(PaintPhaseBlockBackground)),
+        TestDisplayItem(layoutView(), DisplayItem::paintPhaseToDrawingType(PaintPhaseForeground)));
 
     drawNothing(context, layoutView(), PaintPhaseBlockBackground, bound);
     drawRect(context, layoutView(), PaintPhaseForeground, bound);
-    EXPECT_EQ((size_t)2, newDisplayItemsBeforeUpdate().size());
-    EXPECT_TRUE(isCached(newDisplayItemsBeforeUpdate()[0]));
-    EXPECT_TRUE(isCached(newDisplayItemsBeforeUpdate()[1]));
+
+    EXPECT_DISPLAY_LIST_BASE(rootDisplayItemList().newDisplayItems(), 2,
+        TestDisplayItem(layoutView(), DisplayItem::drawingTypeToCachedDrawingType(DisplayItem::paintPhaseToDrawingType(PaintPhaseBlockBackground))),
+        TestDisplayItem(layoutView(), DisplayItem::drawingTypeToCachedDrawingType(DisplayItem::paintPhaseToDrawingType(PaintPhaseForeground))));
+
     rootDisplayItemList().commitNewDisplayItems();
-    EXPECT_EQ((size_t)2, rootDisplayItemList().displayItems().size());
-    EXPECT_TRUE(isDrawing(rootDisplayItemList().displayItems()[0]));
-    EXPECT_TRUE(isDrawing(rootDisplayItemList().displayItems()[1]));
+
+    EXPECT_DISPLAY_LIST_BASE(rootDisplayItemList().displayItems(), 2,
+        TestDisplayItem(layoutView(), DisplayItem::paintPhaseToDrawingType(PaintPhaseBlockBackground)),
+        TestDisplayItem(layoutView(), DisplayItem::paintPhaseToDrawingType(PaintPhaseForeground)));
 }
 
 template <typename T>
@@ -205,8 +134,8 @@ TEST_F(LayoutObjectDrawingRecorderTestForSlimmingPaintV2, PaintOffsetCache)
     }
 
     rootDisplayItemList().commitNewDisplayItems();
-    EXPECT_EQ((size_t)1, rootDisplayItemList().displayItems().size());
-    EXPECT_TRUE(isDrawing(rootDisplayItemList().displayItems()[0]));
+    EXPECT_DISPLAY_LIST_BASE(rootDisplayItemList().displayItems(), 1,
+        TestDisplayItem(layoutView(), DisplayItem::paintPhaseToDrawingType(PaintPhaseForeground)));
 
     // Ensure we cannot use the cache with a new paint offset.
     LayoutPoint newPaintOffset(2, 3);
@@ -220,8 +149,8 @@ TEST_F(LayoutObjectDrawingRecorderTestForSlimmingPaintV2, PaintOffsetCache)
     }
 
     rootDisplayItemList().commitNewDisplayItems();
-    EXPECT_EQ((size_t)1, rootDisplayItemList().displayItems().size());
-    EXPECT_TRUE(isDrawing(rootDisplayItemList().displayItems()[0]));
+    EXPECT_DISPLAY_LIST_BASE(rootDisplayItemList().displayItems(), 1,
+        TestDisplayItem(layoutView(), DisplayItem::paintPhaseToDrawingType(PaintPhaseForeground)));
 
     // Ensure the old paint offset cannot be used.
     EXPECT_FALSE(LayoutObjectDrawingRecorder::useCachedDrawingIfPossible(context, layoutView(), PaintPhaseForeground, paintOffset));
@@ -229,8 +158,8 @@ TEST_F(LayoutObjectDrawingRecorderTestForSlimmingPaintV2, PaintOffsetCache)
     // Ensure the new paint offset can be used.
     EXPECT_TRUE(LayoutObjectDrawingRecorder::useCachedDrawingIfPossible(context, layoutView(), PaintPhaseForeground, newPaintOffset));
     rootDisplayItemList().commitNewDisplayItems();
-    EXPECT_EQ((size_t)1, rootDisplayItemList().displayItems().size());
-    EXPECT_TRUE(isDrawing(rootDisplayItemList().displayItems()[0]));
+    EXPECT_DISPLAY_LIST_BASE(rootDisplayItemList().displayItems(), 1,
+        TestDisplayItem(layoutView(), DisplayItem::paintPhaseToDrawingType(PaintPhaseForeground)));
 }
 
 } // namespace

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

@@ -134,9 +134,9 @@ public:
     bool displayItemConstructionIsDisabled() const { return m_constructionDisabled; }
     void setDisplayItemConstructionIsDisabled(const bool disable) { m_constructionDisabled = disable; }
 
-#if ENABLE(ASSERT)
-    size_t newDisplayItemsSize() const { return m_newDisplayItems.size(); }
-#endif
+    // Returns displayItems added using createAndAppend() since beginning or the last
+    // commitNewDisplayItems(). Use with care.
+    DisplayItems& newDisplayItems() { return m_newDisplayItems; }
 
 #ifndef NDEBUG
     void showDebugData() const;
@@ -169,12 +169,6 @@ protected:
         , m_nextScope(1) { }
 
 private:
-    friend class DisplayItemListTest;
-    friend class DisplayItemListPaintTest;
-    friend class DisplayItemListPaintTestForSlimmingPaintV2;
-    friend class LayoutObjectDrawingRecorderTest;
-    friend class LayoutObjectDrawingRecorderTestForSlimmingPaintV2;
-
     // Set new item state (scopes, cache skipping, etc) for a new item.
     void processNewItem(DisplayItem&);
 

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

@@ -20,11 +20,11 @@ namespace blink {
 class DisplayItemListTest : public ::testing::Test {
 public:
     DisplayItemListTest()
-        : m_originalSlimmingPaintV2Enabled(RuntimeEnabledFeatures::slimmingPaintV2Enabled()) { }
+        : m_displayItemList(DisplayItemList::create())
+        , m_originalSlimmingPaintV2Enabled(RuntimeEnabledFeatures::slimmingPaintV2Enabled()) { }
 
 protected:
-    DisplayItemList& displayItemList() { return m_displayItemList; }
-    const DisplayItems& newDisplayItemsBeforeUpdate() { return displayItemList().m_newDisplayItems; }
+    DisplayItemList& displayItemList() { return *m_displayItemList; }
 
 private:
     void SetUp() override
@@ -37,7 +37,7 @@ private:
         RuntimeEnabledFeatures::setSlimmingPaintV2Enabled(m_originalSlimmingPaintV2Enabled);
     }
 
-    DisplayItemList m_displayItemList;
+    OwnPtr<DisplayItemList> m_displayItemList;
     bool m_originalSlimmingPaintV2Enabled;
 };
 
@@ -494,7 +494,7 @@ TEST_F(DisplayItemListTest, CachedSubsequenceSwapOrder)
     EXPECT_TRUE(SubsequenceRecorder::useCachedSubsequenceIfPossible(context, container2));
     EXPECT_TRUE(SubsequenceRecorder::useCachedSubsequenceIfPossible(context, container1));
 
-    EXPECT_DISPLAY_LIST(newDisplayItemsBeforeUpdate(), 2,
+    EXPECT_DISPLAY_LIST(displayItemList().newDisplayItems(), 2,
         TestDisplayItem(container2, DisplayItem::CachedSubsequence),
         TestDisplayItem(container1, DisplayItem::CachedSubsequence));
 
@@ -608,7 +608,7 @@ TEST_F(DisplayItemListTest, CachedNestedSubsequenceUpdate)
         EXPECT_TRUE(SubsequenceRecorder::useCachedSubsequenceIfPossible(context, content1));
         drawRect(context, container1, foregroundDrawingType, FloatRect(100, 100, 100, 100));
     }
-    EXPECT_DISPLAY_LIST(newDisplayItemsBeforeUpdate(), 7,
+    EXPECT_DISPLAY_LIST(displayItemList().newDisplayItems(), 7,
         TestDisplayItem(content2, DisplayItem::BeginSubsequence),
         TestDisplayItem(content2, foregroundDrawingType),
         TestDisplayItem(content2, DisplayItem::EndSubsequence),
@@ -673,9 +673,11 @@ TEST_F(DisplayItemListTest, Scope)
     drawRect(context, content, foregroundDrawingType, rect2);
     displayItemList().endScope();
 
-    EXPECT_TRUE(newDisplayItemsBeforeUpdate()[0].isCachedDrawing());
-    EXPECT_TRUE(newDisplayItemsBeforeUpdate()[1].isDrawing());
-    EXPECT_TRUE(newDisplayItemsBeforeUpdate()[2].isDrawing());
+    EXPECT_DISPLAY_LIST(displayItemList().newDisplayItems(), 3,
+        TestDisplayItem(multicol, DisplayItem::drawingTypeToCachedDrawingType(backgroundDrawingType)),
+        TestDisplayItem(content, foregroundDrawingType),
+        TestDisplayItem(content, foregroundDrawingType));
+
     displayItemList().commitNewDisplayItems();
 
     EXPECT_DISPLAY_LIST(displayItemList().displayItems(), 3,
@@ -702,19 +704,15 @@ TEST_F(DisplayItemListTest, Scope)
     displayItemList().endScope();
 
     // We should repaint everything on invalidation of the scope container.
-    EXPECT_TRUE(newDisplayItemsBeforeUpdate()[0].isDrawing());
-    EXPECT_TRUE(newDisplayItemsBeforeUpdate()[1].isDrawing());
-    EXPECT_TRUE(newDisplayItemsBeforeUpdate()[2].isDrawing());
-    EXPECT_TRUE(newDisplayItemsBeforeUpdate()[3].isDrawing());
-    displayItemList().commitNewDisplayItems();
-
-    EXPECT_DISPLAY_LIST(displayItemList().displayItems(), 4,
+    EXPECT_DISPLAY_LIST(displayItemList().newDisplayItems(), 4,
         TestDisplayItem(multicol, backgroundDrawingType),
         TestDisplayItem(content, foregroundDrawingType),
         TestDisplayItem(content, foregroundDrawingType),
         TestDisplayItem(content, foregroundDrawingType));
-    EXPECT_NE(picture1, static_cast<const DrawingDisplayItem&>(displayItemList().displayItems()[1]).picture());
-    EXPECT_NE(picture2, static_cast<const DrawingDisplayItem&>(displayItemList().displayItems()[2]).picture());
+    EXPECT_NE(picture1, static_cast<const DrawingDisplayItem&>(displayItemList().newDisplayItems()[1]).picture());
+    EXPECT_NE(picture2, static_cast<const DrawingDisplayItem&>(displayItemList().newDisplayItems()[2]).picture());
+
+    displayItemList().commitNewDisplayItems();
 }
 
 TEST_F(DisplayItemListTest, OptimizeNoopPairs)

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

@@ -37,7 +37,7 @@ DrawingRecorder::DrawingRecorder(GraphicsContext& context, const DisplayItemClie
     , m_displayItemClient(displayItemClient)
     , m_displayItemType(displayItemType)
 #if ENABLE(ASSERT)
-    , m_displayItemPosition(RuntimeEnabledFeatures::slimmingPaintEnabled() ? m_context.displayItemList()->newDisplayItemsSize() : 0)
+    , m_displayItemPosition(RuntimeEnabledFeatures::slimmingPaintEnabled() ? m_context.displayItemList()->newDisplayItems().size() : 0)
     , m_underInvalidationCheckingMode(DrawingDisplayItem::CheckPicture)
 #endif
 {
@@ -91,7 +91,7 @@ DrawingRecorder::~DrawingRecorder()
         m_context.restore();
 
     m_context.setInDrawingRecorder(false);
-    ASSERT(m_displayItemPosition == m_context.displayItemList()->newDisplayItemsSize());
+    ASSERT(m_displayItemPosition == m_context.displayItemList()->newDisplayItems().size());
 #endif
 
     m_context.displayItemList()->createAndAppend<DrawingDisplayItem>(m_displayItemClient