Clean up GifDrawable resources more reliably.

Each time we call get() on a drawable resource, we
get a new Drawable. We call get() repeatedly on
resources when they are retrieved from either the
set of active resources or the in memory cache.
Each time we create a new GifDrawable it holds on
to one or two temporary Bitmaps outside it's
shared state to render the current frame and obey
the dispose_previous method. This change means we
more aggressively cleanup those resources when
we think each Drawable is no longer being used.
The side affect is that we may reset back to
the beginning of the Drawable in some
circumstances.

Cleanup in in memory resources makes it less
likely that frames would be retrieved from
in memory, so this also works toward #207.

library/src/androidTest/java/com/bumptech/glide/load/engine/cache/MemorySizeCalculatorTest.java

@@ -3,6 +3,7 @@ package com.bumptech.glide.load.engine.cache;
 import android.app.ActivityManager;
 import android.content.Context;
 import android.os.Build;
+import com.bumptech.glide.tests.Util;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -30,12 +31,9 @@ public class MemorySizeCalculatorTest {
 
     @After
     public void tearDown() {
-        setSdkVersionInt(initialSdkVersion);
+        Util.setSdkVersionInt(initialSdkVersion);
     }
 
-    private void setSdkVersionInt(int version) {
-        Robolectric.Reflection.setFinalStaticField(Build.VERSION.class, "SDK_INT", version);
-    }
 
     @Test
     public void testDefaultMemoryCacheSizeIsTwiceScreenSize() {
@@ -103,7 +101,7 @@ public class MemorySizeCalculatorTest {
         final int normalMemoryCacheSize = harness.getCalculator().getMemoryCacheSize();
         final int normalBitmapPoolSize = harness.getCalculator().getBitmapPoolSize();
 
-        setSdkVersionInt(10);
+        Util.setSdkVersionInt(10);
 
         final int smallMemoryCacheSize = harness.getCalculator().getMemoryCacheSize();
         final int smallBitmapPoolSize = harness.getCalculator().getBitmapPoolSize();

library/src/androidTest/java/com/bumptech/glide/load/resource/bitmap/BitmapResourceTest.java

@@ -3,11 +3,11 @@ package com.bumptech.glide.load.resource.bitmap;
 import android.graphics.Bitmap;
 import android.os.Build;
 import com.bumptech.glide.load.engine.bitmap_recycle.BitmapPool;
+import com.bumptech.glide.tests.Util;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.robolectric.Robolectric;
 import org.robolectric.RobolectricTestRunner;
 
 import static org.junit.Assert.assertEquals;
@@ -32,7 +32,7 @@ public class BitmapResourceTest {
 
     @After
     public void tearDown() {
-        Robolectric.Reflection.setFinalStaticField(Build.VERSION.class, "SDK_INT", currentBuildVersion);
+        Util.setSdkVersionInt(currentBuildVersion);
     }
 
     @Test
@@ -42,7 +42,7 @@ public class BitmapResourceTest {
 
     @Test
     public void testSizeIsBasedOnDimensPreKitKat() {
-        Robolectric.Reflection.setFinalStaticField(Build.VERSION.class, "SDK_INT", 18);
+        Util.setSdkVersionInt(18);
         assertEquals(harness.bitmap.getWidth() * harness.bitmap.getHeight() * 4, harness.resource.getSize());
     }
 

library/src/androidTest/java/com/bumptech/glide/load/resource/gif/GifDrawableTest.java

@@ -6,12 +6,15 @@ import android.graphics.Paint;
 import android.graphics.PixelFormat;
 import android.graphics.Rect;
 import android.graphics.drawable.Drawable;
+import android.os.Build;
 import com.bumptech.glide.gifdecoder.GifDecoder;
 import com.bumptech.glide.gifdecoder.GifHeader;
 import com.bumptech.glide.load.Transformation;
 import com.bumptech.glide.load.engine.bitmap_recycle.BitmapPool;
 import com.bumptech.glide.load.resource.drawable.GlideDrawable;
 import com.bumptech.glide.tests.GlideShadowLooper;
+import com.bumptech.glide.tests.Util;
+import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -43,6 +46,7 @@ public class GifDrawableTest {
     private int frameWidth;
     private Bitmap firstFrame;
     private BitmapPool bitmapPool;
+    private int initialSdkVersion;
 
     @Before
     public void setUp() {
@@ -53,6 +57,12 @@ public class GifDrawableTest {
         bitmapPool = mock(BitmapPool.class);
         drawable = new GifDrawable(gifDecoder, frameManager, firstFrame, bitmapPool);
         drawable.setCallback(cb);
+        initialSdkVersion = Build.VERSION.SDK_INT;
+    }
+
+    @After
+    public void tearDown() {
+        Util.setSdkVersionInt(initialSdkVersion);
     }
 
     @Test
@@ -199,7 +209,7 @@ public class GifDrawableTest {
     }
 
     @Test
-    public void testStopsWhenCurrentFrameFinishesIfHasNoCallback() {
+    public void testStopsWhenCurrentFrameFinishesIfHasNoCallbackAndIsAtLeastAtHoneycomb() {
         drawable.setIsRunning(true);
         drawable.setCallback(null);
         drawable.onFrameRead(0);
@@ -207,6 +217,38 @@ public class GifDrawableTest {
         assertFalse(drawable.isRunning());
     }
 
+    @Test
+    public void testDoesNotStopWhenCurrentFrameFinishesIfHasNoCallbackAndIsPreHoneycomb() {
+        Util.setSdkVersionInt(10);
+
+        drawable.setIsRunning(true);
+        drawable.setCallback(null);
+        drawable.onFrameRead(0);
+
+        assertTrue(drawable.isRunning());
+    }
+
+    @Test
+    public void testResetsFrameManagerWhenCurrentFinishesIfHasNoCallbackAndIsAtLeastAtHoneycomb() {
+        drawable.setIsRunning(true);
+        drawable.setCallback(null);
+        drawable.onFrameRead(0);
+
+
+        verify(frameManager).clear();
+    }
+
+    @Test
+    public void testDoesNotResetFrameManagerWhenCurrentFinishesIfHasNoCallbackPreHoneycomb() {
+        Util.setSdkVersionInt(10);
+
+        drawable.setIsRunning(true);
+        drawable.setCallback(null);
+        drawable.onFrameRead(0);
+
+        verify(frameManager, never()).clear();
+    }
+
     @Test
     public void testSetsIsRunningFalseOnStop() {
         drawable.start();
@@ -224,6 +266,26 @@ public class GifDrawableTest {
         assertFalse(drawable.isRunning());
     }
 
+    @Test
+    public void testDoesNotResetOnStopIfAtLeastAtHoneycomb() {
+        drawable.start();
+        drawable.stop();
+
+        verify(frameManager, never()).clear();
+        // invalidate once from start.
+        verify(cb, times(1)).invalidateDrawable(eq(drawable));
+    }
+
+    @Test
+    public void testDoesResetOnStopIfPreHoneycomb() {
+        Util.setSdkVersionInt(10);
+        drawable.start();
+        drawable.stop();
+
+        verify(frameManager).clear();
+        verify(cb, times(2)).invalidateDrawable(eq(drawable));
+    }
+
     @Test
     public void testStartsOnSetVisibleTrueIfRunning() {
         drawable.start();

library/src/androidTest/java/com/bumptech/glide/manager/RequestManagerRetrieverTest.java

@@ -10,6 +10,7 @@ import android.support.v4.app.FragmentActivity;
 import com.bumptech.glide.RequestManager;
 import com.bumptech.glide.tests.BackgroundUtil;
 import com.bumptech.glide.tests.GlideShadowLooper;
+import com.bumptech.glide.tests.Util;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -50,17 +51,13 @@ public class RequestManagerRetrieverTest {
 
     @After
     public void tearDown() {
-        setSdkVersionInt(initialSdkVersion);
+        Util.setSdkVersionInt(initialSdkVersion);
 
         Robolectric.shadowOf(Looper.getMainLooper()).runToEndOfTasks();
         assertThat(retriever.pendingRequestManagerFragments.entrySet(), empty());
         assertThat(retriever.pendingSupportRequestManagerFragments.entrySet(), empty());
     }
 
-    private void setSdkVersionInt(int version) {
-        Robolectric.Reflection.setFinalStaticField(Build.VERSION.class, "SDK_INT", version);
-    }
-
     @Test
     public void testCreatesNewFragmentIfNoneExists() {
         for (RetrieverHarness harness : harnesses) {
@@ -261,7 +258,7 @@ public class RequestManagerRetrieverTest {
 
     @Test
     public void testDoesNotThrowIfAskedToGetManagerForActivityPreHoneycomb() {
-        setSdkVersionInt(Build.VERSION_CODES.GINGERBREAD_MR1);
+        Util.setSdkVersionInt(Build.VERSION_CODES.GINGERBREAD_MR1);
         Activity activity = mock(Activity.class);
         when(activity.getApplicationContext()).thenReturn(Robolectric.application);
         when(activity.getFragmentManager()).thenThrow(new NoSuchMethodError());
@@ -271,7 +268,7 @@ public class RequestManagerRetrieverTest {
 
     @Test
     public void testDoesNotThrowIfAskedToGetManagerForActivityPreJellYBeanMr1() {
-        setSdkVersionInt(Build.VERSION_CODES.JELLY_BEAN);
+        Util.setSdkVersionInt(Build.VERSION_CODES.JELLY_BEAN);
         Activity activity = Robolectric.buildActivity(Activity.class).create().start().resume().get();
         Activity spyActivity = Mockito.spy(activity);
         when(spyActivity.isDestroyed()).thenThrow(new NoSuchMethodError());
@@ -281,7 +278,7 @@ public class RequestManagerRetrieverTest {
 
     @Test
     public void testDoesNotThrowIfAskedToGetManagerForFragmentPreHoneyCombMr2() {
-        setSdkVersionInt(Build.VERSION_CODES.HONEYCOMB_MR1);
+        Util.setSdkVersionInt(Build.VERSION_CODES.HONEYCOMB_MR1);
         Activity activity = Robolectric.buildActivity(Activity.class).create().start().resume().get();
         android.app.Fragment fragment = new android.app.Fragment();
 
@@ -296,7 +293,7 @@ public class RequestManagerRetrieverTest {
 
     @Test
     public void testDoesNotThrowIfAskedToGetManagerForFragmentPreJellyBeanMr1() {
-        setSdkVersionInt(Build.VERSION_CODES.JELLY_BEAN);
+        Util.setSdkVersionInt(Build.VERSION_CODES.JELLY_BEAN);
         Activity activity = Robolectric.buildActivity(Activity.class).create().start().resume().get();
         android.app.Fragment fragment = new android.app.Fragment();
 

library/src/androidTest/java/com/bumptech/glide/tests/Util.java

 package com.bumptech.glide.tests;
 
+import android.os.Build;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
+import org.robolectric.Robolectric;
 
 import java.io.File;
 import java.io.FileInputStream;
@@ -70,4 +72,9 @@ public class Util {
             }
         };
     }
+
+    public static void setSdkVersionInt(int version) {
+        Robolectric.Reflection.setFinalStaticField(Build.VERSION.class, "SDK_INT", version);
+    }
+
 }

library/src/main/java/com/bumptech/glide/load/resource/gif/GifDrawable.java

@@ -133,6 +133,22 @@ public class GifDrawable extends GlideDrawable implements GifFrameManager.FrameC
     public void stop() {
         isStarted = false;
         stopRunning();
+
+        // On APIs > honeycomb we know our drawable is not being displayed anymore when it's callback is cleared and so
+        // we can use the absence of a callback as an indication that it's ok to clear our temporary data. Prior to
+        // honeycomb we can't tell if our callback is null and instead eagerly reset to avoid holding on to resources we
+        // no longer need.
+        if (Build.VERSION.SDK_INT < Build.VERSION_CODES.HONEYCOMB) {
+            reset();
+        }
+    }
+
+    /**
+     * Clears temporary data and resets the drawable back to the first frame.
+     */
+    private void reset() {
+        frameManager.clear();
+        invalidateSelf();
     }
 
     private void startRunning() {
@@ -222,8 +238,9 @@ public class GifDrawable extends GlideDrawable implements GifFrameManager.FrameC
     @TargetApi(Build.VERSION_CODES.HONEYCOMB)
     @Override
     public void onFrameRead(int frameIndex) {
-        if (Build.VERSION_CODES.HONEYCOMB <= Build.VERSION.SDK_INT && getCallback() == null) {
+        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.HONEYCOMB && getCallback() == null) {
             stop();
+            reset();
             return;
         }
         if (!isRunning) {

library/src/main/java/com/bumptech/glide/load/resource/gif/GifFrameManager.java

@@ -100,11 +100,15 @@ class GifFrameManager {
         if (current != null) {
             mainHandler.removeCallbacks(current);
             Glide.clear(current);
+            current = null;
         }
         if (next != null) {
             mainHandler.removeCallbacks(next);
             Glide.clear(next);
+            next = null;
         }
+
+        decoder.resetFrameIndex();
     }
 
     class DelayTarget extends SimpleTarget<Bitmap> implements Runnable {

samples/giphy/src/main/java/com/bumptech/glide/samples/giphy/FullscreenActivity.java

@@ -11,7 +11,10 @@ import android.widget.ImageView;
 import com.bumptech.glide.Glide;
 import com.bumptech.glide.load.engine.DiskCacheStrategy;
 import com.bumptech.glide.load.resource.drawable.GlideDrawable;
+import com.bumptech.glide.load.resource.gif.GifDrawable;
 import com.bumptech.glide.load.resource.transcode.BitmapToGlideDrawableTranscoder;
+import com.bumptech.glide.request.RequestListener;
+import com.bumptech.glide.request.target.Target;
 import com.google.gson.Gson;
 
 /**
@@ -19,6 +22,7 @@ import com.google.gson.Gson;
  */
 public class FullscreenActivity extends Activity {
     private static final String EXTRA_RESULT_JSON = "result_json";
+    private GifDrawable gifDrawable;
 
     public static Intent getIntent(Context context, Api.GifResult result) {
         Intent intent = new Intent(context, FullscreenActivity.class);
@@ -42,6 +46,14 @@ public class FullscreenActivity extends Activity {
                 ClipboardManager clipboard = (ClipboardManager) getSystemService(Context.CLIPBOARD_SERVICE);
                 ClipData clip = ClipData.newPlainText("giphy_url", result.images.original.url);
                 clipboard.setPrimaryClip(clip);
+
+                if (gifDrawable != null) {
+                    if (gifDrawable.isRunning()) {
+                        gifDrawable.stop();
+                    } else {
+                        gifDrawable.start();
+                    }
+                }
             }
         });
 
@@ -54,6 +66,24 @@ public class FullscreenActivity extends Activity {
                         .transcode(new BitmapToGlideDrawableTranscoder(this), GlideDrawable.class)
                         .diskCacheStrategy(DiskCacheStrategy.SOURCE)
                 )
+                .listener(new RequestListener<String, GlideDrawable>() {
+                    @Override
+                    public boolean onException(Exception e, String model, Target<GlideDrawable> target,
+                            boolean isFirstResource) {
+                        return false;
+                    }
+
+                    @Override
+                    public boolean onResourceReady(GlideDrawable resource, String model, Target<GlideDrawable> target,
+                            boolean isFromMemoryCache, boolean isFirstResource) {
+                        if (resource instanceof GifDrawable) {
+                            gifDrawable = (GifDrawable) resource;
+                        } else {
+                            gifDrawable = null;
+                        }
+                        return false;
+                    }
+                })
                 .into(gifView);
     }
 }

third_party/gif_decoder/src/main/java/com/bumptech/glide/gifdecoder/GifDecoder.java

@@ -230,6 +230,10 @@ public class GifDecoder {
         return framePointer;
     }
 
+    public void resetFrameIndex() {
+        framePointer = -1;
+    }
+
     /**
      * Gets the "Netscape" iteration count, if any. A count of 0 means repeat indefinitely.
      *