cc: Don't leak a ref to SkColorFilter.

skia/ext/refptr.h

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef SKIA_EXT_REFPTR_H_
 #define SKIA_EXT_REFPTR_H_
 
+#include "base/basictypes.h"
+#include "base/logging.h"
 #include "third_party/skia/include/core/SkRefCnt.h"
 
 namespace skia {
 
 // When creating/receiving a ref-counted pointer from Skia, wrap that pointer in
 // this class to avoid dealing with the ref-counting and prevent leaks/crashes
 // due to ref-counting bugs.
 //
 // Example of creating a new SkShader* and setting it on a SkPaint:
 //   skia::RefPtr<SkShader> shader = skia::AdoptRef(SkGradientShader::Create());
@@ skipping 62 matching lines @@
 
   T* get() const { return ptr_; }
   T& operator*() const { return *ptr_; }
   T* operator->() const { return ptr_; }
 
   typedef T* RefPtr::*unspecified_bool_type;
   operator unspecified_bool_type() const {
     return ptr_ ? &RefPtr::ptr_ : NULL;
   }
 
+  class Receiver {
+   public:
+    ~Receiver() {
+      DCHECK(!*refptr_) << "RefPtr must be empty.";
+      refptr_->ptr_ = ptr_;
+    }
+
+    operator T**() { return &ptr_; }
+
+   private:
+    friend class RefPtr;
+
+    explicit Receiver(RefPtr* refptr) : refptr_(refptr), ptr_(NULL) {
+      DCHECK(!*refptr_) << "RefPtr must be empty.";
+    }
+
+    RefPtr* refptr_;
+    T* ptr_;
+
+    DISALLOW_COPY_AND_ASSIGN(Receiver);
+  };
+
+  // Call this and pass it to a function that takes a T** argument and
+  // sets it to an object with an unowned reference.

[awong, 2013/07/17 23:51:02]

suggestion:
Call this and pass the returned temporary to a function taking a T** argument.
The returned temporary will set the original RefPtr to an object with an unowned
reference.

+  // NOTE: Do not use this function as part of a subexpression, and try to use

[awong, 2013/07/17 23:51:02]

suggestion:
Do not use this function as part of a subexpression where the RefPtr is also
used somewhere else. Destructors of temporaries are only run at the end of a
full expression so the initialization will not be complete and the RefPtr will
remain empty.

[danakj, 2013/07/17 23:58:29]

Thanks, will do.

I think that I can also DCHECK this to guarantee safety (tho it requires adding
a bool RefPtr and adding DCHECK to a lot of places, so I think only in debug
builds would be appropriate).

+  // the RefPtr in the same subexpression, as this operation will not be
+  // complete and the RefPtr will be empty still.
+  //
+  // Bad (refptr will be NULL when it is dereffed):
+  //   Foo(refptr.ReceiveAndAdoptRef()) && refptr->Bar();
+  // Good:
+  //   bool ok = Foo(refptr.ReceiveAndAdoptRef());
+  //   ok && refptr->Bar();
+  Receiver ReceiveAndAdoptRef() { return Receiver(this); }
+
  private:
   T* ptr_;
 
   // This function cannot be public because Skia starts its ref-counted
   // objects at refcnt=1.  This makes it impossible to differentiate
   // between a newly created object (that doesn't need to be ref'd) or an
   // already existing object with one owner (that does need to be ref'd so that
   // this RefPtr can also manage its lifetime).
   explicit RefPtr(T* ptr) : ptr_(ptr) {}
 
   template<typename U>
   friend RefPtr<U> AdoptRef(U* ptr);
 
   template<typename U>
   friend RefPtr<U> SharePtr(U* ptr);
 };
 
 // For objects that have an unowned reference (such as newly created objects).
 template<typename T>
 RefPtr<T> AdoptRef(T* ptr) { return RefPtr<T>(ptr); }
 
 // For objects that are already owned. This doesn't take ownership of existing
 // references and adds a new one.
 template<typename T>
 RefPtr<T> SharePtr(T* ptr) { return RefPtr<T>(SkSafeRef(ptr)); }
 
 }  // namespace skia
 
 #endif  // SKIA_EXT_REFPTR_H_