Followup to bug 661973: Fix bug with COM outparams and add convenience operators. r=cjones
authorBas Schouten <bas.schouten@live.nl>
Tue, 21 Jun 2011 21:44:00 -0700
changeset 72021 ca61a0de54f1db83a6c22844386474d9bf4fd342
parent 72020 c461448acd96b3b38a62961e5444a8484d32693c
child 72022 b6e018a94db7b7b5b775d364d7c7be6ac10581e8
push idunknown
push userunknown
push dateunknown
reviewerscjones
bugs661973
milestone7.0a1
Followup to bug 661973: Fix bug with COM outparams and add convenience operators. r=cjones
mfbt/RefPtr.h
--- a/mfbt/RefPtr.h
+++ b/mfbt/RefPtr.h
@@ -126,16 +126,18 @@ private:
  */
 template<typename T>
 class RefPtr
 {
     // To allow them to use unref()
     friend class TemporaryRef<T>;
     friend class OutParamRef<T>;
 
+    struct dontRef {};
+
 public:
     RefPtr() : ptr(0) { }
     RefPtr(const RefPtr& o) : ptr(ref(o.ptr)) {}
     RefPtr(const TemporaryRef<T>& o) : ptr(o.drop()) {}
     RefPtr(T* t) : ptr(ref(t)) {}
 
     template<typename U>
     RefPtr(const RefPtr<U>& o) : ptr(ref(o.get())) {}
@@ -159,23 +161,25 @@ public:
     RefPtr& operator=(const RefPtr<U>& o) {
         assign(ref(o.get()));
         return *this;
     }
 
     TemporaryRef<T> forget() {
         T* tmp = ptr;
         ptr = 0;
-        return TemporaryRef<T>(tmp);
+        return TemporaryRef<T>(tmp, dontRef());
     }
 
     T* get() const { return ptr; }
     operator T*() const { return ptr; }
     T* operator->() const { return ptr; }
     T& operator*() const { return *ptr; }
+    template<typename U>
+    operator TemporaryRef<U>() { return forget(); }
 
 private:
     void assign(T* t) {
         unref(ptr);
         ptr = t;
     }
 
     T* ptr;
@@ -201,70 +205,82 @@ private:
  * references to other objects, or unref on destruction.
  */
 template<typename T>
 class TemporaryRef
 {
     // To allow it to construct TemporaryRef from a bare T*
     friend class RefPtr<T>;
 
+    typedef typename RefPtr<T>::dontRef dontRef;
+
 public:
+    TemporaryRef(T* t) : ptr(RefPtr<T>::ref(t)) {}
     TemporaryRef(const TemporaryRef& o) : ptr(o.drop()) {}
 
     template<typename U>
     TemporaryRef(const TemporaryRef<U>& o) : ptr(o.drop()) {}
 
     ~TemporaryRef() { RefPtr<T>::unref(ptr); }
 
     T* drop() const {
         T* tmp = ptr;
         ptr = 0;
         return tmp;
     }
 
 private:
-    TemporaryRef(T* t) : ptr(t) {}
+    TemporaryRef(T* t, const dontRef&) : ptr(t) {}
 
     mutable T* ptr;
 
     TemporaryRef();
     TemporaryRef& operator=(const TemporaryRef&);
 };
 
 /**
  * OutParamRef is a wrapper that tracks a refcounted pointer passed as
- * an outparam argument to a function.  If OutParamRef holds a ref to
- * an object that's reassigned during a function call in which the
- * OutParamRef is an outparam, then the old object is unref'd and the
- * new object is ref'd.
+ * an outparam argument to a function.  OutParamRef implements COM T**
+ * outparam semantics: this requires the callee to AddRef() the T*
+ * returned through the T** outparam on behalf of the caller.  This
+ * means the caller (through OutParamRef) must Release() the old
+ * object contained in the tracked RefPtr.  It's OK if the callee
+ * returns the same T* passed to it through the T** outparam, as long
+ * as the callee obeys the COM discipline.
  *
  * Prefer returning TemporaryRef<T> from functions over creating T**
  * outparams and passing OutParamRef<T> to T**.  Prefer RefPtr<T>*
  * outparams over T** outparams.
  */
 template<typename T>
 class OutParamRef
 {
     friend OutParamRef byRef<T>(RefPtr<T>&);
 
 public:
-    ~OutParamRef() { refPtr = tmp; }
+    ~OutParamRef() {
+        RefPtr<T>::unref(refPtr.ptr);
+        refPtr.ptr = tmp;
+    }
 
     operator T**() { return &tmp; }
 
 private:
     OutParamRef(RefPtr<T>& p) : refPtr(p), tmp(p.get()) {}
 
     RefPtr<T>& refPtr;
     T* tmp;
 
     OutParamRef();
     OutParamRef& operator=(const OutParamRef&);
 };
 
+/**
+ * byRef cooperates with OutParamRef to implement COM outparam semantics.
+ */
 template<typename T>
 OutParamRef<T>
 byRef(RefPtr<T>& ptr)
 {
     return OutParamRef<T>(ptr);
 }
 
 } // namespace mozilla
@@ -294,47 +310,56 @@ struct Foo : public RefCounted<Foo>
 };
 int Foo::numDestroyed;
 
 struct Bar : public Foo { };
 
 TemporaryRef<Foo>
 NewFoo()
 {
-    RefPtr<Foo> f = new Foo();
-    return f.forget();
+    return RefPtr<Foo>(new Foo());
 }
 
 TemporaryRef<Foo>
 NewBar()
 {
-    RefPtr<Bar> b = new Bar();
-    return b.forget();
+    return new Bar();
 }
 
 void
 GetNewFoo(Foo** f)
 {
     *f = new Bar();
+    // Kids, don't try this at home
+    (*f)->AddRef();
 }
 
 void
 GetPassedFoo(Foo** f)
-{}
+{
+    // Kids, don't try this at home
+    (*f)->AddRef();
+}
 
 void
 GetNewFoo(RefPtr<Foo>* f)
 {
     *f = new Bar();
 }
 
 void
 GetPassedFoo(RefPtr<Foo>* f)
 {}
 
+TemporaryRef<Foo>
+GetNullFoo()
+{
+    return 0;
+}
+
 int
 main(int argc, char** argv)
 {
     // This should blow up
 //    Foo* f = new Foo(); delete f;
 
     MOZ_ASSERT(0 == Foo::numDestroyed);
     {
@@ -402,12 +427,18 @@ main(int argc, char** argv)
     }
     MOZ_ASSERT(12 == Foo::numDestroyed);
 
     {
         RefPtr<Foo> f1 = new Bar();
     }
     MOZ_ASSERT(13 == Foo::numDestroyed);
 
+    {
+        RefPtr<Foo> f = GetNullFoo();
+        MOZ_ASSERT(13 == Foo::numDestroyed);
+    }
+    MOZ_ASSERT(13 == Foo::numDestroyed);
+
     return 0;
 }
 
 #endif