Add skia::RefPtr class to wrap ref counted classes from Skia.

skia/ext/refptr.h

Index: skia/ext/refptr.h
diff --git a/skia/ext/refptr.h b/skia/ext/refptr.h
new file mode 100644
index 0000000000000000000000000000000000000000..598f6d783caf526399def95476d8af63ba8f7559
--- /dev/null
+++ b/skia/ext/refptr.h
@@ -0,0 +1,75 @@
+// 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 "third_party/skia/include/core/SkRefCnt.h"
+
+namespace skia {
+
+template<typename T>

[jamesr, 2012/11/29 07:34:06]

could you add some class-level comments indicating how this is supposed to be
used, perhaps with some example code?

[danakj, 2012/11/29 21:37:11]

Done.

+class RefPtr {
+ public:
+  RefPtr()
+      : ptr_(NULL) {
+  }
+
+  RefPtr(const RefPtr& other)
+      : ptr_(other.ptr_) {
+    SkSafeRef(ptr_);
+  }
+
+  // Skia objects come with a reference of 1, steal that reference here to
+  // take ownership.
+  RefPtr(T* ptr) : ptr_(ptr) {}

[Stephen White, 2012/11/29 18:42:04]

Out of curiosity, why did you decide on a public constructor for this one? 
Wouldn't it be better to have an explicit RefPtr::Adopt(), a la WebKit?

I like Adopt() because it follows the "making wrong code look wrong" principle: 
if I see a "new Foo" or Foo::Create() without an Adopt, my spidey sense tingles.
 The current implementation also seems like it would make problems like this
harder to spot:

skia::RefPtr<T> foo(otherRefPtr.get());

instead of

skia::RefPtr<T> foo(otherRefPtr);

[danakj, 2012/11/29 21:37:11]

I was having difficulty making an Adopt() method that would deal well with
superclasses.

// This wouldn't compile because no implicit upcasting.
skia::RefPtr<SkShader> s = skia::Adopt(new SkSpecializedShader);

So I ended up with way too much typing:
skia::RefPtr<SkShader> s = skia::RefPtr<SkShader>::Adopt(new
SkSpecializedShader);

Almost 80 cols of boilerplate, which I found hard to justify. So I went with a
constructor.

However, jamesr's comment on PassAs() here points out a great way to solve this.

+
+  ~RefPtr() {
+    clear();
+  }
+
+  // Skia objects come with a reference of 1, steal that reference here to
+  // take ownership.

[jamesr, 2012/11/29 07:34:06]

this comment's repeated  and i can't really tell what it means here

[danakj, 2012/11/29 21:37:11]

This, and the constructor, take a raw skia pointer, and steal that reference to
take ownership of the raw pointer.

Did you miss the T* parameter here, or is there something I can do to word this
better?

[danakj, 2012/11/29 21:38:02]

Actually with AdoptRef() this problem goes away :)

+  RefPtr& operator=(T* ptr) {
+    clear();
+    ptr_ = ptr;
+    return *this;
+  }
+
+  RefPtr& operator=(const RefPtr& other) {
+    SkRefCnt_SafeAssign(ptr_, other.ptr_);
+    return *this;
+  }
+
+  void clear() {
+    T* to_unref = ptr_;
+    ptr_ = NULL;
+    SkSafeUnref(to_unref);
+  }
+
+  // Convert the RefPtr<T> into a RefPtr<P>.

[jamesr, 2012/11/29 07:34:06]

could you just make operator= accept RefPtr<U>s and then rely on
SkRefCnt_SafeAssign() compiling to make sure the types are convertable?

[danakj, 2012/11/29 21:37:11]

Yaaaaa!! thanks!

+  template<typename P>
+  RefPtr<P> PassAs() {
+    // Moves ownership to the returned RefPtr.
+    RefPtr<P> as = ptr_;
+    ptr_ = NULL;
+    return as;
+  }
+
+  T* get() const { return ptr_; }
+  T& operator*() const { return static_cast<T&>(*ptr_); }
+  T* operator->() const { return static_cast<T*>(ptr_); }
+
+  typedef T* RefPtr::*unspecified_bool_type;
+  operator unspecified_bool_type() const {
+    return ptr_ ? &RefPtr::ptr_ : NULL;
+  }
+
+ private:
+  T* ptr_;
+};
+
+}  // namespace skia
+
+#endif  // SKIA_EXT_REFPTR_H_