am 20df46ed: am 3ed91a81: Merge "Fix race in bitmap decode vs release."

* commit '20df46edeb2ddac7be4a4587e2ce210a735a1871':
  Fix race in bitmap decode vs release.

1 files changed, 39 insertions, 20 deletions

diff --git a/framesequence/src/android/support/rastermill/FrameSequenceDrawable.java b/framesequence/src/android/support/rastermill/FrameSequenceDrawable.java
index af25c82..2228b1d 100644
--- a/framesequence/src/android/support/rastermill/FrameSequenceDrawable.java
+++ b/framesequence/src/android/support/rastermill/FrameSequenceDrawable.java
@@ -161,13 +161,26 @@ public class FrameSequenceDrawable extends Drawable implements Animatable, Runna
             int lastFrame = nextFrame - 2;
             long invalidateTimeMs = mFrameSequenceState.getFrame(nextFrame, bitmap, lastFrame);
 
+            boolean schedule = false;
+            Bitmap bitmapToRelease = null;
             synchronized (mLock) {
-                if (mNextFrameToDecode < 0 || mState != STATE_DECODING) return;
-                mNextSwap = invalidateTimeMs + mLastSwap;
-
-                mState = STATE_WAITING_TO_SWAP;
+                if (mDestroyed) {
+                    bitmapToRelease = mBackBitmap;
+                    mBackBitmap = null;
+                } else if (mNextFrameToDecode >= 0 && mState == STATE_DECODING) {
+                    schedule = true;
+                    mNextSwap = invalidateTimeMs + mLastSwap;
+                    mState = STATE_WAITING_TO_SWAP;
+                }
+            }
+            if (schedule) {
+                scheduleSelf(FrameSequenceDrawable.this, mNextSwap);
+            }
+            if (bitmapToRelease != null) {
+                // destroy the bitmap here, since there's no safe way to get back to
+                // drawable thread - drawable is likely detached, so schedule is noop.
+                mBitmapProvider.releaseBitmap(bitmapToRelease);
             }
-            scheduleSelf(FrameSequenceDrawable.this, mNextSwap);
         }
     };
 
@@ -238,30 +251,31 @@ public class FrameSequenceDrawable extends Drawable implements Animatable, Runna
      * If no BitmapProvider is attached to the drawable, recycle() is called on the Bitmaps.
      */
     public void destroy() {
-        destroy(mBitmapProvider);
-    }
-
-    private void destroy(BitmapProvider bitmapProvider) {
-        if (bitmapProvider == null) {
+        if (mBitmapProvider == null) {
             throw new IllegalStateException("BitmapProvider must be non-null");
         }
 
         Bitmap bitmapToReleaseA;
-        Bitmap bitmapToReleaseB;
+        Bitmap bitmapToReleaseB = null;
         synchronized (mLock) {
             checkDestroyedLocked();
 
             bitmapToReleaseA = mFrontBitmap;
-            bitmapToReleaseB = mBackBitmap;
-
             mFrontBitmap = null;
-            mBackBitmap = null;
+
+            if (mState != STATE_DECODING) {
+                bitmapToReleaseB = mBackBitmap;
+                mBackBitmap = null;
+            }
+
             mDestroyed = true;
         }
 
         // For simplicity and safety, we don't destroy the state object here
-        bitmapProvider.releaseBitmap(bitmapToReleaseA);
-        bitmapProvider.releaseBitmap(bitmapToReleaseB);
+        mBitmapProvider.releaseBitmap(bitmapToReleaseA);
+        if (bitmapToReleaseB != null) {
+            mBitmapProvider.releaseBitmap(bitmapToReleaseB);
+        }
     }
 
     @Override
@@ -322,12 +336,17 @@ public class FrameSequenceDrawable extends Drawable implements Animatable, Runna
 
     @Override
     public void run() {
-        // set ready to swap
+        // set ready to swap as necessary
+        boolean invalidate = false;
         synchronized (mLock) {
-            if (mState != STATE_WAITING_TO_SWAP || mNextFrameToDecode < 0) return;
-            mState = STATE_READY_TO_SWAP;
+            if (mNextFrameToDecode >= 0 && mState == STATE_WAITING_TO_SWAP) {
+                mState = STATE_READY_TO_SWAP;
+                invalidate = true;
+            }
+        }
+        if (invalidate) {
+            invalidateSelf();
         }
-        invalidateSelf();
     }
 
     @Override