Fix NDK access to recycle'd bitmaps

Also kills off one user of GraphicsJNI.h!

Change-Id: Icbf979e485b3b6ec2f37e18ff654b8ff1e44fb35
Fixes: 34712423
Test: cts CtsGraphicsTestCases --test android.graphics.cts.BitmapTest#testNdkAccessAfterRecycle passes

3 files changed, 81 insertions, 58 deletions

diff --git a/core/jni/android/graphics/Bitmap.cpp b/core/jni/android/graphics/Bitmap.cpp
index 3ca455dc24cf..db4f30bf1fff 100755
--- a/core/jni/android/graphics/Bitmap.cpp
+++ b/core/jni/android/graphics/Bitmap.cpp
@@ -232,6 +232,73 @@ Bitmap& toBitmap(JNIEnv* env, jlong bitmapHandle) {
     return localBitmap->bitmap();
 }
 
+void imageInfo(JNIEnv* env, jobject bitmap, AndroidBitmapInfo* info) {
+    SkASSERT(info);
+    SkASSERT(env);
+    SkASSERT(bitmap);
+    SkASSERT(env->IsInstanceOf(bitmap, gBitmap_class));
+    jlong bitmapHandle = env->GetLongField(bitmap, gBitmap_nativePtr);
+    LocalScopedBitmap localBitmap(bitmapHandle);
+
+    const SkImageInfo& imageInfo = localBitmap->info();
+    info->width = imageInfo.width();
+    info->height = imageInfo.height();
+    info->stride = localBitmap->rowBytes();
+    info->flags = 0;
+    switch (imageInfo.colorType()) {
+        case kN32_SkColorType:
+            info->format = ANDROID_BITMAP_FORMAT_RGBA_8888;
+            break;
+        case kRGB_565_SkColorType:
+            info->format = ANDROID_BITMAP_FORMAT_RGB_565;
+            break;
+        case kARGB_4444_SkColorType:
+            info->format = ANDROID_BITMAP_FORMAT_RGBA_4444;
+            break;
+        case kAlpha_8_SkColorType:
+            info->format = ANDROID_BITMAP_FORMAT_A_8;
+            break;
+        default:
+            info->format = ANDROID_BITMAP_FORMAT_NONE;
+            break;
+    }
+}
+
+void* lockPixels(JNIEnv* env, jobject bitmap) {
+    SkASSERT(env);
+    SkASSERT(bitmap);
+    SkASSERT(env->IsInstanceOf(bitmap, gBitmap_class));
+    jlong bitmapHandle = env->GetLongField(bitmap, gBitmap_nativePtr);
+
+    LocalScopedBitmap localBitmap(bitmapHandle);
+    if (!localBitmap->valid()) return nullptr;
+
+    SkPixelRef& pixelRef = localBitmap->bitmap();
+    pixelRef.lockPixels();
+    if (!pixelRef.pixels()) {
+        pixelRef.unlockPixels();
+        return nullptr;
+    }
+    pixelRef.ref();
+    return pixelRef.pixels();
+}
+
+bool unlockPixels(JNIEnv* env, jobject bitmap) {
+    SkASSERT(env);
+    SkASSERT(bitmap);
+    SkASSERT(env->IsInstanceOf(bitmap, gBitmap_class));
+    jlong bitmapHandle = env->GetLongField(bitmap, gBitmap_nativePtr);
+
+    LocalScopedBitmap localBitmap(bitmapHandle);
+    if (!localBitmap->valid()) return false;
+
+    SkPixelRef& pixelRef = localBitmap->bitmap();
+    pixelRef.notifyPixelsChanged();
+    pixelRef.unlockPixels();
+    pixelRef.unref();
+    return true;
+}
+
 } // namespace bitmap
 
 } // namespace android
diff --git a/core/jni/android/graphics/Bitmap.h b/core/jni/android/graphics/Bitmap.h
index 387a1285ba12..a4bfc94b79e3 100644
--- a/core/jni/android/graphics/Bitmap.h
+++ b/core/jni/android/graphics/Bitmap.h
@@ -17,6 +17,7 @@
 #define BITMAP_H_
 
 #include <jni.h>
+#include <android/bitmap.h>
 #include <SkBitmap.h>
 #include <SkColorTable.h>
 #include <SkImageInfo.h>
@@ -44,6 +45,13 @@ void toSkBitmap(jlong bitmapHandle, SkBitmap* outBitmap);
 Bitmap& toBitmap(JNIEnv* env, jobject bitmap);
 Bitmap& toBitmap(JNIEnv* env, jlong bitmapHandle);
 
+// NDK access
+void imageInfo(JNIEnv* env, jobject bitmap, AndroidBitmapInfo* info);
+// Returns a pointer to the pixels or nullptr if the bitmap is not valid
+void* lockPixels(JNIEnv* env, jobject bitmap);
+// Returns true if unlocked, false if the bitmap is no longer valid (destroyed)
+bool unlockPixels(JNIEnv* env, jobject bitmap);
+
 /** Reinitialize a bitmap. bitmap must already have its SkAlphaType set in
     sync with isPremultiplied
 */
diff --git a/native/graphics/jni/bitmap.cpp b/native/graphics/jni/bitmap.cpp
index 6d2de98759c8..bf5cabbe4bc2 100644
--- a/native/graphics/jni/bitmap.cpp
+++ b/native/graphics/jni/bitmap.cpp
@@ -15,11 +15,7 @@
  */
 
 #include <android/bitmap.h>
-
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wunused-parameter"
-#include <GraphicsJNI.h>
-#pragma GCC diagnostic pop
+#include <Bitmap.h>
 
 int AndroidBitmap_getInfo(JNIEnv* env, jobject jbitmap,
                           AndroidBitmapInfo* info) {
@@ -27,32 +23,8 @@ int AndroidBitmap_getInfo(JNIEnv* env, jobject jbitmap,
         return ANDROID_BITMAP_RESULT_BAD_PARAMETER;
     }
 
-    SkBitmap bm;
-    GraphicsJNI::getSkBitmap(env, jbitmap, &bm);
-
     if (info) {
-        info->width     = bm.width();
-        info->height    = bm.height();
-        info->stride    = bm.rowBytes();
-        info->flags     = 0;
-
-        switch (bm.colorType()) {
-            case kN32_SkColorType:
-                info->format = ANDROID_BITMAP_FORMAT_RGBA_8888;
-                break;
-            case kRGB_565_SkColorType:
-                info->format = ANDROID_BITMAP_FORMAT_RGB_565;
-                break;
-            case kARGB_4444_SkColorType:
-                info->format = ANDROID_BITMAP_FORMAT_RGBA_4444;
-                break;
-            case kAlpha_8_SkColorType:
-                info->format = ANDROID_BITMAP_FORMAT_A_8;
-                break;
-            default:
-                info->format = ANDROID_BITMAP_FORMAT_NONE;
-                break;
-        }
+        android::bitmap::imageInfo(env, jbitmap, info);
     }
     return ANDROID_BITMAP_RESULT_SUCCESS;
 }
@@ -62,19 +34,11 @@ int AndroidBitmap_lockPixels(JNIEnv* env, jobject jbitmap, void** addrPtr) {
         return ANDROID_BITMAP_RESULT_BAD_PARAMETER;
     }
 
-    SkPixelRef* pixelRef = GraphicsJNI::refSkPixelRef(env, jbitmap);
-    if (!pixelRef) {
+    void* addr = android::bitmap::lockPixels(env, jbitmap);
+    if (!addr) {
         return ANDROID_BITMAP_RESULT_JNI_EXCEPTION;
     }
 
-    pixelRef->lockPixels();
-    void* addr = pixelRef->pixels();
-    if (NULL == addr) {
-        pixelRef->unlockPixels();
-        pixelRef->unref();
-        return ANDROID_BITMAP_RESULT_ALLOCATION_FAILED;
-    }
-
     if (addrPtr) {
         *addrPtr = addr;
     }
@@ -86,26 +50,10 @@ int AndroidBitmap_unlockPixels(JNIEnv* env, jobject jbitmap) {
         return ANDROID_BITMAP_RESULT_BAD_PARAMETER;
     }
 
-    SkPixelRef* pixelRef = GraphicsJNI::refSkPixelRef(env, jbitmap);
-    if (!pixelRef) {
+    bool unlocked = android::bitmap::unlockPixels(env, jbitmap);
+    if (!unlocked) {
         return ANDROID_BITMAP_RESULT_JNI_EXCEPTION;
     }
-
-    // notifyPixelsChanged() needs be called to apply writes to GL-backed
-    // bitmaps.  Note that this will slow down read-only accesses to the
-    // bitmaps, but the NDK methods are primarily intended to be used for
-    // writes.
-    pixelRef->notifyPixelsChanged();
-
-    pixelRef->unlockPixels();
-    // Awkward in that we need to double-unref as the call to get the SkPixelRef
-    // did a ref(), so we need to unref() for the local ref and for the previous
-    // AndroidBitmap_lockPixels(). However this keeps GraphicsJNI a bit safer
-    // if others start using it without knowing about android::Bitmap's "fun"
-    // ref counting mechanism(s).
-    pixelRef->unref();
-    pixelRef->unref();
-
     return ANDROID_BITMAP_RESULT_SUCCESS;
 }