Fix Gifs not appearing.

7 files changed, 266 insertions, 13 deletions

diff --git a/library/src/main/java/com/bumptech/glide/GenericRequestBuilder.java b/library/src/main/java/com/bumptech/glide/GenericRequestBuilder.java
index 30e3051b..27fd7cae 100644
--- a/library/src/main/java/com/bumptech/glide/GenericRequestBuilder.java
+++ b/library/src/main/java/com/bumptech/glide/GenericRequestBuilder.java
@@ -521,6 +521,10 @@ public class GenericRequestBuilder<ModelType, DataType, ResourceType, TranscodeT
      * @return The given target.
      */
     public <Y extends Target<TranscodeType>> Y into(Y target) {
+        if (target == null) {
+            throw new IllegalArgumentException("You must pass in a non null Target");
+        }
+
         Request previous = target.getRequest();
 
         if (previous != null) {
@@ -547,6 +551,9 @@ public class GenericRequestBuilder<ModelType, DataType, ResourceType, TranscodeT
      * @return The {@link BitmapImageViewTarget} used to wrap the given {@link ImageView}.
      */
     public Target<TranscodeType> into(ImageView view) {
+        if (view == null) {
+            throw new IllegalArgumentException("You must pass in a non null View");
+        }
         return into(glide.buildImageViewTarget(view, transcodeClass));
     }
 
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 281c1ad7..254f30e6 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
@@ -13,6 +13,7 @@ import com.bumptech.glide.load.SkipCache;
 import com.bumptech.glide.load.Transformation;
 import com.bumptech.glide.load.engine.bitmap_recycle.BitmapPool;
 import com.bumptech.glide.load.engine.cache.MemorySizeCalculator;
+import com.bumptech.glide.load.model.NullEncoder;
 import com.bumptech.glide.load.resource.NullDecoder;
 import com.bumptech.glide.load.resource.bitmap.BitmapEncoder;
 import com.bumptech.glide.load.resource.bitmap.StreamBitmapDecoder;
@@ -32,7 +33,7 @@ class GifFrameManager {
     private final Handler mainHandler;
     private final ResourceEncoder<Bitmap> encoder;
     private final Context context;
-
+    private final NullEncoder<GifDecoder> sourceEncoder;
     private Transformation<Bitmap> transformation;
     private final int targetWidth;
     private final int targetHeight;
@@ -61,6 +62,7 @@ class GifFrameManager {
         calculator = new MemorySizeCalculator(context);
         frameLoader = new GifFrameModelLoader();
         frameResourceDecoder = new GifFrameResourceDecoder(bitmapPool);
+        sourceEncoder = NullEncoder.get();
 
         if (!decoder.isTransparent()) {
             // For non transparent gifs, we can beat the performance of our gif decoder for each frame by decoding jpegs
@@ -100,10 +102,11 @@ class GifFrameManager {
                 .using(frameLoader, GifDecoder.class)
                 .load(decoder)
                 .as(Bitmap.class)
+                .sourceEncoder(sourceEncoder)
                 .decoder(frameResourceDecoder)
                 .cacheDecoder(cacheDecoder)
-                .transform(transformation)
                 .encoder(encoder)
+                .transform(transformation)
                 .skipMemoryCache(skipCache)
                 .into(next);
     }
diff --git a/library/src/main/java/com/bumptech/glide/request/target/DrawableImageViewTarget.java b/library/src/main/java/com/bumptech/glide/request/target/DrawableImageViewTarget.java
index 8fbd0e48..9c298264 100644
--- a/library/src/main/java/com/bumptech/glide/request/target/DrawableImageViewTarget.java
+++ b/library/src/main/java/com/bumptech/glide/request/target/DrawableImageViewTarget.java
@@ -1,5 +1,6 @@
 package com.bumptech.glide.request.target;
 
+import android.graphics.drawable.Animatable;
 import android.graphics.drawable.Drawable;
 import android.widget.ImageView;
 import com.bumptech.glide.request.GlideAnimation;
@@ -15,17 +16,19 @@ public class DrawableImageViewTarget extends ViewTarget<ImageView, Drawable> {
 
     @Override
     public void onResourceReady(Drawable resource, GlideAnimation<Drawable> animation) {
-
-        //TODO: Try to generalize this to other sizes/shapes.
-        // This is a dirty hack that tries to make loading square thumbnails and then square full images less costly by
-        // forcing both the smaller thumb and the larger version to have exactly the same intrinsic dimensions. If a
-        // drawable is replaced in an ImageView by another drawable with different intrinsic dimensions, the ImageView
-        // requests a layout. Scrolling rapidly while replacing thumbs with larger images triggers lots of these calls
-        // and causes significant amounts of jank.
-        float viewRatio = view.getWidth() / (float) view.getHeight();
-        float drawableRatio = resource.getIntrinsicWidth() / (float) resource.getIntrinsicHeight();
-        if (Math.abs(viewRatio - 1f) <= SQUARE_RATIO_MARGIN && Math.abs(drawableRatio - 1f) <= SQUARE_RATIO_MARGIN) {
-            resource = new SquaringDrawable(resource, view.getWidth());
+        if (!(resource instanceof Animatable)) {
+            //TODO: Try to generalize this to other sizes/shapes.
+            // This is a dirty hack that tries to make loading square thumbnails and then square full images less costly by
+            // forcing both the smaller thumb and the larger version to have exactly the same intrinsic dimensions. If a
+            // drawable is replaced in an ImageView by another drawable with different intrinsic dimensions, the ImageView
+            // requests a layout. Scrolling rapidly while replacing thumbs with larger images triggers lots of these calls
+            // and causes significant amounts of jank.
+            float viewRatio = view.getWidth() / (float) view.getHeight();
+            float drawableRatio = resource.getIntrinsicWidth() / (float) resource.getIntrinsicHeight();
+            if (Math.abs(viewRatio - 1f) <= SQUARE_RATIO_MARGIN &&
+                    Math.abs(drawableRatio - 1f) <= SQUARE_RATIO_MARGIN) {
+                resource = new SquaringDrawable(resource, view.getWidth());
+            }
         }
 
         if (animation == null || !animation.animate(view.getDrawable(), resource, view, this)) {
diff --git a/library/src/main/java/com/bumptech/glide/request/target/ViewTarget.java b/library/src/main/java/com/bumptech/glide/request/target/ViewTarget.java
index 5f77d534..e4179aff 100644
--- a/library/src/main/java/com/bumptech/glide/request/target/ViewTarget.java
+++ b/library/src/main/java/com/bumptech/glide/request/target/ViewTarget.java
@@ -40,6 +40,9 @@ public abstract class ViewTarget<T extends View, Z> implements Target<Z> {
     private final SizeDeterminer sizeDeterminer;
 
     public ViewTarget(T view) {
+        if (view == null) {
+            throw new NullPointerException("View must not be null!");
+        }
         this.view = view;
         sizeDeterminer = new SizeDeterminer(view);
     }
diff --git a/library/src/test/java/com/bumptech/glide/GenericRequestBuilderTest.java b/library/src/test/java/com/bumptech/glide/GenericRequestBuilderTest.java
index baaf1fc5..b3c96924 100644
--- a/library/src/test/java/com/bumptech/glide/GenericRequestBuilderTest.java
+++ b/library/src/test/java/com/bumptech/glide/GenericRequestBuilderTest.java
@@ -1,5 +1,6 @@
 package com.bumptech.glide;
 
+import android.widget.ImageView;
 import com.bumptech.glide.load.ResourceDecoder;
 import com.bumptech.glide.load.ResourceEncoder;
 import com.bumptech.glide.manager.RequestTracker;
@@ -114,6 +115,16 @@ public class GenericRequestBuilderTest {
         verify(requestTracker).removeRequest(eq(previous));
     }
 
+    @Test(expected = IllegalArgumentException.class)
+    public void testThrowsIfGivenNullTarget() {
+        getNullModelRequest().into((Target) null);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testThrowsIfGivenNullView() {
+        getNullModelRequest().into((ImageView) null);
+    }
+
     private GenericRequestBuilder getNullModelRequest() {
         return new GenericRequestBuilder(Robolectric.application, null, null, Object.class, mock(Glide.class),
                 requestTracker);
diff --git a/library/src/test/java/com/bumptech/glide/request/target/DrawableImageViewTargetTest.java b/library/src/test/java/com/bumptech/glide/request/target/DrawableImageViewTargetTest.java
new file mode 100644
index 00000000..6caa833f
--- /dev/null
+++ b/library/src/test/java/com/bumptech/glide/request/target/DrawableImageViewTargetTest.java
@@ -0,0 +1,212 @@
+package com.bumptech.glide.request.target;
+
+import android.graphics.Color;
+import android.graphics.drawable.AnimationDrawable;
+import android.graphics.drawable.BitmapDrawable;
+import android.graphics.drawable.ColorDrawable;
+import android.graphics.drawable.Drawable;
+import android.view.View;
+import android.widget.ImageView;
+import com.bumptech.glide.request.GlideAnimation;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
+import org.robolectric.Robolectric;
+import org.robolectric.RobolectricTestRunner;
+
+import static junit.framework.Assert.assertEquals;
+import static junit.framework.Assert.assertFalse;
+import static junit.framework.Assert.assertNull;
+import static junit.framework.Assert.assertTrue;
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Matchers.isNull;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+@RunWith(RobolectricTestRunner.class)
+public class DrawableImageViewTargetTest {
+    private ImageView view;
+    private DrawableImageViewTarget target;
+
+    @Before
+    public void setUp() {
+        view = new ImageView(Robolectric.application);
+        target = new DrawableImageViewTarget(view);
+    }
+
+    @Test
+    public void testSetsDrawableOnViewInOnResourceReadyWhenAnimationIsNull() {
+        Drawable resource = new ColorDrawable(Color.BLUE);
+        target.onResourceReady(resource, null);
+
+        assertEquals(resource, view.getDrawable());
+    }
+
+    @Test
+    public void testSetsDrawableOnViewInOnResourceReadyWhenAnimationReturnsFalse() {
+        GlideAnimation animation = mock(GlideAnimation.class);
+        when(animation.animate(any(Drawable.class), any(Drawable.class), any(View.class), any(Target.class)))
+                .thenReturn(false);
+        Drawable resource = new ColorDrawable(Color.GRAY);
+        target.onResourceReady(resource, animation);
+
+        assertEquals(resource, view.getDrawable());
+    }
+
+    @Test
+    public void testDoesNotSetDrawableOnViewInOnResourceReadyWhenAnimationReturnsTrue() {
+        Drawable resource = new ColorDrawable(Color.RED);
+        GlideAnimation animation = mock(GlideAnimation.class);
+        when(animation.animate((Drawable) isNull(), eq(resource), eq(view), eq(target))).thenReturn(true);
+        target.onResourceReady(resource, animation);
+
+        assertNull(view.getDrawable());
+    }
+
+    @Test
+    public void testSetsPlaceholderOnView() {
+        Drawable placeholder = new ColorDrawable(Color.RED);
+        target.setPlaceholder(placeholder);
+
+        assertEquals(placeholder, view.getDrawable());
+    }
+
+    @Test
+    public void testProvidesCurrentPlaceholderToAnimationIfPresent() {
+        Drawable placeholder = new ColorDrawable(Color.BLACK);
+        view.setImageDrawable(placeholder);
+
+        GlideAnimation animation = mock(GlideAnimation.class);
+
+        target.onResourceReady(new ColorDrawable(Color.GREEN), animation);
+
+        verify(animation).animate(eq(placeholder), any(Drawable.class), any(View.class), any(Target.class));
+    }
+
+    @Test
+    public void testWrapsDrawableInSquaringDrawableIfDrawableAndViewAreSquare() {
+        ImageView mockView = mock(ImageView.class);
+        when(mockView.getWidth()).thenReturn(100);
+        when(mockView.getHeight()).thenReturn(100);
+        DrawableImageViewTarget target = new DrawableImageViewTarget(mockView);
+        Drawable drawable = new ColorDrawable(Color.RED) {
+            @Override
+            public int getIntrinsicHeight() {
+                return 100;
+            }
+
+            @Override
+            public int getIntrinsicWidth() {
+                return 100;
+            }
+        };
+
+
+        target.onResourceReady(drawable, null);
+
+        doAnswer(new Answer() {
+            @Override
+            public Object answer(InvocationOnMock invocation) throws Throwable {
+                Drawable drawable = (Drawable) invocation.getArguments()[0];
+                assertTrue(drawable instanceof SquaringDrawable);
+                return null;
+            }
+        }).when(mockView).setImageDrawable(any(Drawable.class));
+        verify(mockView).setImageDrawable(any(BitmapDrawable.class));
+    }
+
+    @Test
+    public void testDoesNotWrapInSquaringDrawableIfDrawableIsAnimated() {
+        ImageView mockView = mock(ImageView.class);
+        when(mockView.getWidth()).thenReturn(100);
+        when(mockView.getHeight()).thenReturn(100);
+        DrawableImageViewTarget target = new DrawableImageViewTarget(mockView);
+        Drawable drawable = new AnimationDrawable() {
+            @Override
+            public int getIntrinsicHeight() {
+                return 100;
+            }
+
+            @Override
+            public int getIntrinsicWidth() {
+                return 100;
+            }
+        };
+         target.onResourceReady(drawable, null);
+
+        doAnswer(new Answer() {
+            @Override
+            public Object answer(InvocationOnMock invocation) throws Throwable {
+                Drawable drawable = (Drawable) invocation.getArguments()[0];
+                assertFalse(drawable instanceof SquaringDrawable);
+                return null;
+            }
+        }).when(mockView).setImageDrawable(any(Drawable.class));
+        verify(mockView).setImageDrawable(any(BitmapDrawable.class));
+    }
+
+    @Test
+    public void testDoesNotWrapInSquaringDrawableIfDrawableIsNotSquare() {
+        ImageView mockView = mock(ImageView.class);
+        when(mockView.getWidth()).thenReturn(100);
+        when(mockView.getHeight()).thenReturn(100);
+        DrawableImageViewTarget target = new DrawableImageViewTarget(mockView);
+        Drawable drawable = new ColorDrawable(Color.RED) {
+            @Override
+            public int getIntrinsicHeight() {
+                return 100;
+            }
+
+            @Override
+            public int getIntrinsicWidth() {
+                return 150;
+            }
+        };
+         target.onResourceReady(drawable, null);
+
+        doAnswer(new Answer() {
+            @Override
+            public Object answer(InvocationOnMock invocation) throws Throwable {
+                Drawable drawable = (Drawable) invocation.getArguments()[0];
+                assertFalse(drawable instanceof SquaringDrawable);
+                return null;
+            }
+        }).when(mockView).setImageDrawable(any(Drawable.class));
+        verify(mockView).setImageDrawable(any(BitmapDrawable.class));
+    }
+
+    @Test
+    public void testDoesNotWrapInSquaringDrawableIfViewNotSquare() {
+        ImageView mockView = mock(ImageView.class);
+        when(mockView.getWidth()).thenReturn(100);
+        when(mockView.getHeight()).thenReturn(150);
+        DrawableImageViewTarget target = new DrawableImageViewTarget(mockView);
+        Drawable drawable = new ColorDrawable(Color.RED) {
+            @Override
+            public int getIntrinsicHeight() {
+                return 100;
+            }
+
+            @Override
+            public int getIntrinsicWidth() {
+                return 100;
+            }
+        };
+         target.onResourceReady(drawable, null);
+
+        doAnswer(new Answer() {
+            @Override
+            public Object answer(InvocationOnMock invocation) throws Throwable {
+                Drawable drawable = (Drawable) invocation.getArguments()[0];
+                assertFalse(drawable instanceof SquaringDrawable);
+                return null;
+            }
+        }).when(mockView).setImageDrawable(any(Drawable.class));
+        verify(mockView).setImageDrawable(any(BitmapDrawable.class));
+    }
+}
\ No newline at end of file
diff --git a/library/src/test/java/com/bumptech/glide/request/target/ViewTargetTest.java b/library/src/test/java/com/bumptech/glide/request/target/ViewTargetTest.java
index 66832dc4..ecc0deda 100644
--- a/library/src/test/java/com/bumptech/glide/request/target/ViewTargetTest.java
+++ b/library/src/test/java/com/bumptech/glide/request/target/ViewTargetTest.java
@@ -242,6 +242,20 @@ public class ViewTargetTest {
         verify(cb).onSizeReady(eq(width), eq(height));
     }
 
+    @Test(expected = NullPointerException.class)
+    public void testThrowsIfGivenNullView() {
+        ViewTarget viewTarget = new ViewTarget(null) {
+            @Override
+            public void onResourceReady(Object resource, GlideAnimation glideAnimation) {
+
+            }
+
+            @Override
+            public void setPlaceholder(Drawable placeholder) {
+
+            }
+        };
+    }
 
     @Implements(ViewTreeObserver.class)
     public static class PreDrawShadowViewTreeObserver extends ShadowViewTreeObserver {