Change SkCanvas to *not* inherit from SkRefCnt

Definitely tricky for classes like SkNWayCanvas, where the caller (today)
need not pay attention to ownership of the canvases it gave the NWay
(after this CL, the caller *must* managed ownership)

BUG=skia:

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=4441

DOCS_PREVIEW= https://skia.org/?cl=4441

Change-Id: Ib1ac07a3cdf0686d78e7aaa4735d45cc90bea081
Reviewed-on: https://skia-review.googlesource.com/4441
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>
Reviewed-by: Robert Phillips <robertphillips@google.com>

14 files changed, 44 insertions, 46 deletions

diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index cd4dcbc2a5..a8d73a982a 100644
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -19,6 +19,7 @@
 #include "SkImageFilter.h"
 #include "SkImageFilterCache.h"
 #include "SkLatticeIter.h"
+#include "SkMakeUnique.h"
 #include "SkMatrixUtils.h"
 #include "SkMetaData.h"
 #include "SkNx.h"
@@ -3323,7 +3324,8 @@ static bool supported_for_raster_canvas(const SkImageInfo& info) {
     return true;
 }
 
-SkCanvas* SkCanvas::NewRasterDirect(const SkImageInfo& info, void* pixels, size_t rowBytes) {
+std::unique_ptr<SkCanvas> SkCanvas::MakeRasterDirect(const SkImageInfo& info, void* pixels,
+                                                     size_t rowBytes) {
     if (!supported_for_raster_canvas(info)) {
         return nullptr;
     }
@@ -3332,7 +3334,7 @@ SkCanvas* SkCanvas::NewRasterDirect(const SkImageInfo& info, void* pixels, size_
     if (!bitmap.installPixels(info, pixels, rowBytes)) {
         return nullptr;
     }
-    return new SkCanvas(bitmap);
+    return skstd::make_unique<SkCanvas>(bitmap);
 }
 
 ///////////////////////////////////////////////////////////////////////////////
diff --git a/src/core/SkConfig8888.cpp b/src/core/SkConfig8888.cpp
index 639a444181..fd945cff12 100644
--- a/src/core/SkConfig8888.cpp
+++ b/src/core/SkConfig8888.cpp
@@ -354,8 +354,8 @@ bool SkPixelInfo::CopyPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t
         if (!bm.installPixels(srcInfo, const_cast<void*>(srcPixels), srcRB, ctable, nullptr, nullptr)) {
             return false;
         }
-        sk_sp<SkCanvas> canvas(SkCanvas::NewRasterDirect(dstInfo, dstPixels, dstRB));
-        if (nullptr == canvas.get()) {
+        std::unique_ptr<SkCanvas> canvas = SkCanvas::MakeRasterDirect(dstInfo, dstPixels, dstRB);
+        if (!canvas) {
             return false;
         }
 
diff --git a/src/core/SkMultiPictureDraw.cpp b/src/core/SkMultiPictureDraw.cpp
index b3c6368767..1df27ff4cc 100644
--- a/src/core/SkMultiPictureDraw.cpp
+++ b/src/core/SkMultiPictureDraw.cpp
@@ -18,7 +18,7 @@ void SkMultiPictureDraw::DrawData::draw() {
 void SkMultiPictureDraw::DrawData::init(SkCanvas* canvas, const SkPicture* picture,
                                         const SkMatrix* matrix, const SkPaint* paint) {
     fPicture = SkRef(picture);
-    fCanvas = SkRef(canvas);
+    fCanvas = canvas;
     if (matrix) {
         fMatrix = *matrix;
     } else {
@@ -34,7 +34,6 @@ void SkMultiPictureDraw::DrawData::init(SkCanvas* canvas, const SkPicture* pictu
 void SkMultiPictureDraw::DrawData::Reset(SkTDArray<DrawData>& data) {
     for (int i = 0; i < data.count(); ++i) {
         data[i].fPicture->unref();
-        data[i].fCanvas->unref();
         delete data[i].fPaint;
     }
     data.rewind();
diff --git a/src/core/SkSpecialSurface.cpp b/src/core/SkSpecialSurface.cpp
index beba9150c4..b490421aa8 100644
--- a/src/core/SkSpecialSurface.cpp
+++ b/src/core/SkSpecialSurface.cpp
@@ -29,7 +29,7 @@ public:
     virtual sk_sp<SkSpecialImage> onMakeImageSnapshot() = 0;
 
 protected:
-    sk_sp<SkCanvas> fCanvas;   // initialized by derived classes in ctors
+    std::unique_ptr<SkCanvas> fCanvas;   // initialized by derived classes in ctors
 
 private:
     typedef SkSpecialSurface INHERITED;
diff --git a/src/image/SkSurface.cpp b/src/image/SkSurface.cpp
index 38bab9e32f..3d6670f169 100644
--- a/src/image/SkSurface.cpp
+++ b/src/image/SkSurface.cpp
@@ -58,14 +58,12 @@ SkSurfaceProps::SkSurfaceProps(const SkSurfaceProps& other)
 SkSurface_Base::SkSurface_Base(int width, int height, const SkSurfaceProps* props)
     : INHERITED(width, height, props)
 {
-    fCachedCanvas = nullptr;
     fCachedImage = nullptr;
 }
 
 SkSurface_Base::SkSurface_Base(const SkImageInfo& info, const SkSurfaceProps* props)
     : INHERITED(info, props)
 {
-    fCachedCanvas = nullptr;
     fCachedImage = nullptr;
 }
 
@@ -76,7 +74,6 @@ SkSurface_Base::~SkSurface_Base() {
     }
 
     SkSafeUnref(fCachedImage);
-    SkSafeUnref(fCachedCanvas);
 }
 
 void SkSurface_Base::onDraw(SkCanvas* canvas, SkScalar x, SkScalar y, const SkPaint* paint) {
diff --git a/src/image/SkSurface_Base.h b/src/image/SkSurface_Base.h
index 8351bb87e6..a8c1d8f0a4 100644
--- a/src/image/SkSurface_Base.h
+++ b/src/image/SkSurface_Base.h
@@ -89,8 +89,8 @@ public:
     uint32_t newGenerationID();
 
 private:
-    SkCanvas*   fCachedCanvas;
-    SkImage*    fCachedImage;
+    std::unique_ptr<SkCanvas>   fCachedCanvas;
+    SkImage*                    fCachedImage;
 
     void aboutToDraw(ContentChangeMode mode);
 
@@ -106,12 +106,12 @@ private:
 
 SkCanvas* SkSurface_Base::getCachedCanvas() {
     if (nullptr == fCachedCanvas) {
-        fCachedCanvas = this->onNewCanvas();
+        fCachedCanvas = std::unique_ptr<SkCanvas>(this->onNewCanvas());
         if (fCachedCanvas) {
             fCachedCanvas->setSurfaceBase(this);
         }
     }
-    return fCachedCanvas;
+    return fCachedCanvas.get();
 }
 
 sk_sp<SkImage> SkSurface_Base::refCachedImage(SkBudgeted budgeted, ForceUnique unique) {
diff --git a/src/pdf/SkPDFDocument.cpp b/src/pdf/SkPDFDocument.cpp
index ab5f465971..92e82fc018 100644
--- a/src/pdf/SkPDFDocument.cpp
+++ b/src/pdf/SkPDFDocument.cpp
@@ -217,7 +217,7 @@ SkCanvas* SkPDFDocument::onBeginPage(SkScalar width, SkScalar height,
             SkScalarRoundToInt(width), SkScalarRoundToInt(height));
     fPageDevice.reset(
             SkPDFDevice::Create(pageSize, fRasterDpi, this));
-    fCanvas = sk_make_sp<SkPDFCanvas>(fPageDevice);
+    fCanvas.reset(new SkPDFCanvas(fPageDevice));
     fCanvas->clipRect(trimBox);
     fCanvas->translate(trimBox.x(), trimBox.y());
     return fCanvas.get();
diff --git a/src/pdf/SkPDFDocument.h b/src/pdf/SkPDFDocument.h
index b62a7a59a2..15e147961d 100644
--- a/src/pdf/SkPDFDocument.h
+++ b/src/pdf/SkPDFDocument.h
@@ -76,7 +76,7 @@ private:
     SkTHashSet<SkPDFFont*> fFonts;
     sk_sp<SkPDFDict> fDests;
     sk_sp<SkPDFDevice> fPageDevice;
-    sk_sp<SkCanvas> fCanvas;
+    std::unique_ptr<SkCanvas> fCanvas;
     sk_sp<SkPDFObject> fID;
     sk_sp<SkPDFObject> fXMP;
     SkScalar fRasterDpi;
diff --git a/src/svg/SkSVGCanvas.cpp b/src/svg/SkSVGCanvas.cpp
index d3511c031f..95a4625417 100644
--- a/src/svg/SkSVGCanvas.cpp
+++ b/src/svg/SkSVGCanvas.cpp
@@ -7,11 +7,12 @@
 
 #include "SkSVGCanvas.h"
 #include "SkSVGDevice.h"
+#include "SkMakeUnique.h"
 
-SkCanvas* SkSVGCanvas::Create(const SkRect& bounds, SkXMLWriter* writer) {
+std::unique_ptr<SkCanvas> SkSVGCanvas::Make(const SkRect& bounds, SkXMLWriter* writer) {
     // TODO: pass full bounds to the device
     SkISize size = bounds.roundOut().size();
     sk_sp<SkBaseDevice> device(SkSVGDevice::Create(size, writer));
 
-    return new SkCanvas(device.get());
+    return skstd::make_unique<SkCanvas>(device.get());
 }
diff --git a/src/utils/SkCanvasStateUtils.cpp b/src/utils/SkCanvasStateUtils.cpp
index 062dc13ed9..58f26b6c79 100644
--- a/src/utils/SkCanvasStateUtils.cpp
+++ b/src/utils/SkCanvasStateUtils.cpp
@@ -100,14 +100,12 @@ class SkCanvasState_v1 : public SkCanvasState {
 public:
     static const int32_t kVersion = 1;
 
-    SkCanvasState_v1(SkCanvas* canvas)
-    : INHERITED(kVersion, canvas)
-    {
+    SkCanvasState_v1(SkCanvas* canvas) : INHERITED(kVersion, canvas) {
         layerCount = 0;
         layers = nullptr;
         mcState.clipRectCount = 0;
         mcState.clipRects = nullptr;
-        originalCanvas = SkRef(canvas);
+        originalCanvas = canvas;
     }
 
     ~SkCanvasState_v1() {
@@ -118,10 +116,6 @@ public:
 
         sk_free(mcState.clipRects);
         sk_free(layers);
-
-        // it is now safe to free the canvas since there should be no remaining
-        // references to the content that is referenced by this canvas (e.g. pixels)
-        originalCanvas->unref();
     }
 
     SkMCState mcState;
@@ -284,7 +278,8 @@ static void setup_canvas_from_MC_state(const SkMCState& state, SkCanvas* canvas)
     canvas->clipRegion(clip, SkCanvas::kReplace_Op);
 }
 
-static SkCanvas* create_canvas_from_canvas_layer(const SkCanvasLayerState& layerState) {
+static std::unique_ptr<SkCanvas>
+make_canvas_from_canvas_layer(const SkCanvasLayerState& layerState) {
     SkASSERT(kRaster_CanvasBackend == layerState.type);
 
     SkBitmap bitmap;
@@ -304,15 +299,15 @@ static SkCanvas* create_canvas_from_canvas_layer(const SkCanvasLayerState& layer
     SkASSERT(!bitmap.empty());
     SkASSERT(!bitmap.isNull());
 
-    sk_sp<SkCanvas> canvas(new SkCanvas(bitmap));
+    std::unique_ptr<SkCanvas> canvas(new SkCanvas(bitmap));
 
     // setup the matrix and clip
     setup_canvas_from_MC_state(layerState.mcState, canvas.get());
 
-    return canvas.release();
+    return canvas;
 }
 
-SkCanvas* SkCanvasStateUtils::CreateFromCanvasState(const SkCanvasState* state) {
+std::unique_ptr<SkCanvas> SkCanvasStateUtils::MakeFromCanvasState(const SkCanvasState* state) {
     SkASSERT(state);
     // Currently there is only one possible version.
     SkASSERT(SkCanvasState_v1::kVersion == state->version);
@@ -323,14 +318,14 @@ SkCanvas* SkCanvasStateUtils::CreateFromCanvasState(const SkCanvasState* state)
         return nullptr;
     }
 
-    sk_sp<SkCanvasStack> canvas(new SkCanvasStack(state->width, state->height));
+    std::unique_ptr<SkCanvasStack> canvas(new SkCanvasStack(state->width, state->height));
 
     // setup the matrix and clip on the n-way canvas
     setup_canvas_from_MC_state(state_v1->mcState, canvas.get());
 
     // Iterate over the layers and add them to the n-way canvas
     for (int i = state_v1->layerCount - 1; i >= 0; --i) {
-        sk_sp<SkCanvas> canvasLayer(create_canvas_from_canvas_layer(state_v1->layers[i]));
+        std::unique_ptr<SkCanvas> canvasLayer = make_canvas_from_canvas_layer(state_v1->layers[i]);
         if (!canvasLayer.get()) {
             return nullptr;
         }
@@ -338,7 +333,7 @@ SkCanvas* SkCanvasStateUtils::CreateFromCanvasState(const SkCanvasState* state)
                                                              state_v1->layers[i].y));
     }
 
-    return canvas.release();
+    return std::move(canvas);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
diff --git a/src/utils/SkLua.cpp b/src/utils/SkLua.cpp
index a2234565cb..27127df6a8 100644
--- a/src/utils/SkLua.cpp
+++ b/src/utils/SkLua.cpp
@@ -73,6 +73,13 @@ template <typename T> void push_obj(lua_State* L, const T& obj) {
     lua_setmetatable(L, -2);
 }
 
+template <typename T> T* push_ptr(lua_State* L, T* ptr) {
+    *(T**)lua_newuserdata(L, sizeof(T*)) = ptr;
+    luaL_getmetatable(L, get_mtname<T>());
+    lua_setmetatable(L, -2);
+    return ptr;
+}
+
 template <typename T> T* push_ref(lua_State* L, T* ref) {
     *(T**)lua_newuserdata(L, sizeof(T*)) = SkSafeRef(ref);
     luaL_getmetatable(L, get_mtname<T>());
@@ -333,7 +340,7 @@ void SkLua::pushPath(const SkPath& path, const char key[]) {
 }
 
 void SkLua::pushCanvas(SkCanvas* canvas, const char key[]) {
-    push_ref(fL, canvas);
+    push_ptr(fL, canvas);
     CHECK_SETFIELD(key);
 }
 
@@ -715,7 +722,7 @@ static int lcanvas_newSurface(lua_State* L) {
 }
 
 static int lcanvas_gc(lua_State* L) {
-    get_ref<SkCanvas>(L, 1)->unref();
+    // don't know how to track a ptr...
     return 0;
 }
 
@@ -757,7 +764,7 @@ const struct luaL_Reg gSkCanvas_Methods[] = {
 
 static int ldocument_beginPage(lua_State* L) {
     const SkRect* contentPtr = nullptr;
-    push_ref(L, get_ref<SkDocument>(L, 1)->beginPage(lua2scalar(L, 2),
+    push_ptr(L, get_ref<SkDocument>(L, 1)->beginPage(lua2scalar(L, 2),
                                                      lua2scalar(L, 3),
                                                      contentPtr));
     return 1;
@@ -1750,7 +1757,7 @@ static int lsurface_getCanvas(lua_State* L) {
     if (nullptr == canvas) {
         lua_pushnil(L);
     } else {
-        push_ref(L, canvas);
+        push_ptr(L, canvas);
         // note: we don't unref canvas, since getCanvas did not ref it.
         // warning: this is weird: now Lua owns a ref on this canvas, but what if they let
         // the real owner (the surface) go away, but still hold onto the canvas?
@@ -1814,7 +1821,7 @@ static int lpicturerecorder_beginRecording(lua_State* L) {
         return 1;
     }
 
-    push_ref(L, canvas);
+    push_ptr(L, canvas);
     return 1;
 }
 
@@ -1824,7 +1831,7 @@ static int lpicturerecorder_getCanvas(lua_State* L) {
         lua_pushnil(L);
         return 1;
     }
-    push_ref(L, canvas);
+    push_ptr(L, canvas);
     return 1;
 }
 
diff --git a/src/utils/SkNWayCanvas.cpp b/src/utils/SkNWayCanvas.cpp
index b2d71d231b..a910284da0 100644
--- a/src/utils/SkNWayCanvas.cpp
+++ b/src/utils/SkNWayCanvas.cpp
@@ -15,7 +15,6 @@ SkNWayCanvas::~SkNWayCanvas() {
 
 void SkNWayCanvas::addCanvas(SkCanvas* canvas) {
     if (canvas) {
-        canvas->ref();
         *fList.append() = canvas;
     }
 }
@@ -23,13 +22,11 @@ void SkNWayCanvas::addCanvas(SkCanvas* canvas) {
 void SkNWayCanvas::removeCanvas(SkCanvas* canvas) {
     int index = fList.find(canvas);
     if (index >= 0) {
-        canvas->unref();
         fList.removeShuffle(index);
     }
 }
 
 void SkNWayCanvas::removeAll() {
-    fList.unrefAll();
     fList.reset();
 }
 
diff --git a/src/utils/SkNullCanvas.cpp b/src/utils/SkNullCanvas.cpp
index b5ee8d30a1..1f28706e20 100644
--- a/src/utils/SkNullCanvas.cpp
+++ b/src/utils/SkNullCanvas.cpp
@@ -9,10 +9,10 @@
 
 #include "SkCanvas.h"
 #include "SkNWayCanvas.h"
+#include "SkMakeUnique.h"
 
-
-SkCanvas* SkCreateNullCanvas() {
+std::unique_ptr<SkCanvas> SkMakeNullCanvas() {
     // An N-Way canvas forwards calls to N canvas's. When N == 0 it's
     // effectively a null canvas.
-    return new SkNWayCanvas(0, 0);
+    return std::unique_ptr<SkCanvas>(new SkNWayCanvas(0, 0));
 }
diff --git a/src/xps/SkDocument_XPS.cpp b/src/xps/SkDocument_XPS.cpp
index e333b5adff..d05764a7bf 100644
--- a/src/xps/SkDocument_XPS.cpp
+++ b/src/xps/SkDocument_XPS.cpp
@@ -58,7 +58,7 @@ protected:
 
 private:
     SkXPSDevice fDevice;
-    sk_sp<SkCanvas> fCanvas;
+    std::unique_ptr<SkCanvas> fCanvas;
     SkVector fUnitsPerMeter;
     SkVector fPixelsPerMeter;
 };