Only use skia::RefPtr for refcounting

skia/ext/refptr.h

Index: skia/ext/refptr.h
diff --git a/skia/ext/refptr.h b/skia/ext/refptr.h
index 514d453e034d69ccd10b96de8bb3a7f8666a93c0..1f80de3743f4e8bf0fe81aed2aae9d495a512a5f 100644
--- a/skia/ext/refptr.h
+++ b/skia/ext/refptr.h
@@ -13,7 +13,7 @@ namespace skia {
 // this class to avoid dealing with the ref-counting and prevent leaks/crashes
 // due to ref-counting bugs.
 //
-// Example of Creating an SkShader* and setting it on a SkPaint:
+// Example of creating a new SkShader* and setting it on a SkPaint:
 //   skia::RefPtr<SkShader> shader = skia::AdoptRef(SkGradientShader::Create());
 //   paint.setShader(shader.get());
 //
@@ -25,12 +25,16 @@ namespace skia {
 //   }
 //   skia::RefPtr<SkShader> member_refptr_;
 //
-// When returning a ref-counted ponter, also return the skia::RefPtr instead. An
-// example method that creates an SkShader* and returns it:
+// When returning a ref-counted pointer, also return the skia::RefPtr instead.
+// An example method that creates an SkShader* and returns it:
 //   skia::RefPtr<SkShader> MakeAShader() {
 //     return skia::AdoptRef(SkGradientShader::Create());
 //   }
 //
+// If a Skia API returns a raw pointer that already has an owner, then use

[vandebo (ex-Chrome), 2013/05/17 20:47:49]

The object under consideration is reference counted, it does not have a single
owner.  ShareRef does not enable sharing of references, it creates a new
reference.  Suggestion:

To take a scoped reference to an object which does not have a unowned reference
i.e. has all its references owned by other objects, use the "ShareRef" helper.

[enne (OOO), 2013/05/17 21:37:02]

Incorporated most of that wording.

+// the ShareRef helper to have a RefPtr share that ref with the other owner.
+//   skia::RefPtr<SkShader> shader = skia::ShareRef(paint.getShader());
+//
 // Never call ref() or unref() on the underlying ref-counted pointer. If you
 // AdoptRef() the raw pointer immediately into a skia::RefPtr and always work
 // with skia::RefPtr instances instead, the ref-counting will be taken care of
@@ -84,15 +88,28 @@ class RefPtr {
  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 to be ref'd so that

[vandebo (ex-Chrome), 2013/05/17 20:47:49]

typo: ...that does *need* to be ref'd...

[enne (OOO), 2013/05/17 21:37:02]

Done.

+  // 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> ShareRef(U* ptr);
 };
 
+// For newly created raw pointers without any owners.

[vandebo (ex-Chrome), 2013/05/17 20:47:49]

Technically the object doesn't need to be newly created.  It can be the result
of a function that returns a new reference.  Suggestion: For objects that have
an unowned reference (like newly create objects).

[enne (OOO), 2013/05/17 21:37:02]

Done.

 template<typename T>
 RefPtr<T> AdoptRef(T* ptr) { return RefPtr<T>(ptr); }
 
+// For pointers already owned. Doesn't take ownership of existing references.

[vandebo (ex-Chrome), 2013/05/17 20:47:49]

Suggestion: For objects that do not have unowned references (will ref the
object).

[enne (OOO), 2013/05/17 21:37:02]

Incorporated that wording into this comment.

+template<typename T>
+RefPtr<T> ShareRef(T* ptr) { return RefPtr<T>(SkSafeRef(ptr)); }
+
 }  // namespace skia
 
 #endif  // SKIA_EXT_REFPTR_H_