Merge "Update AndroidPixelRef to prevent VM from cleaning up memory prematurely." into lmp-mr1-dev

3 files changed, 16 insertions, 166 deletions

diff --git a/core/jni/android/graphics/Graphics.cpp b/core/jni/android/graphics/Graphics.cpp
index 2eccfbd..a51af40 100644
--- a/core/jni/android/graphics/Graphics.cpp
+++ b/core/jni/android/graphics/Graphics.cpp
@@ -492,19 +492,15 @@ AndroidPixelRef::AndroidPixelRef(JNIEnv* env, const SkImageInfo& info, void* sto
         SkMallocPixelRef(info, storage, rowBytes, ctable, (storageObj == NULL)),
         fWrappedPixelRef(NULL) {
     SkASSERT(storage);
+    SkASSERT(storageObj);
     SkASSERT(env);
 
     if (env->GetJavaVM(&fVM) != JNI_OK) {
         SkDebugf("------ [%p] env->GetJavaVM failed\n", env);
         sk_throw();
     }
-    fStorageObj = storageObj;
-    fHasGlobalRef = false;
-    fGlobalRefCnt = 0;
-
-    // If storageObj is NULL, the memory was NOT allocated on the Java heap
-    fOnJavaHeap = (storageObj != NULL);
 
+    fStorageObj = (jbyteArray) env->NewGlobalRef(storageObj);
 }
 
 AndroidPixelRef::AndroidPixelRef(AndroidPixelRef& wrappedPixelRef, const SkImageInfo& info,
@@ -516,91 +512,18 @@ AndroidPixelRef::AndroidPixelRef(AndroidPixelRef& wrappedPixelRef, const SkImage
     SkASSERT(fWrappedPixelRef);
     SkSafeRef(fWrappedPixelRef);
 
-    // don't need to initialize these, as all the relevant logic delegates to the wrapped ref
+    // don't need to initialize this, as all the relevant logic delegates to the wrapped ref
     fStorageObj = NULL;
-    fHasGlobalRef = false;
-    fGlobalRefCnt = 0;
-    fOnJavaHeap = false;
 }
 
 AndroidPixelRef::~AndroidPixelRef() {
     if (fWrappedPixelRef) {
         SkSafeUnref(fWrappedPixelRef);
-    } else if (fOnJavaHeap) {
+    } else {
+        SkASSERT(fStorageObj);
         JNIEnv* env = vm2env(fVM);
-
-        if (fStorageObj && fHasGlobalRef) {
-            env->DeleteGlobalRef(fStorageObj);
-        }
-        fStorageObj = NULL;
-    }
-}
-jbyteArray AndroidPixelRef::getStorageObj() {
-    if (fWrappedPixelRef) {
-        return fWrappedPixelRef->fStorageObj;
-    }
-    return fStorageObj;
-}
-
-void AndroidPixelRef::setLocalJNIRef(jbyteArray arr) {
-    if (fWrappedPixelRef) {
-        // delegate java obj management to the wrapped ref
-        fWrappedPixelRef->setLocalJNIRef(arr);
-    } else if (!fHasGlobalRef) {
-        fStorageObj = arr;
-    }
-}
-
-void AndroidPixelRef::globalRef(void* localref) {
-    if (fWrappedPixelRef) {
-        // delegate java obj management to the wrapped ref
-        fWrappedPixelRef->globalRef(localref);
-
-        // Note: we only ref and unref the wrapped AndroidPixelRef so that
-        // bitmap->pixelRef()->globalRef() and globalUnref() can be used in a pair, even if
-        // the bitmap has its underlying AndroidPixelRef swapped out/wrapped
-        return;
-    }
-    if (fOnJavaHeap && sk_atomic_inc(&fGlobalRefCnt) == 0) {
-        JNIEnv *env = vm2env(fVM);
-
-        // If JNI ref was passed, it is always used
-        if (localref) fStorageObj = (jbyteArray) localref;
-
-        if (fStorageObj == NULL) {
-            SkDebugf("No valid local ref to create a JNI global ref\n");
-            sk_throw();
-        }
-        if (fHasGlobalRef) {
-            // This should never happen
-            SkDebugf("Already holding a JNI global ref");
-            sk_throw();
-        }
-
-        fStorageObj = (jbyteArray) env->NewGlobalRef(fStorageObj);
-        // TODO: Check for failure here
-        fHasGlobalRef = true;
-    }
-    ref();
-}
-
-void AndroidPixelRef::globalUnref() {
-    if (fWrappedPixelRef) {
-        // delegate java obj management to the wrapped ref
-        fWrappedPixelRef->globalUnref();
-        return;
-    }
-    if (fOnJavaHeap && sk_atomic_dec(&fGlobalRefCnt) == 1) {
-        JNIEnv *env = vm2env(fVM);
-        if (!fHasGlobalRef) {
-            SkDebugf("We don't have a global ref!");
-            sk_throw();
-        }
         env->DeleteGlobalRef(fStorageObj);
-        fStorageObj = NULL;
-        fHasGlobalRef = false;
     }
-    unref();
 }
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -657,25 +580,6 @@ bool JavaPixelAllocator::allocPixelRef(SkBitmap* bitmap, SkColorTable* ctable) {
 
 ////////////////////////////////////////////////////////////////////////////////
 
-JavaHeapBitmapRef::JavaHeapBitmapRef(JNIEnv* env, SkBitmap* nativeBitmap, jbyteArray buffer) {
-    fEnv = env;
-    fNativeBitmap = nativeBitmap;
-    fBuffer = buffer;
-
-    // If the buffer is NULL, the backing memory wasn't allocated on the Java heap
-    if (fBuffer) {
-        ((AndroidPixelRef*) fNativeBitmap->pixelRef())->setLocalJNIRef(fBuffer);
-    }
-}
-
-JavaHeapBitmapRef::~JavaHeapBitmapRef() {
-    if (fBuffer) {
-        ((AndroidPixelRef*) fNativeBitmap->pixelRef())->setLocalJNIRef(NULL);
-    }
-}
-
-////////////////////////////////////////////////////////////////////////////////
-
 static jclass make_globalref(JNIEnv* env, const char classname[])
 {
     jclass c = env->FindClass(classname);
diff --git a/core/jni/android/graphics/GraphicsJNI.h b/core/jni/android/graphics/GraphicsJNI.h
index dcc97e5..42973ba 100644
--- a/core/jni/android/graphics/GraphicsJNI.h
+++ b/core/jni/android/graphics/GraphicsJNI.h
@@ -123,52 +123,11 @@ public:
 
     virtual ~AndroidPixelRef();
 
-    jbyteArray getStorageObj();
-
-    void setLocalJNIRef(jbyteArray arr);
-
-    /** Used to hold a ref to the pixels when the Java bitmap may be collected.
-     *  If specified, 'localref' is a valid JNI local reference to the byte array
-     *  containing the pixel data.
-     *
-     *  'localref' may only be NULL if setLocalJNIRef() was already called with
-     *  a JNI local ref that is still valid.
-     */
-    virtual void globalRef(void* localref=NULL);
-
-    /** Release a ref that was acquired using globalRef(). */
-    virtual void globalUnref();
-
 private:
     AndroidPixelRef* const fWrappedPixelRef; // if set, delegate memory management calls to this
 
     JavaVM* fVM;
-    bool fOnJavaHeap; // If true, the memory was allocated on the Java heap
-
     jbyteArray fStorageObj; // The Java byte[] object used as the bitmap backing store
-    bool fHasGlobalRef; // If true, fStorageObj holds a JNI global ref
-
-    mutable int32_t fGlobalRefCnt;
-};
-
-/** A helper class for accessing Java-heap-allocated bitmaps.
- *  This should be used when calling into a JNI method that retains a
- *  reference to the bitmap longer than the lifetime of the Java Bitmap.
- *
- *  After creating an instance of this class, a call to
- *  AndroidPixelRef::globalRef() will allocate a JNI global reference
- *  to the backing buffer object.
- */
-class JavaHeapBitmapRef {
-public:
-
-    JavaHeapBitmapRef(JNIEnv *env, SkBitmap* nativeBitmap, jbyteArray buffer);
-    ~JavaHeapBitmapRef();
-
-private:
-    JNIEnv* fEnv;
-    SkBitmap* fNativeBitmap;
-    jbyteArray fBuffer;
 };
 
 /** Allocator which allocates the backing buffer in the Java heap.
diff --git a/core/jni/android_view_GLES20Canvas.cpp b/core/jni/android_view_GLES20Canvas.cpp
index b023ebd..9bbd4fc 100644
--- a/core/jni/android_view_GLES20Canvas.cpp
+++ b/core/jni/android_view_GLES20Canvas.cpp
@@ -347,11 +347,8 @@ static void android_view_GLES20Canvas_concatMatrix(JNIEnv* env, jobject clazz,
 // ----------------------------------------------------------------------------
 
 static void android_view_GLES20Canvas_drawBitmap(JNIEnv* env, jobject clazz,
-        jlong rendererPtr, jlong bitmapPtr, jbyteArray buffer,
-        jfloat left, jfloat top, jlong paintPtr) {
+        jlong rendererPtr, jlong bitmapPtr, jfloat left, jfloat top, jlong paintPtr) {
     SkBitmap* bitmap = reinterpret_cast<SkBitmap*>(bitmapPtr);
-    // This object allows the renderer to allocate a global JNI ref to the buffer object.
-    JavaHeapBitmapRef bitmapRef(env, bitmap, buffer);
 
     DisplayListRenderer* renderer = reinterpret_cast<DisplayListRenderer*>(rendererPtr);
     Paint* paint = reinterpret_cast<Paint*>(paintPtr);
@@ -364,12 +361,10 @@ static void android_view_GLES20Canvas_drawBitmap(JNIEnv* env, jobject clazz,
 }
 
 static void android_view_GLES20Canvas_drawBitmapRect(JNIEnv* env, jobject clazz,
-        jlong rendererPtr, jlong bitmapPtr, jbyteArray buffer,
+        jlong rendererPtr, jlong bitmapPtr,
         float srcLeft, float srcTop, float srcRight, float srcBottom,
         float dstLeft, float dstTop, float dstRight, float dstBottom, jlong paintPtr) {
     SkBitmap* bitmap = reinterpret_cast<SkBitmap*>(bitmapPtr);
-    // This object allows the renderer to allocate a global JNI ref to the buffer object.
-    JavaHeapBitmapRef bitmapRef(env, bitmap, buffer);
 
     DisplayListRenderer* renderer = reinterpret_cast<DisplayListRenderer*>(rendererPtr);
     Paint* paint = reinterpret_cast<Paint*>(paintPtr);
@@ -378,11 +373,8 @@ static void android_view_GLES20Canvas_drawBitmapRect(JNIEnv* env, jobject clazz,
 }
 
 static void android_view_GLES20Canvas_drawBitmapMatrix(JNIEnv* env, jobject clazz,
-        jlong rendererPtr, jlong bitmapPtr, jbyteArray buffer,
-        jlong matrixPtr, jlong paintPtr) {
+        jlong rendererPtr, jlong bitmapPtr, jlong matrixPtr, jlong paintPtr) {
     SkBitmap* bitmap = reinterpret_cast<SkBitmap*>(bitmapPtr);
-    // This object allows the renderer to allocate a global JNI ref to the buffer object.
-    JavaHeapBitmapRef bitmapRef(env, bitmap, buffer);
 
     DisplayListRenderer* renderer = reinterpret_cast<DisplayListRenderer*>(rendererPtr);
     SkMatrix* matrix = reinterpret_cast<SkMatrix*>(matrixPtr);
@@ -427,12 +419,9 @@ static void android_view_GLES20Canvas_drawBitmapData(JNIEnv* env, jobject clazz,
 }
 
 static void android_view_GLES20Canvas_drawBitmapMesh(JNIEnv* env, jobject clazz,
-        jlong rendererPtr, jlong bitmapPtr, jbyteArray buffer,
-        jint meshWidth, jint meshHeight, jfloatArray vertices, jint offset, jintArray colors,
-        jint colorOffset, jlong paintPtr) {
+        jlong rendererPtr, jlong bitmapPtr, jint meshWidth, jint meshHeight,
+        jfloatArray vertices, jint offset, jintArray colors, jint colorOffset, jlong paintPtr) {
     SkBitmap* bitmap = reinterpret_cast<SkBitmap*>(bitmapPtr);
-    // This object allows the renderer to allocate a global JNI ref to the buffer object.
-    JavaHeapBitmapRef bitmapRef(env, bitmap, buffer);
 
     jfloat* verticesArray = vertices ? env->GetFloatArrayElements(vertices, NULL) + offset : NULL;
     jint* colorsArray = colors ? env->GetIntArrayElements(colors, NULL) + colorOffset : NULL;
@@ -446,11 +435,9 @@ static void android_view_GLES20Canvas_drawBitmapMesh(JNIEnv* env, jobject clazz,
 }
 
 static void android_view_GLES20Canvas_drawPatch(JNIEnv* env, jobject clazz,
-        jlong rendererPtr, jlong bitmapPtr, jbyteArray buffer, jlong patchPtr,
+        jlong rendererPtr, jlong bitmapPtr, jlong patchPtr,
         float left, float top, float right, float bottom, jlong paintPtr) {
     SkBitmap* bitmap = reinterpret_cast<SkBitmap*>(bitmapPtr);
-    // This object allows the renderer to allocate a global JNI ref to the buffer object.
-    JavaHeapBitmapRef bitmapRef(env, bitmap, buffer);
 
     DisplayListRenderer* renderer = reinterpret_cast<DisplayListRenderer*>(rendererPtr);
     Res_png_9patch* patch = reinterpret_cast<Res_png_9patch*>(patchPtr);
@@ -914,14 +901,14 @@ static JNINativeMethod gMethods[] = {
     { "nGetMatrix",         "(JJ)V",           (void*) android_view_GLES20Canvas_getMatrix },
     { "nConcatMatrix",      "(JJ)V",           (void*) android_view_GLES20Canvas_concatMatrix },
 
-    { "nDrawBitmap",        "(JJ[BFFJ)V",      (void*) android_view_GLES20Canvas_drawBitmap },
-    { "nDrawBitmap",        "(JJ[BFFFFFFFFJ)V",(void*) android_view_GLES20Canvas_drawBitmapRect },
-    { "nDrawBitmap",        "(JJ[BJJ)V",       (void*) android_view_GLES20Canvas_drawBitmapMatrix },
+    { "nDrawBitmap",        "(JJFFJ)V",      (void*) android_view_GLES20Canvas_drawBitmap },
+    { "nDrawBitmap",        "(JJFFFFFFFFJ)V",(void*) android_view_GLES20Canvas_drawBitmapRect },
+    { "nDrawBitmap",        "(JJJJ)V",       (void*) android_view_GLES20Canvas_drawBitmapMatrix },
     { "nDrawBitmap",        "(J[IIIFFIIZJ)V",  (void*) android_view_GLES20Canvas_drawBitmapData },
 
-    { "nDrawBitmapMesh",    "(JJ[BII[FI[IIJ)V",(void*) android_view_GLES20Canvas_drawBitmapMesh },
+    { "nDrawBitmapMesh",    "(JJII[FI[IIJ)V",(void*) android_view_GLES20Canvas_drawBitmapMesh },
 
-    { "nDrawPatch",         "(JJ[BJFFFFJ)V",   (void*) android_view_GLES20Canvas_drawPatch },
+    { "nDrawPatch",         "(JJJFFFFJ)V",   (void*) android_view_GLES20Canvas_drawPatch },
 
     { "nDrawColor",         "(JII)V",          (void*) android_view_GLES20Canvas_drawColor },
     { "nDrawRect",          "(JFFFFJ)V",       (void*) android_view_GLES20Canvas_drawRect },