Merge "Add warning if an in-use Bitmap is reconfigured" into mnc-dev

6 files changed, 44 insertions, 25 deletions

diff --git a/core/jni/android/graphics/Bitmap.cpp b/core/jni/android/graphics/Bitmap.cpp
index 8ae2e3b..9babe82 100755
--- a/core/jni/android/graphics/Bitmap.cpp
+++ b/core/jni/android/graphics/Bitmap.cpp
@@ -188,18 +188,43 @@ size_t Bitmap::rowBytes() const {
     return mPixelRef->rowBytes();
 }
 
-SkPixelRef* Bitmap::pixelRef() const {
+SkPixelRef* Bitmap::peekAtPixelRef() const {
     assertValid();
     return mPixelRef.get();
 }
 
+SkPixelRef* Bitmap::refPixelRef() {
+    assertValid();
+    android::AutoMutex _lock(mLock);
+    return refPixelRefLocked();
+}
+
+SkPixelRef* Bitmap::refPixelRefLocked() {
+    mPixelRef->ref();
+    if (mPixelRef->unique()) {
+        // We just restored this from 0, pin the pixels and inc the strong count
+        // Note that there *might be* an incoming onStrongRefDestroyed from whatever
+        // last unref'd
+        pinPixelsLocked();
+        mPinnedRefCount++;
+    }
+    return mPixelRef.get();
+}
+
 void Bitmap::reconfigure(const SkImageInfo& info, size_t rowBytes,
         SkColorTable* ctable) {
+    {
+        android::AutoMutex _lock(mLock);
+        if (mPinnedRefCount) {
+            ALOGW("Called reconfigure on a bitmap that is in use! This may"
+                    " cause graphical corruption!");
+        }
+    }
     mPixelRef->reconfigure(info, rowBytes, ctable);
 }
 
 void Bitmap::reconfigure(const SkImageInfo& info) {
-    mPixelRef->reconfigure(info, mPixelRef->rowBytes(), mPixelRef->colorTable());
+    reconfigure(info, mPixelRef->rowBytes(), mPixelRef->colorTable());
 }
 
 void Bitmap::detachFromJava() {
@@ -287,18 +312,10 @@ void Bitmap::unpinPixelsLocked() {
 void Bitmap::getSkBitmap(SkBitmap* outBitmap) {
     assertValid();
     android::AutoMutex _lock(mLock);
-    mPixelRef->ref();
-    if (mPixelRef->unique()) {
-        // We just restored this from 0, pin the pixels and inc the strong count
-        // Note that there *might be* an incoming onStrongRefDestroyed from whatever
-        // last unref'd
-        pinPixelsLocked();
-        mPinnedRefCount++;
-    }
     // Safe because mPixelRef is a WrappedPixelRef type, otherwise rowBytes()
     // would require locking the pixels first.
     outBitmap->setInfo(mPixelRef->info(), mPixelRef->rowBytes());
-    outBitmap->setPixelRef(mPixelRef.get())->unref();
+    outBitmap->setPixelRef(refPixelRefLocked())->unref();
     outBitmap->setHasHardwareMipMap(hasHardwareMipMap());
 }
 
@@ -323,7 +340,7 @@ public:
     }
 
     void* pixels() {
-        return mBitmap->pixelRef()->pixels();
+        return mBitmap->peekAtPixelRef()->pixels();
     }
 
     bool valid() {
@@ -780,7 +797,7 @@ static jint Bitmap_config(JNIEnv* env, jobject, jlong bitmapHandle) {
 
 static jint Bitmap_getGenerationId(JNIEnv* env, jobject, jlong bitmapHandle) {
     LocalScopedBitmap bitmap(bitmapHandle);
-    return static_cast<jint>(bitmap->pixelRef()->getGenerationID());
+    return static_cast<jint>(bitmap->peekAtPixelRef()->getGenerationID());
 }
 
 static jboolean Bitmap_isPremultiplied(JNIEnv* env, jobject, jlong bitmapHandle) {
@@ -800,10 +817,10 @@ static void Bitmap_setHasAlpha(JNIEnv* env, jobject, jlong bitmapHandle,
         jboolean hasAlpha, jboolean requestPremul) {
     LocalScopedBitmap bitmap(bitmapHandle);
     if (hasAlpha) {
-        bitmap->pixelRef()->changeAlphaType(
+        bitmap->peekAtPixelRef()->changeAlphaType(
                 requestPremul ? kPremul_SkAlphaType : kUnpremul_SkAlphaType);
     } else {
-        bitmap->pixelRef()->changeAlphaType(kOpaque_SkAlphaType);
+        bitmap->peekAtPixelRef()->changeAlphaType(kOpaque_SkAlphaType);
     }
 }
 
@@ -812,9 +829,9 @@ static void Bitmap_setPremultiplied(JNIEnv* env, jobject, jlong bitmapHandle,
     LocalScopedBitmap bitmap(bitmapHandle);
     if (!bitmap->info().isOpaque()) {
         if (isPremul) {
-            bitmap->pixelRef()->changeAlphaType(kPremul_SkAlphaType);
+            bitmap->peekAtPixelRef()->changeAlphaType(kPremul_SkAlphaType);
         } else {
-            bitmap->pixelRef()->changeAlphaType(kUnpremul_SkAlphaType);
+            bitmap->peekAtPixelRef()->changeAlphaType(kUnpremul_SkAlphaType);
         }
     }
 }
@@ -1164,7 +1181,7 @@ static jboolean Bitmap_sameAs(JNIEnv* env, jobject, jlong bm0Handle,
 
 static jlong Bitmap_refPixelRef(JNIEnv* env, jobject, jlong bitmapHandle) {
     LocalScopedBitmap bitmap(bitmapHandle);
-    SkPixelRef* pixelRef = bitmap.valid() ? bitmap->pixelRef() : nullptr;
+    SkPixelRef* pixelRef = bitmap.valid() ? bitmap->peekAtPixelRef() : nullptr;
     SkSafeRef(pixelRef);
     return reinterpret_cast<jlong>(pixelRef);
 }
diff --git a/core/jni/android/graphics/Bitmap.h b/core/jni/android/graphics/Bitmap.h
index d6e5c61..efeb898 100644
--- a/core/jni/android/graphics/Bitmap.h
+++ b/core/jni/android/graphics/Bitmap.h
@@ -62,7 +62,8 @@ public:
     int width() const { return info().width(); }
     int height() const { return info().height(); }
     size_t rowBytes() const;
-    SkPixelRef* pixelRef() const;
+    SkPixelRef* peekAtPixelRef() const;
+    SkPixelRef* refPixelRef();
     bool valid() const { return mPixelStorageType != PixelStorageType::Invalid; }
 
     void reconfigure(const SkImageInfo& info, size_t rowBytes, SkColorTable* ctable);
@@ -88,6 +89,7 @@ private:
     JNIEnv* jniEnv();
     bool shouldDisposeSelfLocked();
     void assertValid() const;
+    SkPixelRef* refPixelRefLocked();
 
     android::Mutex mLock;
     int mPinnedRefCount = 0;
diff --git a/core/jni/android/graphics/BitmapFactory.cpp b/core/jni/android/graphics/BitmapFactory.cpp
index cdd397d..3ca4e72 100644
--- a/core/jni/android/graphics/BitmapFactory.cpp
+++ b/core/jni/android/graphics/BitmapFactory.cpp
@@ -184,7 +184,7 @@ public:
         }
 
         mBitmap->reconfigure(info, bitmap->rowBytes(), ctable);
-        bitmap->setPixelRef(mBitmap->pixelRef());
+        bitmap->setPixelRef(mBitmap->refPixelRef())->unref();
 
         // since we're already allocated, we lockPixels right away
         // HeapAllocator/JavaPixelAllocator behaves this way too
@@ -258,7 +258,7 @@ static jobject doDecode(JNIEnv* env, SkStreamRewindable* stream, jobject padding
     unsigned int existingBufferSize = 0;
     if (javaBitmap != NULL) {
         reuseBitmap = GraphicsJNI::getBitmap(env, javaBitmap);
-        if (reuseBitmap->pixelRef()->isImmutable()) {
+        if (reuseBitmap->peekAtPixelRef()->isImmutable()) {
             ALOGW("Unable to reuse an immutable bitmap as an image decoder target.");
             javaBitmap = NULL;
             reuseBitmap = nullptr;
diff --git a/core/jni/android/graphics/Graphics.cpp b/core/jni/android/graphics/Graphics.cpp
index 0deb8cc..1c6f7de 100644
--- a/core/jni/android/graphics/Graphics.cpp
+++ b/core/jni/android/graphics/Graphics.cpp
@@ -352,8 +352,8 @@ void GraphicsJNI::getSkBitmap(JNIEnv* env, jobject bitmap, SkBitmap* outBitmap)
     getBitmap(env, bitmap)->getSkBitmap(outBitmap);
 }
 
-SkPixelRef* GraphicsJNI::getSkPixelRef(JNIEnv* env, jobject bitmap) {
-    return getBitmap(env, bitmap)->pixelRef();
+SkPixelRef* GraphicsJNI::refSkPixelRef(JNIEnv* env, jobject bitmap) {
+    return getBitmap(env, bitmap)->refPixelRef();
 }
 
 SkColorType GraphicsJNI::getNativeBitmapColorType(JNIEnv* env, jobject jconfig) {
diff --git a/core/jni/android/graphics/GraphicsJNI.h b/core/jni/android/graphics/GraphicsJNI.h
index e748bac..ef9c2a9 100644
--- a/core/jni/android/graphics/GraphicsJNI.h
+++ b/core/jni/android/graphics/GraphicsJNI.h
@@ -52,7 +52,7 @@ public:
     static android::Canvas* getNativeCanvas(JNIEnv*, jobject canvas);
     static android::Bitmap* getBitmap(JNIEnv*, jobject bitmap);
     static void getSkBitmap(JNIEnv*, jobject bitmap, SkBitmap* outBitmap);
-    static SkPixelRef* getSkPixelRef(JNIEnv*, jobject bitmap);
+    static SkPixelRef* refSkPixelRef(JNIEnv*, jobject bitmap);
     static SkRegion* getNativeRegion(JNIEnv*, jobject region);
 
     // Given the 'native' long held by the Rasterizer.java object, return a
diff --git a/core/jni/android_view_SurfaceControl.cpp b/core/jni/android_view_SurfaceControl.cpp
index 1965cd3..77af341 100644
--- a/core/jni/android_view_SurfaceControl.cpp
+++ b/core/jni/android_view_SurfaceControl.cpp
@@ -180,7 +180,7 @@ static jobject nativeScreenshotBitmap(JNIEnv* env, jclass clazz,
             (void*) screenshot->getPixels(), (void*) screenshot.get(), DeleteScreenshot,
             screenshotInfo, rowBytes, nullptr);
     screenshot.detach();
-    bitmap->pixelRef()->setImmutable();
+    bitmap->peekAtPixelRef()->setImmutable();
 
     return GraphicsJNI::createBitmap(env, bitmap,
             GraphicsJNI::kBitmapCreateFlag_Premultiplied, NULL);