diff --git a/instrumentation/src/androidTest/java/com/bumptech/glide/load/resource/gif/GifDrawableTest.java b/instrumentation/src/androidTest/java/com/bumptech/glide/load/resource/gif/GifDrawableTest.java new file mode 100644 index 0000000000000000000000000000000000000000..5a78fb00dcf7fdb4773c230a64a3009c87607cc6 --- /dev/null +++ b/instrumentation/src/androidTest/java/com/bumptech/glide/load/resource/gif/GifDrawableTest.java @@ -0,0 +1,157 @@ +package com.bumptech.glide.load.resource.gif; + +import static android.support.test.InstrumentationRegistry.getTargetContext; +import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.fail; +import static org.junit.Assume.assumeTrue; + +import android.content.Context; +import android.graphics.drawable.Drawable; +import android.os.Build; +import android.os.Handler; +import android.os.Looper; +import android.support.annotation.Nullable; +import android.support.test.runner.AndroidJUnit4; +import android.view.View; +import android.view.WindowManager; +import android.view.WindowManager.LayoutParams; +import android.widget.ImageView; +import com.bumptech.glide.load.DataSource; +import com.bumptech.glide.load.engine.GlideException; +import com.bumptech.glide.load.resource.gif.GifDrawable.GifState; +import com.bumptech.glide.load.resource.gif.GifFrameLoader.OnEveryFrameListener; +import com.bumptech.glide.request.RequestListener; +import com.bumptech.glide.request.target.Target; +import com.bumptech.glide.test.GlideApp; +import com.bumptech.glide.test.ResourceIds; +import com.bumptech.glide.test.TearDownGlide; +import com.bumptech.glide.util.Preconditions; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TestName; +import org.junit.runner.RunWith; + +@RunWith(AndroidJUnit4.class) +public class GifDrawableTest { + @Rule public TestName testName = new TestName(); + @Rule public TearDownGlide tearDownGlide = new TearDownGlide(); + private Context context; + private Handler mainHandler; + + @Before + public void setUp() { + // Required for us to add a View to a Window. + assumeTrue(Build.VERSION.SDK_INT < Build.VERSION_CODES.M); + + context = getTargetContext(); + mainHandler = new Handler(Looper.getMainLooper()); + } + + @Test + public void loadGif_intoImageView_afterStop_restartsGif() + throws ExecutionException, InterruptedException { + // Mimic the state the Drawable can get into if it was loaded into a View previously and stopped + // so that it ended up with a pending frame that finished after the stop call. + final GifDrawable gifDrawable = GlideApp.with(context) + .asGif() + .load(ResourceIds.raw.dl_world_anim) + .submit(Target.SIZE_ORIGINAL, Target.SIZE_ORIGINAL) + .get(); + final CountDownLatch waitForGifFrame = new CountDownLatch(1); + // Starting/Stopping loads in GIFs must happen on the main thread. + mainHandler.post( + new Runnable() { + @Override + public void run() { + // Make sure a frame is loaded while the drawable is stopped. + GifState gifState = + (GifState) Preconditions.checkNotNull(gifDrawable.getConstantState()); + gifState.frameLoader.setOnEveryFrameReadyListener(new OnEveryFrameListener() { + @Override + public void onFrameReady() { + waitForGifFrame.countDown(); + } + }); + gifDrawable.start(); + gifDrawable.stop(); + } + }); + waitOrThrow(waitForGifFrame); + + // Load the Drawable with the pending frame into a new View and make sure it ends up in the + // running state. + final ImageView imageView = new ImageView(context); + final WaitForLoad waitForLoad = new WaitForLoad<>(); + // Starting loads into Views must happen on the main thread. + mainHandler + .post(new Runnable() { + @Override + public void run() { + addViewToWindow(imageView); + GlideApp.with(context) + .load(gifDrawable) + .listener(waitForLoad) + .override(Target.SIZE_ORIGINAL) + .into(imageView); + } + }); + waitForLoad.await(); + + GifDrawable drawableFromView = (GifDrawable) imageView.getDrawable(); + assertThat(drawableFromView.isRunning()).isTrue(); + } + + // LayoutParams.TYPE_SYSTEM_ALERT. + @SuppressWarnings("deprecation") + private void addViewToWindow(View view) { + final WindowManager.LayoutParams layoutParams = new WindowManager.LayoutParams(); + layoutParams.height = WindowManager.LayoutParams.MATCH_PARENT; + layoutParams.width = WindowManager.LayoutParams.MATCH_PARENT; + layoutParams.type = LayoutParams.TYPE_SYSTEM_ALERT; + final WindowManager windowManager = + (WindowManager) context.getSystemService(Context.WINDOW_SERVICE); + windowManager.addView(view, layoutParams); + } + + private static void waitOrThrow(CountDownLatch latch) { + try { + if (!latch.await(5, TimeUnit.SECONDS)) { + fail("Failed to reach expected condition in the alloted time."); + } + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + } + + private final class WaitForLoad implements RequestListener { + private final CountDownLatch countDownLatch = new CountDownLatch(1); + + void await() { + waitOrThrow(countDownLatch); + } + + @Override + public boolean onLoadFailed(@Nullable GlideException e, Object model, + Target target, + boolean isFirstResource) { + throw new RuntimeException(e); + } + + @Override + public boolean onResourceReady(T resource, Object model, Target target, + DataSource dataSource, + boolean isFirstResource) { + mainHandler.post(new Runnable() { + @Override + public void run() { + countDownLatch.countDown(); + } + }); + return false; + } + } +} diff --git a/instrumentation/src/androidTest/java/com/bumptech/glide/test/ResourceIds.java b/instrumentation/src/androidTest/java/com/bumptech/glide/test/ResourceIds.java index 047e983c974018207b5dc9d3618f13a4f01d0404..62268eb07ee34a4eeb7cbb36cd48f3b914390b44 100644 --- a/instrumentation/src/androidTest/java/com/bumptech/glide/test/ResourceIds.java +++ b/instrumentation/src/androidTest/java/com/bumptech/glide/test/ResourceIds.java @@ -13,6 +13,10 @@ public final class ResourceIds { // Utility class. } + public interface raw { + int dl_world_anim = getResourceId("raw", "dl_world_anim"); + } + public interface drawable { int bitmap_alias = getResourceId("drawable", "bitmap_alias"); int googlelogo_color_120x44dp= getResourceId("drawable", "googlelogo_color_120x44dp"); diff --git a/instrumentation/src/main/AndroidManifest.xml b/instrumentation/src/main/AndroidManifest.xml index d9857b303d76f8cb09ff3f2cca2581005762ce41..fa6e2915fe6e64763b60a965704053026fc22d0e 100644 --- a/instrumentation/src/main/AndroidManifest.xml +++ b/instrumentation/src/main/AndroidManifest.xml @@ -1,5 +1,6 @@ + diff --git a/instrumentation/src/main/res/raw/dl_world_anim.gif b/instrumentation/src/main/res/raw/dl_world_anim.gif new file mode 100644 index 0000000000000000000000000000000000000000..1e3b8dea2c8caba4f89b4c6044a2dcd48a4e31cd Binary files /dev/null and b/instrumentation/src/main/res/raw/dl_world_anim.gif differ diff --git a/library/src/main/java/com/bumptech/glide/load/resource/gif/GifFrameLoader.java b/library/src/main/java/com/bumptech/glide/load/resource/gif/GifFrameLoader.java index 26846d796c23dbc1b5bbf664fcf90e32c2542ef5..35db97423da490d4cf75f9ed1bf9569be1ee4366 100644 --- a/library/src/main/java/com/bumptech/glide/load/resource/gif/GifFrameLoader.java +++ b/library/src/main/java/com/bumptech/glide/load/resource/gif/GifFrameLoader.java @@ -8,6 +8,8 @@ import android.os.Handler; import android.os.Looper; import android.os.Message; import android.os.SystemClock; +import android.support.annotation.Nullable; +import android.support.annotation.VisibleForTesting; import com.bumptech.glide.Glide; import com.bumptech.glide.RequestBuilder; import com.bumptech.glide.RequestManager; @@ -44,6 +46,8 @@ class GifFrameLoader { private Bitmap firstFrame; private Transformation transformation; private DelayTarget pendingTarget; + @Nullable + private GifFrameLoader.OnEveryFrameListener onEveryFrameListener; public interface FrameCallback { void onFrameReady(); @@ -237,8 +241,16 @@ class GifFrameLoader { } } - // Visible for testing. + @VisibleForTesting + void setOnEveryFrameReadyListener(@Nullable OnEveryFrameListener onEveryFrameListener) { + this.onEveryFrameListener = onEveryFrameListener; + } + + @VisibleForTesting void onFrameReady(DelayTarget delayTarget) { + if (onEveryFrameListener != null) { + onEveryFrameListener.onFrameReady(); + } isLoadPending = false; if (isCleared) { handler.obtainMessage(FrameLoaderCallback.MSG_CLEAR, delayTarget).sendToTarget(); @@ -333,4 +345,9 @@ class GifFrameLoader { // See #1510. return new ObjectKey(Math.random()); } + + @VisibleForTesting + interface OnEveryFrameListener { + void onFrameReady(); + } } diff --git a/library/src/main/java/com/bumptech/glide/request/target/ImageViewTarget.java b/library/src/main/java/com/bumptech/glide/request/target/ImageViewTarget.java index 68a37e401c077e7373b1aea622be99fc244cef07..30e06f7af590505c79a31629fde8cff121ec269f 100644 --- a/library/src/main/java/com/bumptech/glide/request/target/ImageViewTarget.java +++ b/library/src/main/java/com/bumptech/glide/request/target/ImageViewTarget.java @@ -111,8 +111,10 @@ public abstract class ImageViewTarget extends ViewTarget } private void setResourceInternal(@Nullable Z resource) { - maybeUpdateAnimatable(resource); + // Order matters here. Set the resource first to make sure that the Drawable has a valid and + // non-null Callback before starting it. setResource(resource); + maybeUpdateAnimatable(resource); } private void maybeUpdateAnimatable(@Nullable Z resource) { diff --git a/library/src/test/java/com/bumptech/glide/request/target/ImageViewTargetTest.java b/library/src/test/java/com/bumptech/glide/request/target/ImageViewTargetTest.java index eeb0b1fcf688a2274b017a8de52bd0b255d7b77e..b8250876f847cddb462f86cc200da11625f29cc4 100644 --- a/library/src/test/java/com/bumptech/glide/request/target/ImageViewTargetTest.java +++ b/library/src/test/java/com/bumptech/glide/request/target/ImageViewTargetTest.java @@ -4,11 +4,13 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import android.graphics.Color; +import android.graphics.drawable.Animatable; import android.graphics.drawable.ColorDrawable; import android.graphics.drawable.Drawable; import android.widget.ImageView; @@ -16,6 +18,7 @@ import com.bumptech.glide.request.transition.Transition; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.InOrder; import org.robolectric.RobolectricTestRunner; import org.robolectric.RuntimeEnvironment; import org.robolectric.annotation.Config; @@ -102,7 +105,23 @@ public class ImageViewTargetTest { verify(animation).transition(eq(placeholder), eq(target)); } - private static class TestTarget extends ImageViewTarget { + @Test + public void onResourceReady_withAnimatableResource_startsAnimatableAfterSetResource() { + AnimatedDrawable drawable = mock(AnimatedDrawable.class); + ImageView view = mock(ImageView.class); + target = new TestTarget(view); + target.onResourceReady(drawable, /*transition=*/ null); + + InOrder order = inOrder(view, drawable); + order.verify(view).setImageDrawable(drawable); + order.verify(drawable).start(); + } + + private abstract static class AnimatedDrawable extends Drawable implements Animatable { + // Intentionally empty. + } + + private static final class TestTarget extends ImageViewTarget { public Drawable resource; public TestTarget(ImageView view) { @@ -112,6 +131,7 @@ public class ImageViewTargetTest { @Override protected void setResource(Drawable resource) { this.resource = resource; + view.setImageDrawable(resource); } } } diff --git a/samples/gallery/src/main/java/com/bumptech/glide/samples/gallery/RecyclerAdapter.java b/samples/gallery/src/main/java/com/bumptech/glide/samples/gallery/RecyclerAdapter.java index 96897779ddebae4dcf9ba5270f935448a62bd7ff..53df5f42b6b633c44cc33556d99d64d93d2d4a03 100644 --- a/samples/gallery/src/main/java/com/bumptech/glide/samples/gallery/RecyclerAdapter.java +++ b/samples/gallery/src/main/java/com/bumptech/glide/samples/gallery/RecyclerAdapter.java @@ -2,7 +2,6 @@ package com.bumptech.glide.samples.gallery; import android.content.Context; import android.graphics.Point; -import android.graphics.drawable.Drawable; import android.support.annotation.NonNull; import android.support.v7.widget.RecyclerView; import android.view.Display; @@ -15,6 +14,7 @@ import android.widget.ImageView; import com.bumptech.glide.ListPreloader; import com.bumptech.glide.RequestBuilder; import com.bumptech.glide.load.Key; +import com.bumptech.glide.load.resource.gif.GifDrawable; import com.bumptech.glide.signature.MediaStoreSignature; import java.util.Collections; import java.util.List; @@ -28,13 +28,13 @@ class RecyclerAdapter extends RecyclerView.Adapter data; private final int screenWidth; - private final GlideRequest requestBuilder; + private final GlideRequest requestBuilder; private int[] actualDimensions; RecyclerAdapter(Context context, List data, GlideRequests glideRequests) { this.data = data; - requestBuilder = glideRequests.asDrawable().fitCenter(); + requestBuilder = glideRequests.asGif().fitCenter(); setHasStableIds(true); @@ -100,7 +100,7 @@ class RecyclerAdapter extends RecyclerView.Adapter getPreloadRequestBuilder(MediaStoreData item) { + public RequestBuilder getPreloadRequestBuilder(MediaStoreData item) { MediaStoreSignature signature = new MediaStoreSignature(item.mimeType, item.dateModified, item.orientation); return requestBuilder