Leaking CGBitmapContext in SkFontHost_mac

src/ports/SkFontHost_mac.cpp

 
 /*
  * Copyright 2006 The Android Open Source Project
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include <vector>
 #ifdef SK_BUILD_FOR_MAC
@@ skipping 64 matching lines @@
     }
 }
 
 /** Acts like a CFRef, but calls CFSafeRelease when it goes out of scope. */
 template<typename CFRef> class AutoCFRelease : private SkNoncopyable {
 public:
     explicit AutoCFRelease(CFRef cfRef = NULL) : fCFRef(cfRef) { }
     ~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= .

         reset(that);
         return *this;
     }
 
     operator CFRef() const { return fCFRef; }
     CFRef get() const { return fCFRef; }
 
     CFRef* operator&() { SkASSERT(fCFRef == NULL); return &fCFRef; }
 private:
     CFRef fCFRef;
@@ skipping 2207 matching lines @@
         return create_typeface(NULL, familyName, (SkTypeface::Style)styleBits);
     }
 };
 
 ///////////////////////////////////////////////////////////////////////////////
 
 SkFontMgr* SkFontMgr::Factory() {
     return SkNEW(SkFontMgr_Mac);
 }
 #endif