add protected method for internal_dispose overrides to jam fRefCnt before

calling destructor.

move SkTRefArray to actually inherit from SkRefCnt
Review URL: https://codereview.appspot.com/6422057

git-svn-id: http://skia.googlecode.com/svn/trunk@4719 2bbb7eff-a529-9590-31e7-b0007b416f81

include/core/SkRefCnt.h

@@ -70,14 +70,26 @@ public:
         SkASSERT(fRefCnt > 0);
     }
 
-private:
-    /** Called when the ref count goes to 0.
-    */
-    virtual void internal_dispose() const {
+protected:
+    /**
+     *  Allow subclasses to call this if they've overridden internal_dispose
+     *  so they can reset fRefCnt before the destructor is called. Should only
+     *  be called right before calling through to inherited internal_dispose()
+     *  or before calling the destructor.
+     */
+    void internal_dispose_restore_refcnt_to_1() const {
 #ifdef SK_DEBUG
-        // so our destructor won't complain
+        SkASSERT(0 == fRefCnt);
         fRefCnt = 1;
 #endif
+    }
+
+private:
+    /**
+     *  Called when the ref count goes to 0.
+     */
+    virtual void internal_dispose() const {
+        this->internal_dispose_restore_refcnt_to_1();
         SkDELETE(this);
     }
 

src/core/SkTRefArray.h

@@ -9,70 +9,104 @@
 #ifndef SkTRefArray_DEFINED
 #define SkTRefArray_DEFINED
 
-#include "SkThread.h"
+#include "SkRefCnt.h"
 #include <new>
 
 /**
  *  Wrapper to manage thread-safe sharing of an array of T objects. The array
  *  cannot be grown or shrunk.
  */
-template <typename T> class SkTRefArray {
-public:
-    static SkTRefArray<T>* Create(int count) {
+template <typename T> class SkTRefArray : public SkRefCnt {
+    /*
+     *  Shared factory to allocate the space needed for our instance plus N
+     *  T entries at the end. We call our constructor, but not the constructors
+     *  for the elements. Those are called by the proper Create method.
+     */
+    static SkTRefArray<T>* Alloc(int count) {
+        // space for us, and our [count] elements
         size_t size = sizeof(SkTRefArray<T>) + count * sizeof(T);
         SkTRefArray<T>* obj = (SkTRefArray<T>*)sk_malloc_throw(size);
         
+        SkNEW_PLACEMENT(obj, SkTRefArray<T>);
         obj->fCount = count;
-        obj->fRefCnt = 1;
-
-        T* array = const_cast<T*>(obj->begin());
-        for (int i = 0; i < count; ++i) {
-            new (&array[i]) T;
-        }
         return obj;
     }
 
+public:
+    /**
+     *  Return a new array with 'count' elements, initialized to their default
+     *  value. To change them to some other value, use writableBegin/End or
+     *  writableAt(), but do that before this array is given to another thread.
+     */
+    static SkTRefArray<T>* Create(int count) {
+        SkTRefArray<T>* obj = Alloc(count);
+        T* array = const_cast<T*>(obj->begin());
+        for (int i = 0; i < count; ++i) {
+            SkNEW_PLACEMENT(&array[i], T);
+        }
+        return obj;
+    }
+    
+    /**
+     *  Return a new array with 'count' elements, initialized from the provided
+     *  src array. To change them to some other value, use writableBegin/End or
+     *  writableAt(), but do that before this array is given to another thread.
+     */
+    static SkTRefArray<T>* Create(const T src[], int count) {
+        SkTRefArray<T>* obj = Alloc(count);
+        T* array = const_cast<T*>(obj->begin());
+        for (int i = 0; i < count; ++i) {
+            SkNEW_PLACEMENT_ARGS(&array[i], T, (src[i]));
+        }
+        return obj;
+    }
+    
     int count() const { return fCount; }
     const T* begin() const { return (const T*)(this + 1); }
-    const T* end() const { return (const T*)(this + 1) + fCount; }
-    const T& operator[](int index) const {
+    const T* end() const { return this->begin() + fCount; }
+    const T& at(int index) const {
         SkASSERT((unsigned)index < (unsigned)fCount);
         return this->begin()[index];
     }
+    const T& operator[](int index) const { return this->at(index); }
 
-    // We mimic SkRefCnt in API, but we don't inherit as we want to control
-    // the allocation/deallocation so we can keep the array in the same
-    // block of memory
+    // For the writable methods, we assert that we are the only owner if we
+    // call these, since other owners are not informed if we change an element.
 
-    int32_t getRefCnt() const { return fRefCnt; }
-
-    void ref() const {
-        SkASSERT(fRefCnt > 0);
-        sk_atomic_inc(&fRefCnt);
+    T* writableBegin() {
+        SkASSERT(1 == this->getRefCnt());
+        return (T*)(this + 1);
     }
-    
-    void unref() const {
-        SkASSERT(fRefCnt > 0);
-        if (sk_atomic_dec(&fRefCnt) == 1) {
-            sk_membar_aquire__after_atomic_dec();
-            this->deleteAll();
-            sk_free((void*)this);
-        }
+    T* writableEnd() {
+        return this->writableBegin() + fCount;
+    }
+    T& writableAt(int index) {
+        SkASSERT((unsigned)index < (unsigned)fCount);
+        return this->writableBegin()[index];
     }
 
-private:
-    int             fCount;
-    mutable int32_t fRefCnt;
-
-    void deleteAll() const {
+protected:
+    virtual void internal_dispose() const SK_OVERRIDE {
         T* array = const_cast<T*>(this->begin());
         int n = fCount;
-
+        
         for (int i = 0; i < n; ++i) {
             array->~T();
             array += 1;
         }
+        
+        this->internal_dispose_restore_refcnt_to_1();
+        this->~SkTRefArray<T>();
+        sk_free((void*)this);
     }
+
+private:
+    int fCount;
+
+    // hide this
+    virtual ~SkTRefArray() {}
+
+    typedef SkRefCnt INHERITED;
 };
 
 #endif

tests/RefCntTest.cpp

@@ -17,10 +17,15 @@
 
 class InstCounterClass {
 public:
-    InstCounterClass() {  gInstCounter += 1; }
-    ~InstCounterClass() { gInstCounter -= 1; }
+    InstCounterClass() { fCount = gInstCounter++; }
+    InstCounterClass(const InstCounterClass& src) {
+        fCount = src.fCount;
+        gInstCounter += 1;
+    }
+    virtual ~InstCounterClass() { gInstCounter -= 1; }
 
     static int gInstCounter;
+    int fCount;
 };
 
 int InstCounterClass::gInstCounter;
@@ -28,13 +33,40 @@ int InstCounterClass::gInstCounter;
 static void test_refarray(skiatest::Reporter* reporter) {
     REPORTER_ASSERT(reporter, 0 == InstCounterClass::gInstCounter);
 
-    int N = 10;
+    const int N = 10;
     SkTRefArray<InstCounterClass>* array = SkTRefArray<InstCounterClass>::Create(N);
+
     REPORTER_ASSERT(reporter, 1 == array->getRefCnt());
+    REPORTER_ASSERT(reporter, N == array->count());
 
     REPORTER_ASSERT(reporter, N == InstCounterClass::gInstCounter);
-    REPORTER_ASSERT(reporter, array->count() == N);
+    array->unref();
+    REPORTER_ASSERT(reporter, 0 == InstCounterClass::gInstCounter);
+    
+    // Now test the copy factory
 
+    int i;
+    InstCounterClass* src = new InstCounterClass[N];
+    REPORTER_ASSERT(reporter, N == InstCounterClass::gInstCounter);
+    for (i = 0; i < N; ++i) {
+        REPORTER_ASSERT(reporter, i == src[i].fCount);
+    }
+
+    array = SkTRefArray<InstCounterClass>::Create(src, N);
+    REPORTER_ASSERT(reporter, 1 == array->getRefCnt());
+    REPORTER_ASSERT(reporter, N == array->count());
+
+    REPORTER_ASSERT(reporter, 2*N == InstCounterClass::gInstCounter);
+    for (i = 0; i < N; ++i) {
+        REPORTER_ASSERT(reporter, i == (*array)[i].fCount);
+    }
+
+    delete[] src;
+    REPORTER_ASSERT(reporter, N == InstCounterClass::gInstCounter);
+
+    for (i = 0; i < N; ++i) {
+        REPORTER_ASSERT(reporter, i == (*array)[i].fCount);
+    }
     array->unref();
     REPORTER_ASSERT(reporter, 0 == InstCounterClass::gInstCounter);
 }