Back PaintRecord with PaintOpBuffer instead of SkPicture

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

Index: third_party/WebKit/Source/platform/graphics/paint/CompositingRecorder.cpp
diff --git a/third_party/WebKit/Source/platform/graphics/paint/CompositingRecorder.cpp b/third_party/WebKit/Source/platform/graphics/paint/CompositingRecorder.cpp
index 086733692eb86a7c76ed5890b410747438cbb8c0..242dd51a0d121ca9671ba44e52be8a2a0c3eeff1 100644
--- a/third_party/WebKit/Source/platform/graphics/paint/CompositingRecorder.cpp
+++ b/third_party/WebKit/Source/platform/graphics/paint/CompositingRecorder.cpp
@@ -29,54 +29,8 @@ CompositingRecorder::CompositingRecorder(GraphicsContext& graphicsContext,
 CompositingRecorder::~CompositingRecorder() {
   if (RuntimeEnabledFeatures::slimmingPaintV2Enabled())
     return;
-  // If the end of the current display list is of the form

[chrishtr, 2017/04/07 00:43:47]

Could you point me at where this heuristic is re-implemented?
Is there a unittest for the new behavior?

[enne (OOO), 2017/04/07 00:54:06]

There's not a unit test, but there's PLENTY of layout test and cc pixel test
coverage.  I'm happy to add a directed unit test in this patch if you'd like.

This optimization is replicated in both DisplayItemList::Raster and
PaintOpBuffer::playback (since a save could be either a DisplayItem or a
PaintOp).

[chrishtr, 2017/04/07 16:10:28]

Yes please.
Ah great. Very nice.

-  // [BeginCompositingDisplayItem] [DrawingDisplayItem], then fold the
-  // BeginCompositingDisplayItem into a new DrawingDisplayItem that replaces
-  // them both. This allows Skia to optimize for the case when the
-  // BeginCompositingDisplayItem represents a simple opacity/color that can be
-  // merged into the opacity/color of the drawing. See crbug.com/628831 for more
-  // details.
-  PaintController& paintController = m_graphicsContext.getPaintController();
-  const DisplayItem* lastDisplayItem = paintController.lastDisplayItem(0);
-  const DisplayItem* secondToLastDisplayItem =
-      paintController.lastDisplayItem(1);
-  if (!RuntimeEnabledFeatures::slimmingPaintV2Enabled() && lastDisplayItem &&
-      secondToLastDisplayItem && lastDisplayItem->drawsContent() &&
-      secondToLastDisplayItem->getType() == DisplayItem::kBeginCompositing) {
-    FloatRect cullRect(
-        ((DrawingDisplayItem*)lastDisplayItem)->GetPaintRecord()->cullRect());
-    const DisplayItemClient& displayItemClient = lastDisplayItem->client();
-    DisplayItem::Type displayItemType = lastDisplayItem->getType();
-
-    // Re-record the last two DisplayItems into a new drawing. The new item
-    // cannot be cached, because it is a mutation of the DisplayItem the client
-    // thought it was painting.
-    paintController.beginSkippingCache();
-    {
-#if DCHECK_IS_ON()
-      // In the recorder's scope we remove the last two display items which
-      // are combined into a new drawing.
-      DisableListModificationCheck disabler;
-#endif
-      DrawingRecorder newRecorder(m_graphicsContext, displayItemClient,
-                                  displayItemType, cullRect);
-      DCHECK(!DrawingRecorder::useCachedDrawingIfPossible(
-          m_graphicsContext, displayItemClient, displayItemType));
-
-      secondToLastDisplayItem->replay(m_graphicsContext);
-      lastDisplayItem->replay(m_graphicsContext);
-      EndCompositingDisplayItem(m_client).replay(m_graphicsContext);
-
-      // Remove the DrawingDisplayItem.
-      paintController.removeLastDisplayItem();
-      // Remove the BeginCompositingDisplayItem.
-      paintController.removeLastDisplayItem();
-    }
-    paintController.endSkippingCache();
-  } else {
-    m_graphicsContext.getPaintController().endItem<EndCompositingDisplayItem>(
-        m_client);
-  }
+  m_graphicsContext.getPaintController().endItem<EndCompositingDisplayItem>(
+      m_client);
 }
 
 }  // namespace blink