Clean up cc/paint interfaces

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

Index: third_party/WebKit/Source/platform/graphics/paint/DrawingDisplayItem.cpp
diff --git a/third_party/WebKit/Source/platform/graphics/paint/DrawingDisplayItem.cpp b/third_party/WebKit/Source/platform/graphics/paint/DrawingDisplayItem.cpp
index c319de5e2da6716fb75b8ca5a9bf571e36b09629..a3279f3c8e0fa7a21a35efab264d4cd3d73c51ce 100644
--- a/third_party/WebKit/Source/platform/graphics/paint/DrawingDisplayItem.cpp
+++ b/third_party/WebKit/Source/platform/graphics/paint/DrawingDisplayItem.cpp
@@ -16,7 +16,7 @@ namespace blink {
 
 void DrawingDisplayItem::replay(GraphicsContext& context) const {
   if (m_record)
-    context.drawRecord(m_record.get());
+    context.drawRecord(m_record);
 }
 
 void DrawingDisplayItem::appendToWebDisplayItemList(
@@ -53,8 +53,8 @@ void DrawingDisplayItem::dumpPropertiesAsDebugString(
 }
 #endif
 
-static bool recordsEqual(const PaintRecord* record1,
-                         const PaintRecord* record2) {
+static bool recordsEqual(sk_sp<const PaintRecord> record1,

[danakj, 2017/03/16 20:06:16]

This seems like it should be T* still? ToSkPicture isn't keeping ownership and
we're not storing it's result just using it as a temporary

[enne (OOO), 2017/03/16 20:57:26]

Agreed here.

+                         sk_sp<const PaintRecord> record2) {
   if (record1->approximateOpCount() != record2->approximateOpCount())
     return false;
 
@@ -64,7 +64,7 @@ static bool recordsEqual(const PaintRecord* record1,
   return data1->equals(data2.get());
 }
 
-static SkBitmap recordToBitmap(const PaintRecord* record) {
+static SkBitmap recordToBitmap(sk_sp<const PaintRecord> record) {
   SkBitmap bitmap;
   SkRect rect = record->cullRect();
   bitmap.allocPixels(SkImageInfo::MakeN32Premul(rect.width(), rect.height()));
@@ -75,8 +75,8 @@ static SkBitmap recordToBitmap(const PaintRecord* record) {
   return bitmap;
 }
 
-static bool bitmapsEqual(const PaintRecord* record1,
-                         const PaintRecord* record2) {
+static bool bitmapsEqual(sk_sp<const PaintRecord> record1,

[danakj, 2017/03/16 20:06:17]

This seems like it should be T* also, it's not passing the ownership anywhere

[enne (OOO), 2017/03/16 20:57:26]

Not agreed here.  This is rasterizing via drawPicture which in this patch takes
an sk_sp.  Sure, the SkiaPaintCanvas doesn't take ownership of the PaintRecord
when rasterizing, but the recording version does, and at the PaintCanvas
interface level, I think you don't know that?

[danakj, 2017/03/16 21:02:01]

Oh ok I missed what recordToBitmap does.

+                         sk_sp<const PaintRecord> record2) {
   SkRect rect = record1->cullRect();
   if (rect != record2->cullRect())
     return false;
@@ -107,8 +107,8 @@ bool DrawingDisplayItem::equals(const DisplayItem& other) const {
   if (!DisplayItem::equals(other))
     return false;
 
-  const PaintRecord* record = this->GetPaintRecord();
-  const PaintRecord* otherRecord =
+  sk_sp<const PaintRecord> record = this->GetPaintRecord();

[danakj, 2017/03/16 20:06:16]

const& probably?

+  sk_sp<const PaintRecord> otherRecord =
       static_cast<const DrawingDisplayItem&>(other).GetPaintRecord();
 
   if (!record && !otherRecord)