Attempt to solve the double-GC problem

Fix the issue where Bitmap requires two GC passes
to release its byte[] by using some questionable
ref-counting hacks to manage whether or not
native has a strong or weak ref to the byte[]

Change-Id: Ia90a883579f61c0b1904b5549a66bd0ef34b32c5

1 files changed, 46 insertions, 42 deletions

diff --git a/graphics/java/android/graphics/Bitmap.java b/graphics/java/android/graphics/Bitmap.java
index be5c52b..c850b07 100644
--- a/graphics/java/android/graphics/Bitmap.java
+++ b/graphics/java/android/graphics/Bitmap.java
@@ -41,13 +41,13 @@ public final class Bitmap implements Parcelable {
      */
     public static final int DENSITY_NONE = 0;
 
-    private final long mSkBitmapPtr;
-
     /**
      * Backing buffer for the Bitmap.
      */
     private byte[] mBuffer;
 
+    // Convenience for JNI access
+    private final long mNativePtr;
     private final BitmapFinalizer mFinalizer;
 
     private final boolean mIsMutable;
@@ -115,17 +115,16 @@ public final class Bitmap implements Parcelable {
         mRequestPremultiplied = requestPremultiplied;
         mBuffer = buffer;
 
-        // we delete this in our finalizer
-        mSkBitmapPtr = nativeBitmap;
-
         mNinePatchChunk = ninePatchChunk;
         mNinePatchInsets = ninePatchInsets;
         if (density >= 0) {
             mDensity = density;
         }
 
-        int nativeAllocationByteCount = buffer == null ? getByteCount() : 0;
-        mFinalizer = new BitmapFinalizer(nativeBitmap, nativeAllocationByteCount);
+        mNativePtr = nativeBitmap;
+        mFinalizer = new BitmapFinalizer(nativeBitmap);
+        int nativeAllocationByteCount = (buffer == null ? getByteCount() : 0);
+        mFinalizer.setNativeAllocationByteCount(nativeAllocationByteCount);
     }
 
     /**
@@ -223,8 +222,8 @@ public final class Bitmap implements Parcelable {
             throw new IllegalStateException("native-backed bitmaps may not be reconfigured");
         }
 
-        nativeReconfigure(mSkBitmapPtr, width, height, config.nativeInt, mBuffer.length,
-                mRequestPremultiplied);
+        nativeReconfigure(mFinalizer.mNativeBitmap, width, height, config.nativeInt,
+                mBuffer.length, mRequestPremultiplied);
         mWidth = width;
         mHeight = height;
     }
@@ -301,7 +300,7 @@ public final class Bitmap implements Parcelable {
      */
     public void recycle() {
         if (!mRecycled && mFinalizer.mNativeBitmap != 0) {
-            if (nativeRecycle(mSkBitmapPtr)) {
+            if (nativeRecycle(mFinalizer.mNativeBitmap)) {
                 // return value indicates whether native pixel object was actually recycled.
                 // false indicates that it is still in use at the native level and these
                 // objects should not be collected now. They will be collected later when the
@@ -331,7 +330,7 @@ public final class Bitmap implements Parcelable {
      * @return The current generation ID for this bitmap.
      */
     public int getGenerationId() {
-        return nativeGenerationId(mSkBitmapPtr);
+        return nativeGenerationId(mFinalizer.mNativeBitmap);
     }
 
     /**
@@ -487,7 +486,7 @@ public final class Bitmap implements Parcelable {
             throw new RuntimeException("Buffer not large enough for pixels");
         }
 
-        nativeCopyPixelsToBuffer(mSkBitmapPtr, dst);
+        nativeCopyPixelsToBuffer(mFinalizer.mNativeBitmap, dst);
 
         // now update the buffer's position
         int position = dst.position();
@@ -527,7 +526,7 @@ public final class Bitmap implements Parcelable {
             throw new RuntimeException("Buffer not large enough for pixels");
         }
 
-        nativeCopyPixelsFromBuffer(mSkBitmapPtr, src);
+        nativeCopyPixelsFromBuffer(mFinalizer.mNativeBitmap, src);
 
         // now update the buffer's position
         int position = src.position();
@@ -549,7 +548,7 @@ public final class Bitmap implements Parcelable {
      */
     public Bitmap copy(Config config, boolean isMutable) {
         checkRecycled("Can't copy a recycled bitmap");
-        Bitmap b = nativeCopy(mSkBitmapPtr, config.nativeInt, isMutable);
+        Bitmap b = nativeCopy(mFinalizer.mNativeBitmap, config.nativeInt, isMutable);
         if (b != null) {
             b.setPremultiplied(mRequestPremultiplied);
             b.mDensity = mDensity;
@@ -810,7 +809,7 @@ public final class Bitmap implements Parcelable {
         }
         bm.setHasAlpha(hasAlpha);
         if (config == Config.ARGB_8888 && !hasAlpha) {
-            nativeErase(bm.mSkBitmapPtr, 0xff000000);
+            nativeErase(bm.mFinalizer.mNativeBitmap, 0xff000000);
         }
         // No need to initialize the bitmap to zeroes with other configs;
         // it is backed by a VM byte array which is by definition preinitialized
@@ -1000,8 +999,8 @@ public final class Bitmap implements Parcelable {
             throw new IllegalArgumentException("quality must be 0..100");
         }
         Trace.traceBegin(Trace.TRACE_TAG_RESOURCES, "Bitmap.compress");
-        boolean result = nativeCompress(mSkBitmapPtr, format.nativeInt, quality,
-                              stream, new byte[WORKING_COMPRESS_STORAGE]);
+        boolean result = nativeCompress(mFinalizer.mNativeBitmap, format.nativeInt,
+                quality, stream, new byte[WORKING_COMPRESS_STORAGE]);
         Trace.traceEnd(Trace.TRACE_TAG_RESOURCES);
         return result;
     }
@@ -1041,7 +1040,7 @@ public final class Bitmap implements Parcelable {
      * @see BitmapFactory.Options#inPremultiplied
      */
     public final boolean isPremultiplied() {
-        return nativeIsPremultiplied(mSkBitmapPtr);
+        return nativeIsPremultiplied(mFinalizer.mNativeBitmap);
     }
 
     /**
@@ -1066,7 +1065,7 @@ public final class Bitmap implements Parcelable {
      */
     public final void setPremultiplied(boolean premultiplied) {
         mRequestPremultiplied = premultiplied;
-        nativeSetPremultiplied(mSkBitmapPtr, premultiplied);
+        nativeSetPremultiplied(mFinalizer.mNativeBitmap, premultiplied);
     }
 
     /** Returns the bitmap's width */
@@ -1158,7 +1157,7 @@ public final class Bitmap implements Parcelable {
      * @return number of bytes between rows of the native bitmap pixels.
      */
     public final int getRowBytes() {
-        return nativeRowBytes(mSkBitmapPtr);
+        return nativeRowBytes(mFinalizer.mNativeBitmap);
     }
 
     /**
@@ -1201,7 +1200,7 @@ public final class Bitmap implements Parcelable {
      * that config, otherwise return null.
      */
     public final Config getConfig() {
-        return Config.nativeToConfig(nativeConfig(mSkBitmapPtr));
+        return Config.nativeToConfig(nativeConfig(mFinalizer.mNativeBitmap));
     }
 
     /** Returns true if the bitmap's config supports per-pixel alpha, and
@@ -1213,7 +1212,7 @@ public final class Bitmap implements Parcelable {
      * it will return true by default.
      */
     public final boolean hasAlpha() {
-        return nativeHasAlpha(mSkBitmapPtr);
+        return nativeHasAlpha(mFinalizer.mNativeBitmap);
     }
 
     /**
@@ -1227,7 +1226,7 @@ public final class Bitmap implements Parcelable {
      * non-opaque per-pixel alpha values.
      */
     public void setHasAlpha(boolean hasAlpha) {
-        nativeSetHasAlpha(mSkBitmapPtr, hasAlpha, mRequestPremultiplied);
+        nativeSetHasAlpha(mFinalizer.mNativeBitmap, hasAlpha, mRequestPremultiplied);
     }
 
     /**
@@ -1248,7 +1247,7 @@ public final class Bitmap implements Parcelable {
      * @see #setHasMipMap(boolean)
      */
     public final boolean hasMipMap() {
-        return nativeHasMipMap(mSkBitmapPtr);
+        return nativeHasMipMap(mFinalizer.mNativeBitmap);
     }
 
     /**
@@ -1272,7 +1271,7 @@ public final class Bitmap implements Parcelable {
      * @see #hasMipMap()
      */
     public final void setHasMipMap(boolean hasMipMap) {
-        nativeSetHasMipMap(mSkBitmapPtr, hasMipMap);
+        nativeSetHasMipMap(mFinalizer.mNativeBitmap, hasMipMap);
     }
 
     /**
@@ -1285,7 +1284,7 @@ public final class Bitmap implements Parcelable {
         if (!isMutable()) {
             throw new IllegalStateException("cannot erase immutable bitmaps");
         }
-        nativeErase(mSkBitmapPtr, c);
+        nativeErase(mFinalizer.mNativeBitmap, c);
     }
 
     /**
@@ -1302,7 +1301,7 @@ public final class Bitmap implements Parcelable {
     public int getPixel(int x, int y) {
         checkRecycled("Can't call getPixel() on a recycled bitmap");
         checkPixelAccess(x, y);
-        return nativeGetPixel(mSkBitmapPtr, x, y);
+        return nativeGetPixel(mFinalizer.mNativeBitmap, x, y);
     }
 
     /**
@@ -1335,7 +1334,7 @@ public final class Bitmap implements Parcelable {
             return; // nothing to do
         }
         checkPixelsAccess(x, y, width, height, offset, stride, pixels);
-        nativeGetPixels(mSkBitmapPtr, pixels, offset, stride,
+        nativeGetPixels(mFinalizer.mNativeBitmap, pixels, offset, stride,
                         x, y, width, height);
     }
 
@@ -1416,7 +1415,7 @@ public final class Bitmap implements Parcelable {
             throw new IllegalStateException();
         }
         checkPixelAccess(x, y);
-        nativeSetPixel(mSkBitmapPtr, x, y, color);
+        nativeSetPixel(mFinalizer.mNativeBitmap, x, y, color);
     }
 
     /**
@@ -1452,7 +1451,7 @@ public final class Bitmap implements Parcelable {
             return; // nothing to do
         }
         checkPixelsAccess(x, y, width, height, offset, stride, pixels);
-        nativeSetPixels(mSkBitmapPtr, pixels, offset, stride,
+        nativeSetPixels(mFinalizer.mNativeBitmap, pixels, offset, stride,
                         x, y, width, height);
     }
 
@@ -1490,7 +1489,7 @@ public final class Bitmap implements Parcelable {
      */
     public void writeToParcel(Parcel p, int flags) {
         checkRecycled("Can't parcel a recycled bitmap");
-        if (!nativeWriteToParcel(mSkBitmapPtr, mIsMutable, mDensity, p)) {
+        if (!nativeWriteToParcel(mFinalizer.mNativeBitmap, mIsMutable, mDensity, p)) {
             throw new RuntimeException("native writeToParcel failed");
         }
     }
@@ -1536,7 +1535,7 @@ public final class Bitmap implements Parcelable {
     public Bitmap extractAlpha(Paint paint, int[] offsetXY) {
         checkRecycled("Can't extractAlpha on a recycled bitmap");
         long nativePaint = paint != null ? paint.getNativeInstance() : 0;
-        Bitmap bm = nativeExtractAlpha(mSkBitmapPtr, nativePaint, offsetXY);
+        Bitmap bm = nativeExtractAlpha(mFinalizer.mNativeBitmap, nativePaint, offsetXY);
         if (bm == null) {
             throw new RuntimeException("Failed to extractAlpha on Bitmap");
         }
@@ -1550,7 +1549,8 @@ public final class Bitmap implements Parcelable {
      *  If other is null, return false.
      */
     public boolean sameAs(Bitmap other) {
-        return this == other || (other != null && nativeSameAs(mSkBitmapPtr, other.mSkBitmapPtr));
+        return this == other || (other != null
+                && nativeSameAs(mFinalizer.mNativeBitmap, other.mFinalizer.mNativeBitmap));
     }
 
     /**
@@ -1565,7 +1565,9 @@ public final class Bitmap implements Parcelable {
      * and therefore is harmless.
      */
     public void prepareToDraw() {
-        nativePrepareToDraw(mSkBitmapPtr);
+        // TODO: Consider having this start an async upload?
+        // With inPurgeable no-op'd there's currently no use for this
+        // method, but it could have interesting future uses.
     }
 
     /**
@@ -1574,7 +1576,7 @@ public final class Bitmap implements Parcelable {
      * @hide
      * */
     public final long refSkPixelRef() {
-        return nativeRefPixelRef(mSkBitmapPtr);
+        return nativeRefPixelRef(mNativePtr);
     }
 
     private static class BitmapFinalizer {
@@ -1582,12 +1584,17 @@ public final class Bitmap implements Parcelable {
 
         // Native memory allocated for the duration of the Bitmap,
         // if pixel data allocated into native memory, instead of java byte[]
-        private final int mNativeAllocationByteCount;
+        private int mNativeAllocationByteCount;
 
-        BitmapFinalizer(long nativeBitmap, int nativeAllocationByteCount) {
+        BitmapFinalizer(long nativeBitmap) {
             mNativeBitmap = nativeBitmap;
-            mNativeAllocationByteCount = nativeAllocationByteCount;
+        }
 
+        public void setNativeAllocationByteCount(int nativeByteCount) {
+            if (mNativeAllocationByteCount != 0) {
+                VMRuntime.getRuntime().registerNativeFree(mNativeAllocationByteCount);
+            }
+            mNativeAllocationByteCount = nativeByteCount;
             if (mNativeAllocationByteCount != 0) {
                 VMRuntime.getRuntime().registerNativeAllocation(mNativeAllocationByteCount);
             }
@@ -1600,9 +1607,7 @@ public final class Bitmap implements Parcelable {
             } catch (Throwable t) {
                 // Ignore
             } finally {
-                if (mNativeAllocationByteCount != 0) {
-                    VMRuntime.getRuntime().registerNativeFree(mNativeAllocationByteCount);
-                }
+                setNativeAllocationByteCount(0);
                 nativeDestructor(mNativeBitmap);
                 mNativeBitmap = 0;
             }
@@ -1654,7 +1659,6 @@ public final class Bitmap implements Parcelable {
                                                     long nativePaint,
                                                     int[] offsetXY);
 
-    private static native void nativePrepareToDraw(long nativeBitmap);
     private static native boolean nativeHasAlpha(long nativeBitmap);
     private static native boolean nativeIsPremultiplied(long nativeBitmap);
     private static native void nativeSetPremultiplied(long nativeBitmap,