Fix crash when Paths are GCd in hw accelerated apps

A recent change to optimize path rendering didn't account for
the destruction of native objects by the VM finalizer. We may be
done with the Java level version before we're done with the native
structure that's used by the display list. For example, a drawing
method on a View that creates a temporary path to render into the
canvas will implicitly create a native structure that is put onto
the GL display list. That temporary path may go away, but the native
version should stick around as long as the display list does.

The fix is to refcount the original native version of the path
and only delete it when the refcoutn reaches zero (which means that
it is no longer needed by any display list). This is a similar mechanism
used for bitmaps and shaders.

Change-Id: I4de1047415066d425d1c689aa60827f97729b470

5 files changed, 67 insertions, 1 deletions

diff --git a/core/jni/android/graphics/Path.cpp b/core/jni/android/graphics/Path.cpp
index 90c4dd453c2e..eb9e004c9861 100644
--- a/core/jni/android/graphics/Path.cpp
+++ b/core/jni/android/graphics/Path.cpp
@@ -36,7 +36,8 @@ public:
     static void finalizer(JNIEnv* env, jobject clazz, SkPath* obj) {
 #ifdef USE_OPENGL_RENDERER
         if (android::uirenderer::Caches::hasInstance()) {
-            android::uirenderer::Caches::getInstance().pathCache.removeDeferred(obj);
+            android::uirenderer::Caches::getInstance().resourceCache.destructor(obj);
+            return;
         }
 #endif
         delete obj;
diff --git a/libs/hwui/DisplayListRenderer.cpp b/libs/hwui/DisplayListRenderer.cpp
index 2df52ae65e12..d5d2ba07154c 100644
--- a/libs/hwui/DisplayListRenderer.cpp
+++ b/libs/hwui/DisplayListRenderer.cpp
@@ -95,6 +95,10 @@ void DisplayList::clearResources() {
         delete mPaths.itemAt(i);
     }
     mPaths.clear();
+    for (size_t i = 0; i < mOriginalPaths.size(); i++) {
+        caches.resourceCache.decrementRefcount(mOriginalPaths.itemAt(i));
+    }
+    mOriginalPaths.clear();
 
     for (size_t i = 0; i < mMatrices.size(); i++) {
         delete mMatrices.itemAt(i);
@@ -146,6 +150,13 @@ void DisplayList::initFromDisplayListRenderer(const DisplayListRenderer& recorde
         mPaths.add(paths.itemAt(i));
     }
 
+    const Vector<SkPath*> &originalPaths = recorder.getOriginalPaths();
+    for (size_t i = 0; i < originalPaths.size(); i++) {
+        SkPath* path = originalPaths.itemAt(i);
+        mOriginalPaths.add(path);
+        caches.resourceCache.incrementRefcount(path);
+    }
+
     const Vector<SkMatrix*> &matrices = recorder.getMatrices();
     for (size_t i = 0; i < matrices.size(); i++) {
         mMatrices.add(matrices.itemAt(i));
@@ -519,6 +530,12 @@ void DisplayListRenderer::reset() {
     }
     mBitmapResources.clear();
 
+    for (size_t i = 0; i < mOriginalPaths.size(); i++) {
+        SkPath* resource = mOriginalPaths.itemAt(i);
+        caches.resourceCache.decrementRefcount(resource);
+    }
+    mOriginalPaths.clear();
+
     for (size_t i = 0; i < mShaders.size(); i++) {
        caches.resourceCache.decrementRefcount(mShaders.itemAt(i));
     }
diff --git a/libs/hwui/DisplayListRenderer.h b/libs/hwui/DisplayListRenderer.h
index 2d0e30a48e05..f39f37f860eb 100644
--- a/libs/hwui/DisplayListRenderer.h
+++ b/libs/hwui/DisplayListRenderer.h
@@ -190,6 +190,7 @@ private:
 
     Vector<SkPaint*> mPaints;
     Vector<SkPath*> mPaths;
+    Vector<SkPath*> mOriginalPaths;
     Vector<SkMatrix*> mMatrices;
     Vector<SkiaShader*> mShaders;
 
@@ -293,6 +294,10 @@ public:
         return mPaths;
     }
 
+    const Vector<SkPath*>& getOriginalPaths() const {
+        return mOriginalPaths;
+    }
+
     const Vector<SkMatrix*>& getMatrices() const {
         return mMatrices;
     }
@@ -371,6 +376,9 @@ private:
         if (pathCopy == NULL || pathCopy->getGenerationID() != path->getGenerationID()) {
             if (pathCopy == NULL) {
                 pathCopy = path;
+                mOriginalPaths.add(path);
+                Caches& caches = Caches::getInstance();
+                caches.resourceCache.incrementRefcount(path);
             } else {
                 pathCopy = new SkPath(*path);
                 mPaths.add(pathCopy);
@@ -452,6 +460,7 @@ private:
     Vector<SkPaint*> mPaints;
     DefaultKeyedVector<SkPaint*, SkPaint*> mPaintMap;
 
+    Vector<SkPath*> mOriginalPaths;
     Vector<SkPath*> mPaths;
     DefaultKeyedVector<SkPath*, SkPath*> mPathMap;
 
diff --git a/libs/hwui/ResourceCache.cpp b/libs/hwui/ResourceCache.cpp
index 70d117a3622b..87fdfb5212a1 100644
--- a/libs/hwui/ResourceCache.cpp
+++ b/libs/hwui/ResourceCache.cpp
@@ -65,6 +65,10 @@ void ResourceCache::incrementRefcount(SkBitmap* bitmapResource) {
     incrementRefcount((void*)bitmapResource, kBitmap);
 }
 
+void ResourceCache::incrementRefcount(SkPath* pathResource) {
+    incrementRefcount((void*)pathResource, kPath);
+}
+
 void ResourceCache::incrementRefcount(SkiaShader* shaderResource) {
     shaderResource->getSkShader()->safeRef();
     incrementRefcount((void*) shaderResource, kShader);
@@ -94,6 +98,10 @@ void ResourceCache::decrementRefcount(SkBitmap* bitmapResource) {
     decrementRefcount((void*) bitmapResource);
 }
 
+void ResourceCache::decrementRefcount(SkPath* pathResource) {
+    decrementRefcount((void*) pathResource);
+}
+
 void ResourceCache::decrementRefcount(SkiaShader* shaderResource) {
     shaderResource->getSkShader()->safeUnref();
     decrementRefcount((void*) shaderResource);
@@ -122,6 +130,24 @@ void ResourceCache::recycle(SkBitmap* resource) {
     }
 }
 
+void ResourceCache::destructor(SkPath* resource) {
+    Mutex::Autolock _l(mLock);
+    ResourceReference* ref = mCache->indexOfKey(resource) >= 0 ? mCache->valueFor(resource) : NULL;
+    if (ref == NULL) {
+        // If we're not tracking this resource, just delete it
+        if (Caches::hasInstance()) {
+            Caches::getInstance().pathCache.removeDeferred(resource);
+        }
+        delete resource;
+        return;
+    }
+    ref->destroyed = true;
+    if (ref->refCount == 0) {
+        deleteResourceReference(resource, ref);
+        return;
+    }
+}
+
 void ResourceCache::destructor(SkBitmap* resource) {
     Mutex::Autolock _l(mLock);
     ResourceReference* ref = mCache->indexOfKey(resource) >= 0 ? mCache->valueFor(resource) : NULL;
@@ -192,6 +218,15 @@ void ResourceCache::deleteResourceReference(void* resource, ResourceReference* r
                 delete bitmap;
             }
             break;
+            case kPath:
+            {
+                SkPath* path = (SkPath*)resource;
+                if (Caches::hasInstance()) {
+                    Caches::getInstance().pathCache.removeDeferred(path);
+                }
+                delete path;
+            }
+            break;
             case kShader:
             {
                 SkiaShader* shader = (SkiaShader*)resource;
diff --git a/libs/hwui/ResourceCache.h b/libs/hwui/ResourceCache.h
index 1bb4390f82d9..2a38910951c4 100644
--- a/libs/hwui/ResourceCache.h
+++ b/libs/hwui/ResourceCache.h
@@ -32,6 +32,7 @@ enum ResourceType {
     kBitmap,
     kShader,
     kColorFilter,
+    kPath,
 };
 
 class ResourceReference {
@@ -53,15 +54,18 @@ class ResourceCache {
 public:
     ResourceCache();
     ~ResourceCache();
+    void incrementRefcount(SkPath* resource);
     void incrementRefcount(SkBitmap* resource);
     void incrementRefcount(SkiaShader* resource);
     void incrementRefcount(SkiaColorFilter* resource);
     void incrementRefcount(const void* resource, ResourceType resourceType);
     void decrementRefcount(void* resource);
     void decrementRefcount(SkBitmap* resource);
+    void decrementRefcount(SkPath* resource);
     void decrementRefcount(SkiaShader* resource);
     void decrementRefcount(SkiaColorFilter* resource);
     void recycle(SkBitmap* resource);
+    void destructor(SkPath* resource);
     void destructor(SkBitmap* resource);
     void destructor(SkiaShader* resource);
     void destructor(SkiaColorFilter* resource);