diff --git a/library/src/androidTest/java/com/bumptech/glide/load/engine/cache/MemorySizeCalculatorTest.java b/library/src/androidTest/java/com/bumptech/glide/load/engine/cache/MemorySizeCalculatorTest.java index 20b7f1233ec49c15e95b6fad5a7ef1f43c67c6b9..ed0c7d90a7dd2c870555fc81e560f38c10709705 100644 --- a/library/src/androidTest/java/com/bumptech/glide/load/engine/cache/MemorySizeCalculatorTest.java +++ b/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(); diff --git a/library/src/androidTest/java/com/bumptech/glide/load/resource/bitmap/BitmapResourceTest.java b/library/src/androidTest/java/com/bumptech/glide/load/resource/bitmap/BitmapResourceTest.java index 289384044e22af4fcd453f9825bdc038db43f88c..19eefe832118018d3b68427e60f6d0ebb6f29d94 100644 --- a/library/src/androidTest/java/com/bumptech/glide/load/resource/bitmap/BitmapResourceTest.java +++ b/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()); } diff --git a/library/src/androidTest/java/com/bumptech/glide/load/resource/gif/GifDrawableTest.java b/library/src/androidTest/java/com/bumptech/glide/load/resource/gif/GifDrawableTest.java index 3e66d83ec07767de945c830e8662099fb663cccc..f5db285bd2573c1696d0f8dd4120e69a8b943017 100644 --- a/library/src/androidTest/java/com/bumptech/glide/load/resource/gif/GifDrawableTest.java +++ b/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(); diff --git a/library/src/androidTest/java/com/bumptech/glide/manager/RequestManagerRetrieverTest.java b/library/src/androidTest/java/com/bumptech/glide/manager/RequestManagerRetrieverTest.java index a5d4599bfb192a14614079084f133d38b292aff6..90711322b942b2302674e9efadcb7fafc464a3db 100644 --- a/library/src/androidTest/java/com/bumptech/glide/manager/RequestManagerRetrieverTest.java +++ b/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(); diff --git a/library/src/androidTest/java/com/bumptech/glide/tests/Util.java b/library/src/androidTest/java/com/bumptech/glide/tests/Util.java index b69ac7f4f277f4c55457c5d97ef584b7fac7b156..73220f4518bdcd375490ff28792a92a20deaf5c3 100644 --- a/library/src/androidTest/java/com/bumptech/glide/tests/Util.java +++ b/library/src/androidTest/java/com/bumptech/glide/tests/Util.java @@ -1,7 +1,9 @@ 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); + } + } diff --git a/library/src/main/java/com/bumptech/glide/load/resource/gif/GifDrawable.java b/library/src/main/java/com/bumptech/glide/load/resource/gif/GifDrawable.java index acf1058d4e2a269bea0a2ed8d7e26481c6ba8c24..fd5ea78243ba7a9e891bd224f62fd5487c9b28ba 100644 --- a/library/src/main/java/com/bumptech/glide/load/resource/gif/GifDrawable.java +++ b/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) { diff --git a/library/src/main/java/com/bumptech/glide/load/resource/gif/GifFrameManager.java b/library/src/main/java/com/bumptech/glide/load/resource/gif/GifFrameManager.java index 42eea5d0e629b14f76828146b55e2c75fd92d98a..661e84d9498cdae59ee697e10d84404a435e2fab 100644 --- a/library/src/main/java/com/bumptech/glide/load/resource/gif/GifFrameManager.java +++ b/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 implements Runnable { diff --git a/samples/giphy/src/main/java/com/bumptech/glide/samples/giphy/FullscreenActivity.java b/samples/giphy/src/main/java/com/bumptech/glide/samples/giphy/FullscreenActivity.java index 6a30ccc440f4de800035e0fdb84741f374971256..7da7a6787ce100279e90fee5710a7ca1eecc7b22 100644 --- a/samples/giphy/src/main/java/com/bumptech/glide/samples/giphy/FullscreenActivity.java +++ b/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() { + @Override + public boolean onException(Exception e, String model, Target target, + boolean isFirstResource) { + return false; + } + + @Override + public boolean onResourceReady(GlideDrawable resource, String model, Target target, + boolean isFromMemoryCache, boolean isFirstResource) { + if (resource instanceof GifDrawable) { + gifDrawable = (GifDrawable) resource; + } else { + gifDrawable = null; + } + return false; + } + }) .into(gifView); } } diff --git a/third_party/gif_decoder/src/main/java/com/bumptech/glide/gifdecoder/GifDecoder.java b/third_party/gif_decoder/src/main/java/com/bumptech/glide/gifdecoder/GifDecoder.java index df891af7fd1b850ba65994dfa8964cdee22daac0..92dc3bfc26a0bde7ccc041bbfea1c0a2e90ffb65 100644 --- a/third_party/gif_decoder/src/main/java/com/bumptech/glide/gifdecoder/GifDecoder.java +++ b/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. *