Adds extra debugging to SkPathRef that can optionally be turned on in a release build.

This is pretty hacky but hopefully will be quite short-lived. Review URL: https://codereview.appspot.com/6584074 git-svn-id: http://skia.googlecode.com/svn/trunk@5793 2bbb7eff-a529-9590-31e7-b0007b416f81
2012-10-03 19:57:01 +00:00 · 2012-10-03 19:57:01 +00:00 · ae09f2dc3f
--- a/include/core/SkPath.h
+++ b/include/core/SkPath.h
@ -29,6 +29,14 @@ class SkAutoPathBoundsUpdate;
 class SkString;
 class SkPathRef;

+#ifndef SK_DEBUG_PATH_REF
+    #ifdef SK_DEBUG
+        #define SK_DEBUG_PATH_REF 1
+    #else
+        #define SK_DEBUG_PATH_REF 0
+    #endif
+#endif
+
 /** \class SkPath

    The SkPath class encapsulates compound (multiple contour) geometric paths
@ -830,7 +838,31 @@ private:
        kSegmentMask_SerializationShift = 0
    };

-    SkAutoTUnref<SkPathRef>   fPathRef;
+#if SK_DEBUG_PATH_REF
+public:
+    /** Debugging wrapper for SkAutoTUnref<SkPathRef> used to track owners (SkPaths)
+        of SkPathRefs */
+    class PathRefDebugRef {
+    public:
+        PathRefDebugRef(SkPath* owner);
+        PathRefDebugRef(SkPathRef* pr, SkPath* owner);
+        ~PathRefDebugRef();
+        void reset(SkPathRef* ref); 
+        void swap(PathRefDebugRef* other);
+        SkPathRef* get() const;
+        SkAutoTUnref<SkPathRef>::BlockRefType *operator->() const;
+        operator SkPathRef*();
+    private:
+        SkAutoTUnref<SkPathRef>   fPathRef;
+        SkPath*                   fOwner;
+    };
+
+private:
+    PathRefDebugRef     fPathRef;
+#else
+    SkAutoTUnref<SkPathRef> fPathRef;
+#endif
+
    mutable SkRect      fBounds;
    int                 fLastMoveToIndex;
    uint8_t             fFillType;
--- a/src/core/SkPath.cpp
+++ b/src/core/SkPath.cpp
@ -13,6 +13,61 @@
 #include "SkPathRef.h"
 #include "SkThread.h"

+
+////////////////////////////////////////////////////////////////////////////
+
+#if SK_DEBUG_PATH_REF
+
+SkPath::PathRefDebugRef::PathRefDebugRef(SkPath* owner) : fOwner(owner) {}
+
+SkPath::PathRefDebugRef::PathRefDebugRef(SkPathRef* pr, SkPath* owner)
+: fPathRef(pr)
+, fOwner(owner) {
+    pr->addOwner(owner);
+}
+
+SkPath::PathRefDebugRef::~PathRefDebugRef() {
+    fPathRef->removeOwner(fOwner);
+}
+
+void SkPath::PathRefDebugRef::reset(SkPathRef* ref) {
+    bool diff = (ref != fPathRef.get());
+    if (diff && NULL != fPathRef.get()) {
+        fPathRef.get()->removeOwner(fOwner);
+    }
+    fPathRef.reset(ref);
+    if (diff && NULL != fPathRef.get()) {
+        fPathRef.get()->addOwner(fOwner);
+    }
+}
+
+void SkPath::PathRefDebugRef::swap(SkPath::PathRefDebugRef* other) {
+    if (other->fPathRef.get() != fPathRef.get()) {
+        other->fPathRef->removeOwner(other->fOwner);
+        other->fPathRef->addOwner(fOwner);
+
+        fPathRef->removeOwner(fOwner);
+        fPathRef->addOwner(other->fOwner);
+    }
+
+    fPathRef.swap(&other->fPathRef);
+}
+
+SkPathRef* SkPath::PathRefDebugRef::get() const { return fPathRef.get(); }
+
+SkAutoTUnref<SkPathRef>::BlockRefType *SkPath::PathRefDebugRef::operator->() const {
+    return fPathRef.operator->();
+}
+
+SkPath::PathRefDebugRef::operator SkPathRef*() {
+    return fPathRef.operator SkPathRef *();
+}
+
+#endif
+    
+////////////////////////////////////////////////////////////////////////////
+
+
 SK_DEFINE_INST_COUNT(SkPath);

 // This value is just made-up for now. When count is 4, calling memset was much
@ -141,7 +196,11 @@ static bool compute_pt_bounds(SkRect* bounds, const SkPathRef& ref) {
 #define INITIAL_LASTMOVETOINDEX_VALUE   ~0

 SkPath::SkPath()
+#if SK_DEBUG_PATH_REF
+    : fPathRef(SkPathRef::CreateEmpty(), this)
+#else
    : fPathRef(SkPathRef::CreateEmpty())
+#endif
    , fFillType(kWinding_FillType)
    , fBoundsIsDirty(true) {
    fConvexity = kUnknown_Convexity;
@ -155,7 +214,11 @@ SkPath::SkPath()
 #endif
 }

-SkPath::SkPath(const SkPath& src) {
+SkPath::SkPath(const SkPath& src)
+#if SK_DEBUG_PATH_REF
+    : fPathRef(this)
+#endif
+{
    SkDEBUGCODE(src.validate();)
    src.fPathRef.get()->ref();
    fPathRef.reset(src.fPathRef.get());
--- a/src/core/SkPathRef.h
+++ b/src/core/SkPathRef.h
@ -32,13 +32,29 @@
 * verb array use ref.verbs()[~i] (because verbs() returns a pointer just beyond the first
 * logical verb or the last verb in memory).
 */
+
+class SkPathRef;
+SkPathRef* gEmptyPathRef; // This path ref should never be deleted once it is created.
+
+// Temporary hackery to try to nail down http://code.google.com/p/chromium/issues/detail?id=148637
+#if SK_DEBUG_PATH_REF
+    #define PR_CONTAINER SkPath::PathRefDebugRef
+    #define SkDEBUGCODE_X(code) code
+    #define SkASSERT_X(cond) SK_DEBUGBREAK(cond)
+    SK_DECLARE_STATIC_MUTEX(gOwnersMutex);
+#else
+    #define PR_CONTAINER SkAutoTUnref<SkPathRef>
+    #define SkDEBUGCODE_X(code) SkDEBUGCODE(code)
+    #define SkASSERT_X(cond) SkASSERT(cond)
+#endif
+
 class SkPathRef : public ::SkRefCnt {
 public:
    SK_DECLARE_INST_COUNT(SkPathRef);

    class Editor {
    public:
-        Editor(SkAutoTUnref<SkPathRef>* pathRef,
+        Editor(PR_CONTAINER* pathRef,
               int incReserveVerbs = 0,
               int incReservePoints = 0) {
            if (pathRef->get()->getRefCnt() > 1) {
@ -50,10 +66,10 @@ public:
            }
            fPathRef = pathRef->get();
            fPathRef->fGenerationID = 0;
-            SkDEBUGCODE(sk_atomic_inc(&fPathRef->fEditorsAttached);)
+            SkDEBUGCODE_X(sk_atomic_inc(&fPathRef->fEditorsAttached);)
        }

-        ~Editor() { SkDEBUGCODE(sk_atomic_dec(&fPathRef->fEditorsAttached);) }
+        ~Editor() { SkDEBUGCODE_X(sk_atomic_dec(&fPathRef->fEditorsAttached);) }

        /**
         * Returns the array of points.
@ -112,28 +128,56 @@ public:
    };

 public:
+#if SK_DEBUG_PATH_REF
+    void addOwner(SkPath* owner) {
+        gOwnersMutex.acquire();
+        for (int i = 0; i < fOwners.count(); ++i) {
+            SkASSERT_X(fOwners[i] != owner);
+        }
+        *fOwners.append() = owner;
+        SkASSERT_X((this->getRefCnt() == fOwners.count()) ||
+                   (this == gEmptyPathRef && this->getRefCnt() == fOwners.count() + 1));
+        gOwnersMutex.release();
+    }
+
+    void removeOwner(SkPath* owner) {
+        gOwnersMutex.acquire();
+        SkASSERT_X((this->getRefCnt() == fOwners.count()) ||
+                   (this == gEmptyPathRef && this->getRefCnt() == fOwners.count() + 1));
+        bool found = false;
+        for (int i = 0; !found && i < fOwners.count(); ++i) {
+            found = (owner == fOwners[i]);
+            if (found) {
+                fOwners.remove(i);
+            }
+        }
+        SkASSERT_X(found);
+        gOwnersMutex.release();
+    }
+#endif

    /**
     * Gets a path ref with no verbs or points.
     */
    static SkPathRef* CreateEmpty() {
-        static SkAutoTUnref<SkPathRef> gEmptyPathRef(SkNEW(SkPathRef));
-        gEmptyPathRef.get()->ref();
-        return gEmptyPathRef.get();
+        if (!gEmptyPathRef) {
+            gEmptyPathRef = SkNEW(SkPathRef); // leak!
+        }
+        return SkRef(gEmptyPathRef);
    }

    /**
     * Transforms a path ref by a matrix, allocating a new one only if necessary.
     */
-    static void CreateTransformedCopy(SkAutoTUnref<SkPathRef>* dst,
+    static void CreateTransformedCopy(PR_CONTAINER* dst,
                                      const SkPathRef& src,
                                      const SkMatrix& matrix) {
        src.validate();
        if (matrix.isIdentity()) {
            if (dst->get() != &src) {
+                src.ref();
                dst->reset(const_cast<SkPathRef*>(&src));
                (*dst)->validate();
-                src.ref();
            }
            return;
        }
@ -184,7 +228,7 @@ public:
     * repopulated with approximately the same number of verbs and points. A new path ref is created
     * only if necessary.
     */
-    static void Rewind(SkAutoTUnref<SkPathRef>* pathRef) {
+    static void Rewind(PR_CONTAINER* pathRef) {
        if (1 == (*pathRef)->getRefCnt()) {
            (*pathRef)->validate();
            (*pathRef)->fVerbCnt = 0;
@ -201,8 +245,21 @@ public:
    }

    virtual ~SkPathRef() {
+        SkASSERT_X(this != gEmptyPathRef);
+#if SK_DEBUG_PATH_REF
+        SkASSERT_X(!fOwners.count());
+#endif
+
        this->validate();
        sk_free(fPoints);
+
+        SkDEBUGCODE_X(fPoints = NULL;)
+        SkDEBUGCODE_X(fVerbs = NULL;)
+        SkDEBUGCODE_X(fVerbCnt = 0x9999999;)
+        SkDEBUGCODE_X(fPointCnt = 0xAAAAAAA;)
+        SkDEBUGCODE_X(fPointCnt = 0xBBBBBBB;)
+        SkDEBUGCODE_X(fGenerationID = 0xEEEEEEEE;)
+        SkDEBUGCODE_X(fEditorsAttached = 0x7777777;)
    }

    int countPoints() const { this->validate(); return fPointCnt; }
@ -282,7 +339,7 @@ public:
 #if NEW_PICTURE_FORMAT
    void writeToBuffer(SkWBuffer* buffer) {
        this->validate();
-        SkDEBUGCODE(size_t beforePos = buffer->pos();)
+        SkDEBUGCODE_X(size_t beforePos = buffer->pos();)

        // TODO: write gen ID here. Problem: We don't know if we're cross process or not from
        // SkWBuffer. Until this is fixed we write 0.
@ -319,7 +376,7 @@ private:
        fPoints = NULL;
        fFreeSpace = 0;
        fGenerationID = kEmptyGenID;
-        SkDEBUGCODE(fEditorsAttached = 0;)
+        SkDEBUGCODE_X(fEditorsAttached = 0;)
        this->validate();
    }

@ -480,7 +537,7 @@ private:
     * for the path ref.
     */
    int32_t genID() const {
-        SkDEBUGCODE(SkASSERT(!fEditorsAttached));
+        SkASSERT_X(!fEditorsAttached);
        if (!fGenerationID) {
            if (0 == fPointCnt && 0 == fVerbCnt) {
                fGenerationID = kEmptyGenID;
@ -521,7 +578,11 @@ private:
        kEmptyGenID = 1, // GenID reserved for path ref with zero points and zero verbs.
    };
    mutable int32_t     fGenerationID;
-    SkDEBUGCODE(int32_t fEditorsAttached;) // assert that only one editor in use at any time.
+    SkDEBUGCODE_X(int32_t fEditorsAttached;) // assert that only one editor in use at any time.
+
+#if SK_DEBUG_PATH_REF
+    SkTDArray<SkPath*> fOwners;
+#endif

    typedef SkRefCnt INHERITED;
 };