From 18981294a58c411a4a33ebb22caf9c75ba639691 Mon Sep 17 00:00:00 2001
From: Leon Scroggins III <scroggo@google.com>
Date: Wed, 17 Dec 2014 11:30:31 -0500
Subject: [PATCH] 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
---
 core/jni/android/graphics/Picture.cpp         |  4 ----
 core/jni/android/graphics/SkiaCanvas.cpp      | 12 ++++++++++--
 core/jni/android/graphics/pdf/PdfDocument.cpp |  5 -----
 include/private/graphics/Canvas.h             | 10 ++++++++++
 4 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/core/jni/android/graphics/Picture.cpp b/core/jni/android/graphics/Picture.cpp
index d048a29a35a89..b0d943e371483 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 793d177d772e2..7de54c4e991f1 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);
+    explicit SkiaCanvas(SkBitmap* bitmap);
 
-    SkiaCanvas(SkCanvas* canvas) : mCanvas(canvas) {
+    /**
+     *  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 be906124dd467..8ba164f86c7bc 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 3cd57f4d1110d..2eda6a42981d9 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