Make SkiaCanvas creation call ref() on SkCanvas.

There are two callers of SkiaCanvas::create_canvas(SkCanvas*). In both
cases, we were calling ref() first. It is necessary to call ref() in
both cases, since we have an SkCanvas returned by
SkPictureRecorder::beginRecording, which does not increment the ref
count to account for the caller. (i.e. the SkPictureRecorder has the
only ref until you call ref()).

Rather than leave the ref up to the caller, since the SkiaCanvas does
the unref(), it should also do the ref(). Update it to do so and
document its behavior.

Also, make SkiaCanvas' constructors explicit.

Change-Id: I894d0a71a87587cf8c2b26deb2384dc49ae090ef

4 files changed, 21 insertions, 12 deletions

diff --git a/core/jni/android/graphics/Picture.cpp b/core/jni/android/graphics/Picture.cpp
index d048a29..b0d943e 100644
--- a/core/jni/android/graphics/Picture.cpp
+++ b/core/jni/android/graphics/Picture.cpp
@@ -42,10 +42,6 @@ Canvas* Picture::beginRecording(int width, int height) {
     mWidth = width;
     mHeight = height;
     SkCanvas* canvas = mRecorder->beginRecording(width, height, NULL, 0);
-    // the java side will wrap this guy in a Canvas.java, which will call
-    // unref in its finalizer, so we have to ref it here, so that both that
-    // Canvas.java and our picture can both be owners
-    canvas->ref();
     return Canvas::create_canvas(canvas);
 }
 
diff --git a/core/jni/android/graphics/SkiaCanvas.cpp b/core/jni/android/graphics/SkiaCanvas.cpp
index 793d177..7de54c4 100644
--- a/core/jni/android/graphics/SkiaCanvas.cpp
+++ b/core/jni/android/graphics/SkiaCanvas.cpp
@@ -44,10 +44,18 @@ namespace android {
 // Holds an SkCanvas reference plus additional native data.
 class SkiaCanvas : public Canvas {
 public:
-    SkiaCanvas(SkBitmap* bitmap);
-
-    SkiaCanvas(SkCanvas* canvas) : mCanvas(canvas) {
+    explicit SkiaCanvas(SkBitmap* bitmap);
+
+    /**
+     *  Create a new SkiaCanvas.
+     *
+     *  @param canvas SkCanvas to handle calls made to this SkiaCanvas. Must
+     *      not be NULL. This constructor will ref() the SkCanvas, and unref()
+     *      it in its destructor.
+     */
+    explicit SkiaCanvas(SkCanvas* canvas) : mCanvas(canvas) {
         SkASSERT(canvas);
+        canvas->ref();
     }
 
     virtual SkCanvas* getSkCanvas() {
diff --git a/core/jni/android/graphics/pdf/PdfDocument.cpp b/core/jni/android/graphics/pdf/PdfDocument.cpp
index be90612..8ba164f 100644
--- a/core/jni/android/graphics/pdf/PdfDocument.cpp
+++ b/core/jni/android/graphics/pdf/PdfDocument.cpp
@@ -73,11 +73,6 @@ public:
         SkCanvas* canvas = page->mPictureRecorder->beginRecording(
                 contentRect.width(), contentRect.height(), NULL, 0);
 
-        // We pass this canvas to Java where it is used to construct
-        // a Java Canvas object which dereferences the pointer when it
-        // is destroyed, so we have to bump up the reference count.
-        canvas->ref();
-
         return canvas;
     }
 
diff --git a/include/private/graphics/Canvas.h b/include/private/graphics/Canvas.h
index 3cd57f4..2eda6a4 100644
--- a/include/private/graphics/Canvas.h
+++ b/include/private/graphics/Canvas.h
@@ -28,6 +28,16 @@ public:
     virtual ~Canvas() {};
 
     static Canvas* create_canvas(SkBitmap* bitmap);
+
+    /**
+     *  Create a new Canvas object which delegates to an SkCanvas.
+     *
+     *  @param skiaCanvas Must not be NULL. All drawing calls will be
+     *      delegated to this object. This function will call ref() on the
+     *      SkCanvas, and the returned Canvas will unref() it upon
+     *      destruction.
+     *  @return new Canvas object. Will not return NULL.
+     */
     static Canvas* create_canvas(SkCanvas* skiaCanvas);
 
     // TODO: enable HWUI to either create similar canvas wrapper or subclass