From 1bd66cb3ffb1a4f684bd9cee6209df28fc860147 Mon Sep 17 00:00:00 2001
From: Sam Judd <judds@google.com>
Date: Mon, 24 Nov 2014 07:31:49 -0800
Subject: [PATCH] Always return new copy in DrawableResource.

Fixes #276
---
 .../resource/drawable/DrawableResourceTest.java |  9 +++++----
 .../resource/gif/GifDrawableResourceTest.java   | 17 -----------------
 .../resource/drawable/DrawableResource.java     | 12 ++++--------
 3 files changed, 9 insertions(+), 29 deletions(-)

diff --git a/library/src/androidTest/java/com/bumptech/glide/load/resource/drawable/DrawableResourceTest.java b/library/src/androidTest/java/com/bumptech/glide/load/resource/drawable/DrawableResourceTest.java
index 1f0330407..3dbc3a079 100644
--- a/library/src/androidTest/java/com/bumptech/glide/load/resource/drawable/DrawableResourceTest.java
+++ b/library/src/androidTest/java/com/bumptech/glide/load/resource/drawable/DrawableResourceTest.java
@@ -1,6 +1,7 @@
 package com.bumptech.glide.load.resource.drawable;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -38,18 +39,18 @@ public class DrawableResourceTest {
     }
 
     @Test
-    public void testReturnsDrawableOnFirstGet() {
-        assertEquals(drawable, resource.get());
+    public void testDoesNotReturnOriginalDrawableOnGet() {
+        when(drawable.getConstantState()).thenReturn(mock(Drawable.ConstantState.class));
+        assertNotEquals(drawable, resource.get());
     }
 
     @Test
-    public void testReturnsNewDrawableOnSecondGet() {
+    public void testReturnsNewDrawableOnGet() {
         GifDrawable expected = mock(GifDrawable.class);
         Drawable.ConstantState constantState = mock(Drawable.ConstantState.class);
         when(constantState.newDrawable()).thenReturn(expected);
         when(drawable.getConstantState()).thenReturn(constantState);
 
-        assertEquals(drawable, resource.get());
         assertEquals(expected, resource.get());
 
         verify(drawable).getConstantState();
diff --git a/library/src/androidTest/java/com/bumptech/glide/load/resource/gif/GifDrawableResourceTest.java b/library/src/androidTest/java/com/bumptech/glide/load/resource/gif/GifDrawableResourceTest.java
index 831b62283..4d930f267 100644
--- a/library/src/androidTest/java/com/bumptech/glide/load/resource/gif/GifDrawableResourceTest.java
+++ b/library/src/androidTest/java/com/bumptech/glide/load/resource/gif/GifDrawableResourceTest.java
@@ -6,7 +6,6 @@ import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 import android.graphics.Bitmap;
-import android.graphics.drawable.Drawable;
 
 import com.bumptech.glide.util.Util;
 
@@ -29,22 +28,6 @@ public class GifDrawableResourceTest {
         resource = new GifDrawableResource(drawable);
     }
 
-    @Test
-    public void testReturnsDrawableOnFirstGet() {
-        assertEquals(drawable, resource.get());
-    }
-
-    @Test
-    public void testReturnsNewDrawableOnSecondGet() {
-        GifDrawable expected = mock(GifDrawable.class);
-        Drawable.ConstantState constantState = mock(Drawable.ConstantState.class);
-        when(constantState.newDrawable()).thenReturn(expected);
-        when(drawable.getConstantState()).thenReturn(constantState);
-
-        resource.get();
-        assertEquals(expected, resource.get());
-    }
-
     @Test
     public void testReturnsDrawableSizePlusFirstFrameSize() {
         final int size = 2134;
diff --git a/library/src/main/java/com/bumptech/glide/load/resource/drawable/DrawableResource.java b/library/src/main/java/com/bumptech/glide/load/resource/drawable/DrawableResource.java
index 643f7b40e..a0730a45e 100644
--- a/library/src/main/java/com/bumptech/glide/load/resource/drawable/DrawableResource.java
+++ b/library/src/main/java/com/bumptech/glide/load/resource/drawable/DrawableResource.java
@@ -15,7 +15,6 @@ import com.bumptech.glide.load.engine.Resource;
  */
 public abstract class DrawableResource<T extends Drawable> implements Resource<T> {
     protected final T drawable;
-    private boolean returnedOriginalDrawable;
 
     public DrawableResource(T drawable) {
         if (drawable == null) {
@@ -25,14 +24,11 @@ public abstract class DrawableResource<T extends Drawable> implements Resource<T
     }
 
     @SuppressWarnings("unchecked")
-    // drawables should always return a copy of the same class
     @Override
     public final T get() {
-        if (!returnedOriginalDrawable) {
-            returnedOriginalDrawable = true;
-            return drawable;
-        } else {
-            return (T) drawable.getConstantState().newDrawable();
-        }
+        // Drawables contain temporary state related to how they're being displayed (alpha, color filter etc), so
+        // return a new copy each time. If we ever return the original drawable, it's temporary state may be changed
+        // and subsequent copies may end up with that temporary state. See #276.
+        return (T) drawable.getConstantState().newDrawable();
     }
 }
-- 
GitLab