Leaking CGBitmapContext in SkFontHost_mac

src/ports/SkFontHost_mac.cpp

Index: src/ports/SkFontHost_mac.cpp
diff --git a/src/ports/SkFontHost_mac.cpp b/src/ports/SkFontHost_mac.cpp
index 21a2b956148b8ac53a17d2a3b94498ffdbf51e18..7f3a3c1ef4d86871d8fac7ddf4f8ddf2c640bac7 100755
--- a/src/ports/SkFontHost_mac.cpp
+++ b/src/ports/SkFontHost_mac.cpp
@@ -82,9 +82,10 @@ public:
     ~AutoCFRelease() { CFSafeRelease(fCFRef); }
 
     void reset(CFRef that = NULL) {
-        CFSafeRetain(that);
-        CFSafeRelease(fCFRef);
-        fCFRef = that;
+        if (that != fCFRef) {
+            CFSafeRelease(fCFRef);
+            fCFRef = that;
+        }
     }
 
     AutoCFRelease& operator =(CFRef that) {

[bungeman-skia, 2014/07/14 17:38:30]

I think I actually wrote this, but now I hate this operator= . The issue is that
it is actually a 'move' operator=. If I had c++11 I would write this as
'AutoCFRelease& operator=(CFRef&& that)' so that way one would be able to
appropriately steal references. This is actually all it is used for, so this
proposed change does work. But it is dangerous because with this patch it still
works by accident; there is nothing preventing an lvalue from appearing on the
rhs. 'reset' on the other hand is an explicit rvalue refcount stealing
construct, and in the futre will be easier to convert to a move assignment
operator.

Since we don't have c++11 yet, I'm in favor of removing this method entirely and
replacing all of its uses with calls to 'reset'. However, to get things moving,
I'm going to lgtm this for the moment so it goes in, and then I'll follow up
with removing the operator= .