Fix shared gif drawables

library/src/main/java/com/bumptech/glide/Glide.java

@@ -38,7 +38,8 @@ import com.bumptech.glide.load.resource.gif.GifDataLoadProvider;
 import com.bumptech.glide.load.resource.gifbitmap.GifBitmapWrapper;
 import com.bumptech.glide.load.resource.gifbitmap.ImageVideoGifDataLoadProvider;
 import com.bumptech.glide.load.resource.transcode.BitmapDrawableTranscoder;
-import com.bumptech.glide.load.resource.transcode.GifBitmapDrawableTranscoder;
+import com.bumptech.glide.load.resource.transcode.GifBitmapWrapperDrawableTranscoder;
+import com.bumptech.glide.load.resource.transcode.GifDataDrawableTranscoder;
 import com.bumptech.glide.load.resource.transcode.ResourceTranscoder;
 import com.bumptech.glide.load.resource.transcode.TranscoderFactory;
 import com.bumptech.glide.provider.DataLoadProviderFactory;
@@ -188,7 +189,8 @@ public class Glide {
         transcoderFactory.register(Bitmap.class, BitmapDrawable.class,
                 new BitmapDrawableTranscoder(context.getResources(), bitmapPool));
         transcoderFactory.register(GifBitmapWrapper.class, Drawable.class,
-                new GifBitmapDrawableTranscoder(context));
+                new GifBitmapWrapperDrawableTranscoder(new BitmapDrawableTranscoder(context.getResources(), bitmapPool),
+                        new GifDataDrawableTranscoder()));
     }
 
     public BitmapPool getBitmapPool() {

library/src/main/java/com/bumptech/glide/load/resource/drawable/BitmapDrawableResource.java → library/src/main/java/com/bumptech/glide/load/resource/bitmap/BitmapDrawableResource.java

-package com.bumptech.glide.load.resource.drawable;
+package com.bumptech.glide.load.resource.bitmap;
 
 import android.graphics.drawable.BitmapDrawable;
 import com.bumptech.glide.Resource;

library/src/main/java/com/bumptech/glide/load/resource/drawable/DrawableResource.java

-package com.bumptech.glide.load.resource.drawable;
-
-import android.graphics.drawable.Drawable;
-import com.bumptech.glide.Resource;
-
-public class DrawableResource extends Resource<Drawable> {
-    private final Drawable drawable;
-    private final Resource wrapped;
-
-    public DrawableResource(Drawable drawable, Resource wrapped) {
-        this.drawable = drawable;
-        this.wrapped = wrapped;
-    }
-
-    @Override
-    public Drawable get() {
-        return drawable;
-    }
-
-    @Override
-    public int getSize() {
-        return wrapped.getSize();
-    }
-
-    @Override
-    protected void recycleInternal() {
-        wrapped.recycle();
-    }
-}

library/src/main/java/com/bumptech/glide/load/resource/transcode/BitmapDrawableTranscoder.java

@@ -4,7 +4,7 @@ import android.content.res.Resources;
 import android.graphics.Bitmap;
 import android.graphics.drawable.BitmapDrawable;
 import com.bumptech.glide.Resource;
-import com.bumptech.glide.load.resource.drawable.BitmapDrawableResource;
+import com.bumptech.glide.load.resource.bitmap.BitmapDrawableResource;
 import com.bumptech.glide.load.engine.bitmap_recycle.BitmapPool;
 
 public class BitmapDrawableTranscoder implements ResourceTranscoder<Bitmap, BitmapDrawable> {

library/src/main/java/com/bumptech/glide/load/resource/transcode/GifBitmapDrawableTranscoder.java → library/src/main/java/com/bumptech/glide/load/resource/transcode/GifBitmapWrapperDrawableTranscoder.java

 package com.bumptech.glide.load.resource.transcode;
 
-import android.content.Context;
-import android.content.res.Resources;
 import android.graphics.Bitmap;
-import android.graphics.drawable.BitmapDrawable;
 import android.graphics.drawable.Drawable;
 import com.bumptech.glide.Resource;
-import com.bumptech.glide.load.resource.drawable.DrawableResource;
+import com.bumptech.glide.load.resource.gif.GifData;
 import com.bumptech.glide.load.resource.gifbitmap.GifBitmapWrapper;
 
-public class GifBitmapDrawableTranscoder implements ResourceTranscoder<GifBitmapWrapper, Drawable> {
-    private final Resources resources;
+public class GifBitmapWrapperDrawableTranscoder implements ResourceTranscoder<GifBitmapWrapper, Drawable> {
+    private final ResourceTranscoder<Bitmap, ? extends Drawable> bitmapDrawableResourceTranscoder;
+    private final ResourceTranscoder<GifData, ? extends Drawable> gifDrawableResourceTranscoder;
 
-    public GifBitmapDrawableTranscoder(Context context) {
-        resources = context.getResources();
+    public GifBitmapWrapperDrawableTranscoder(ResourceTranscoder<Bitmap, ? extends Drawable> bitmapDrawableResourceTranscoder,
+            ResourceTranscoder<GifData, ? extends Drawable> gifDrawableResourceTranscoder) {
+        this.bitmapDrawableResourceTranscoder = bitmapDrawableResourceTranscoder;
+        this.gifDrawableResourceTranscoder = gifDrawableResourceTranscoder;
     }
 
+    @SuppressWarnings("unchecked")
     @Override
     public Resource<Drawable> transcode(Resource<GifBitmapWrapper> toTranscode) {
         GifBitmapWrapper gifBitmap = toTranscode.get();
         Resource<Bitmap> bitmapResource = gifBitmap.getBitmapResource();
 
-        final Resource resource;
-        final Drawable drawable;
+        final Resource<? extends Drawable> result;
         if (bitmapResource != null) {
-            resource = bitmapResource;
-            drawable = new BitmapDrawable(resources, bitmapResource.get());
+            result = bitmapDrawableResourceTranscoder.transcode(bitmapResource);
         } else {
-            resource = gifBitmap.getGifResource();
-            drawable = gifBitmap.getGifResource().get().getDrawable();
+            result = gifDrawableResourceTranscoder.transcode(gifBitmap.getGifResource());
         }
-        return new DrawableResource(drawable, resource);
+        // This is unchecked but always safe, anything that extends a Drawable can be safely cast to a Drawable.
+        return (Resource<Drawable>) result;
     }
 
     @Override
     public String getId() {
-        return "GifBitmapDrawableTranscoder.com.bumptech.glide.load.resource.transcode";
+        return "GifBitmapWrapperDrawableTranscoder.com.bumptech.glide.load.resource.transcode";
     }
 }

library/src/test/java/com/bumptech/glide/load/resource/drawable/BitmapDrawableResourceTest.java → library/src/test/java/com/bumptech/glide/load/resource/bitmap/BitmapDrawableResourceTest.java

-package com.bumptech.glide.load.resource.drawable;
+package com.bumptech.glide.load.resource.bitmap;
 
 import android.graphics.Bitmap;
 import android.graphics.drawable.BitmapDrawable;

library/src/test/java/com/bumptech/glide/load/resource/drawable/DrawableResourceTest.java

-package com.bumptech.glide.load.resource.drawable;
-
-import android.graphics.Color;
-import android.graphics.drawable.ColorDrawable;
-import com.bumptech.glide.Resource;
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.robolectric.RobolectricTestRunner;
-
-import static junit.framework.Assert.assertEquals;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
-
-@RunWith(RobolectricTestRunner.class)
-public class DrawableResourceTest {
-    private ColorDrawable drawable;
-    private DrawableResource resource;
-    private Resource wrapped;
-
-    @Before
-    public void setUp() {
-        drawable = new ColorDrawable(Color.RED);
-        wrapped = mock(Resource.class);
-        resource = new DrawableResource(drawable, wrapped);
-    }
-
-    @Test
-    public void testReturnsGivenSize() {
-        final int size = 100;
-        when(wrapped.getSize()).thenReturn(size);
-        assertEquals(size, resource.getSize());
-    }
-
-    @Test
-    public void testGetReturnsGivenDrawable() {
-        assertEquals(drawable, resource.get());
-    }
-
-    @Test
-    public void testRecyclesWrappedWhenRecycled() {
-        resource.recycleInternal();
-
-        verify(wrapped).recycle();
-    }
-}

library/src/test/java/com/bumptech/glide/load/resource/transcode/GifBitmapDrawableTranscoderTest.java → library/src/test/java/com/bumptech/glide/load/resource/transcode/GifBitmapWrapperDrawableTranscoderTest.java

 package com.bumptech.glide.load.resource.transcode;
 
 import android.graphics.Bitmap;
-import android.graphics.drawable.BitmapDrawable;
 import android.graphics.drawable.Drawable;
 import com.bumptech.glide.Resource;
 import com.bumptech.glide.load.resource.gif.GifData;
-import com.bumptech.glide.load.resource.gif.GifDrawable;
 import com.bumptech.glide.load.resource.gifbitmap.GifBitmapWrapper;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.robolectric.Robolectric;
 import org.robolectric.RobolectricTestRunner;
 
 import static junit.framework.Assert.assertEquals;
 import static junit.framework.Assert.assertNotNull;
+import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 @RunWith(RobolectricTestRunner.class)
-public class GifBitmapDrawableTranscoderTest {
-    private GifBitmapDrawableTranscoder transcoder;
+public class GifBitmapWrapperDrawableTranscoderTest {
+    private GifBitmapWrapperDrawableTranscoder transcoder;
+    private ResourceTranscoder<Bitmap, Drawable> bitmapTranscoder;
+    private ResourceTranscoder<GifData, Drawable> gifDataTranscoder;
 
     @Before
     public void setUp() {
-        transcoder = new GifBitmapDrawableTranscoder(Robolectric.application);
+        bitmapTranscoder = mock(ResourceTranscoder.class);
+        gifDataTranscoder = mock(ResourceTranscoder.class);
+        transcoder = new GifBitmapWrapperDrawableTranscoder(bitmapTranscoder, gifDataTranscoder);
     }
 
     @Test
-    public void testReturnsBitmapDrawableIfGifBitmapHasBitmap() {
+    public void testReturnsDrawableFromBitmapTranscoderIfGifBitmapHasBitmap() {
         GifBitmapWithBitmapHarness harness = new GifBitmapWithBitmapHarness();
+        when(bitmapTranscoder.transcode(eq(harness.bitmapResource))).thenReturn(harness.expected);
 
-        BitmapDrawable transcoded = (BitmapDrawable) transcoder.transcode(harness.gifBitmapResource).get();
-
-        assertEquals(harness.expected, transcoded.getBitmap());
-    }
-
-    @Test
-    public void testReturnedResourceHasBitmapSizeIfGifBitmapHasBitmap() {
-        final int size = 100;
-        GifBitmapWithBitmapHarness harness = new GifBitmapWithBitmapHarness();
-        when(harness.bitmapResource.getSize()).thenReturn(size);
-
-        Resource<Drawable> transcoded = transcoder.transcode(harness.gifBitmapResource);
-
-        assertEquals(size, transcoded.getSize());
+        assertEquals(harness.expected, transcoder.transcode(harness.gifBitmapResource));
     }
 
     @Test
-    public void testReturnsGifDrawableIfGifBitmapHasGif() {
+    public void testReturnsDrawableFromGifTranscoderIfGifBitmapHasGif() {
         GifBitmapWithGifHarness harness = new GifBitmapWithGifHarness();
+        when(gifDataTranscoder.transcode(eq(harness.gifResource))).thenReturn(harness.expected);
 
-        Drawable transcoded = transcoder.transcode(harness.gifBitmapResource).get();
-
-        assertEquals(harness.expected, transcoded);
-    }
-
-    @Test
-    public void testReturnedResourceHasGifDrawableSizeIfGifBitmapHasGif() {
-        final int size = 200;
-        GifBitmapWithGifHarness harness = new GifBitmapWithGifHarness();
-        when(harness.gifResource.getSize()).thenReturn(size);
-
-        Resource<Drawable> transcoded = transcoder.transcode(harness.gifBitmapResource);
-
-        assertEquals(size, transcoded.getSize());
+        assertEquals(harness.expected, transcoder.transcode(harness.gifBitmapResource));
     }
 
     @Test
@@ -75,6 +53,7 @@ public class GifBitmapDrawableTranscoderTest {
     private static class TranscoderHarness {
         Resource<GifBitmapWrapper> gifBitmapResource = mock(Resource.class);
         GifBitmapWrapper gifBitmap = mock(GifBitmapWrapper.class);
+        Resource<Drawable> expected = mock(Resource.class);
 
         public TranscoderHarness() {
             when(gifBitmapResource.get()).thenReturn(gifBitmap);
@@ -82,24 +61,20 @@ public class GifBitmapDrawableTranscoderTest {
     }
 
     private static class GifBitmapWithBitmapHarness extends TranscoderHarness {
-        Bitmap expected = Bitmap.createBitmap(100, 200, Bitmap.Config.ARGB_8888);
         Resource<Bitmap> bitmapResource = mock(Resource.class);
 
         public GifBitmapWithBitmapHarness() {
             super();
-            when(bitmapResource.get()).thenReturn(expected);
             when(gifBitmap.getBitmapResource()).thenReturn(bitmapResource);
         }
     }
 
     private static class GifBitmapWithGifHarness extends TranscoderHarness {
-        GifDrawable expected = mock(GifDrawable.class);
         GifData gifData = mock(GifData.class);
         Resource<GifData> gifResource = mock(Resource.class);
 
         public GifBitmapWithGifHarness() {
             super();
-            when(gifData.getDrawable()).thenReturn(expected);
             when(gifResource.get()).thenReturn(gifData);
             when(gifBitmap.getGifResource()).thenReturn(gifResource);
         }