Avoid recycling Bitmaps/Drawables passed to load().

This is still best effort in that anyone can register a component that
doesn’t follow our conventions and break things, but it should be safer
by default at least.

instrumentation/src/androidTest/java/com/bumptech/glide/LoadBitmapTest.java

@@ -0,0 +1,141 @@
+package com.bumptech.glide;
+
+
+import static com.google.common.truth.Truth.assertThat;
+
+import android.content.Context;
+import android.graphics.Bitmap;
+import android.graphics.BitmapFactory;
+import android.graphics.drawable.Drawable;
+import android.support.test.InstrumentationRegistry;
+import android.support.test.runner.AndroidJUnit4;
+import com.bumptech.glide.load.engine.bitmap_recycle.BitmapPoolAdapter;
+import com.bumptech.glide.load.engine.cache.MemoryCacheAdapter;
+import com.bumptech.glide.request.FutureTarget;
+import com.bumptech.glide.test.ConcurrencyHelper;
+import com.bumptech.glide.test.GlideApp;
+import com.bumptech.glide.test.ResourceIds;
+import com.bumptech.glide.test.TearDownGlide;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+@RunWith(AndroidJUnit4.class)
+public class LoadBitmapTest {
+  @Rule public final TearDownGlide tearDownGlide = new TearDownGlide();
+  private final ConcurrencyHelper concurrency = new ConcurrencyHelper();
+  private Context context;
+
+  @Before
+  public void setUp() {
+    context = InstrumentationRegistry.getTargetContext();
+  }
+
+  @Test
+  public void clearFromRequestBuilder_asDrawable_withLoadedBitmap_doesNotRecycleBitmap() {
+    Glide.init(context, new GlideBuilder()
+        .setMemoryCache(new MemoryCacheAdapter())
+        .setBitmapPool(new BitmapPoolAdapter()));
+    Bitmap bitmap = BitmapFactory.decodeResource(context.getResources(), ResourceIds.raw.canonical);
+    FutureTarget<Drawable> target =
+        concurrency.wait(
+            GlideApp.with(context)
+                .asDrawable()
+                .load(bitmap)
+                .submit(100, 100));
+    Glide.with(context).clear(target);
+
+    // Allow Glide's resource recycler to run on the main thread.
+    concurrency.pokeMainThread();
+
+    assertThat(bitmap.isRecycled()).isFalse();
+  }
+
+  @Test
+  public void transformFromRequestBuilder_asDrawable_withLoadedBitmap_doesNotRecycleBitmap() {
+    Glide.init(context, new GlideBuilder()
+        .setMemoryCache(new MemoryCacheAdapter())
+        .setBitmapPool(new BitmapPoolAdapter()));
+    Bitmap bitmap = BitmapFactory.decodeResource(context.getResources(), ResourceIds.raw.canonical);
+    concurrency.wait(
+        GlideApp.with(context)
+            .asDrawable()
+            .load(bitmap)
+            .centerCrop()
+            .submit(100, 100));
+
+    assertThat(bitmap.isRecycled()).isFalse();
+  }
+
+  @Test
+  public void clearFromRequestManager_withLoadedBitmap_doesNotRecycleBitmap() {
+    Glide.init(context, new GlideBuilder()
+        .setMemoryCache(new MemoryCacheAdapter())
+        .setBitmapPool(new BitmapPoolAdapter()));
+    Bitmap bitmap = BitmapFactory.decodeResource(context.getResources(), ResourceIds.raw.canonical);
+    FutureTarget<Drawable> target =
+        concurrency.wait(
+            GlideApp.with(context)
+                .load(bitmap)
+                .submit(100, 100));
+    Glide.with(context).clear(target);
+
+    // Allow Glide's resource recycler to run on the main thread.
+    concurrency.pokeMainThread();
+
+    assertThat(bitmap.isRecycled()).isFalse();
+  }
+
+  @Test
+  public void transformFromRequestManager_withLoadedBitmap_doesNotRecycleBitmap() {
+    Glide.init(context, new GlideBuilder()
+        .setMemoryCache(new MemoryCacheAdapter())
+        .setBitmapPool(new BitmapPoolAdapter()));
+    Bitmap bitmap = BitmapFactory.decodeResource(context.getResources(), ResourceIds.raw.canonical);
+    concurrency.wait(
+        GlideApp.with(context)
+            .load(bitmap)
+            .centerCrop()
+            .submit(100, 100));
+
+    assertThat(bitmap.isRecycled()).isFalse();
+  }
+
+  @Test
+  public void clearFromRequestBuilder_withLoadedBitmap_asBitmap_doesNotRecycleBitmap() {
+    Glide.init(context, new GlideBuilder()
+        .setMemoryCache(new MemoryCacheAdapter())
+        .setBitmapPool(new BitmapPoolAdapter()));
+    Bitmap bitmap = BitmapFactory.decodeResource(context.getResources(), ResourceIds.raw.canonical);
+    FutureTarget<Bitmap> target =
+        concurrency.wait(
+            GlideApp.with(context)
+                .asBitmap()
+                .load(bitmap)
+                .submit(100, 100));
+    Glide.with(context).clear(target);
+
+    // Allow Glide's resource recycler to run on the main thread.
+    concurrency.pokeMainThread();
+
+    assertThat(bitmap.isRecycled()).isFalse();
+  }
+
+  @Test
+  public void transformFromRequestBuilder_withLoadedBitmap_asBitmap_doesNotRecycleBitmap() {
+    Glide.init(context, new GlideBuilder()
+        .setMemoryCache(new MemoryCacheAdapter())
+        .setBitmapPool(new BitmapPoolAdapter()));
+    Bitmap bitmap = BitmapFactory.decodeResource(context.getResources(), ResourceIds.raw.canonical);
+    concurrency.wait(
+        GlideApp.with(context)
+            .asBitmap()
+            .load(bitmap)
+            .centerCrop()
+            .submit(100, 100));
+
+    assertThat(bitmap.isRecycled()).isFalse();
+  }
+
+}

instrumentation/src/androidTest/java/com/bumptech/glide/LoadDrawableTest.java

@@ -0,0 +1,72 @@
+package com.bumptech.glide;
+
+
+import static com.google.common.truth.Truth.assertThat;
+
+import android.content.Context;
+import android.graphics.Bitmap;
+import android.graphics.BitmapFactory;
+import android.graphics.drawable.BitmapDrawable;
+import android.graphics.drawable.Drawable;
+import android.support.test.InstrumentationRegistry;
+import android.support.test.runner.AndroidJUnit4;
+import com.bumptech.glide.load.engine.bitmap_recycle.BitmapPoolAdapter;
+import com.bumptech.glide.load.engine.cache.MemoryCacheAdapter;
+import com.bumptech.glide.request.FutureTarget;
+import com.bumptech.glide.test.ConcurrencyHelper;
+import com.bumptech.glide.test.GlideApp;
+import com.bumptech.glide.test.ResourceIds;
+import com.bumptech.glide.test.TearDownGlide;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+@RunWith(AndroidJUnit4.class)
+public class LoadDrawableTest {
+  @Rule public final TearDownGlide tearDownGlide = new TearDownGlide();
+  private final ConcurrencyHelper concurrency = new ConcurrencyHelper();
+
+  private Context context;
+
+  @Before
+  public void setUp() {
+    context = InstrumentationRegistry.getTargetContext();
+  }
+
+  @Test
+  public void clear_withLoadedBitmapDrawable_doesNotRecycleBitmap() {
+    Glide.init(context, new GlideBuilder()
+        .setMemoryCache(new MemoryCacheAdapter())
+        .setBitmapPool(new BitmapPoolAdapter()));
+    Bitmap bitmap = BitmapFactory.decodeResource(context.getResources(), ResourceIds.raw.canonical);
+    BitmapDrawable drawable = new BitmapDrawable(context.getResources(), bitmap);
+    FutureTarget<Drawable> target =
+        concurrency.wait(
+            GlideApp.with(context)
+                .load(drawable)
+                .submit(100, 100));
+    Glide.with(context).clear(target);
+
+    // Allow Glide's resource recycler to run on the main thread.
+    concurrency.pokeMainThread();
+
+    assertThat(bitmap.isRecycled()).isFalse();
+  }
+
+  @Test
+  public void transform_withLoadedBitmapDrawable_doesNotRecycleBitmap() {
+    Glide.init(context, new GlideBuilder()
+        .setMemoryCache(new MemoryCacheAdapter())
+        .setBitmapPool(new BitmapPoolAdapter()));
+    Bitmap bitmap = BitmapFactory.decodeResource(context.getResources(), ResourceIds.raw.canonical);
+    BitmapDrawable drawable = new BitmapDrawable(context.getResources(), bitmap);
+    concurrency.wait(
+        GlideApp.with(context)
+            .load(drawable)
+            .centerCrop()
+            .submit(100, 100));
+
+    assertThat(bitmap.isRecycled()).isFalse();
+  }
+}

instrumentation/src/androidTest/java/com/bumptech/glide/NonBitmapDrawableResourcesTest.java

@@ -30,6 +30,7 @@
 import org.junit.rules.ExpectedException;
 import org.junit.rules.TestName;
 import org.junit.runner.RunWith;
+import org.mockito.MockitoAnnotations;
 
 @RunWith(AndroidJUnit4.class)
 public class NonBitmapDrawableResourcesTest {
@@ -41,6 +42,7 @@ public class NonBitmapDrawableResourcesTest {
 
   @Before
   public void setUp() {
+    MockitoAnnotations.initMocks(this);
     context = InstrumentationRegistry.getTargetContext();
   }
 

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

@@ -355,17 +355,17 @@ Registry.BUCKET_BITMAP, Bitmap.class, Bitmap.class, new UnitBitmapDecoder())
             Registry.BUCKET_BITMAP_DRAWABLE,
             ByteBuffer.class,
             BitmapDrawable.class,
-            new BitmapDrawableDecoder<>(resources, bitmapPool, byteBufferBitmapDecoder))
+            new BitmapDrawableDecoder<>(resources, byteBufferBitmapDecoder))
         .append(
             Registry.BUCKET_BITMAP_DRAWABLE,
             InputStream.class,
             BitmapDrawable.class,
-            new BitmapDrawableDecoder<>(resources, bitmapPool, streamBitmapDecoder))
+            new BitmapDrawableDecoder<>(resources, streamBitmapDecoder))
         .append(
             Registry.BUCKET_BITMAP_DRAWABLE,
             ParcelFileDescriptor.class,
             BitmapDrawable.class,
-            new BitmapDrawableDecoder<>(resources, bitmapPool, videoBitmapDecoder))
+            new BitmapDrawableDecoder<>(resources, videoBitmapDecoder))
         .append(BitmapDrawable.class, new BitmapDrawableEncoder(bitmapPool, bitmapEncoder))
         /* GIFs */
         .append(
@@ -440,7 +440,7 @@ Uri.class, Bitmap.class, new ResourceBitmapDecoder(resourceDrawableDecoder, bitm
         .register(
             Bitmap.class,
             BitmapDrawable.class,
-            new BitmapDrawableTranscoder(resources, bitmapPool))
+            new BitmapDrawableTranscoder(resources))
         .register(Bitmap.class, byte[].class, new BitmapBytesTranscoder())
         .register(GifDrawable.class, byte[].class, new GifDrawableBytesTranscoder());
 

library/src/main/java/com/bumptech/glide/load/resource/bitmap/BitmapDrawableDecoder.java

@@ -4,7 +4,7 @@
 import android.content.res.Resources;
 import android.graphics.Bitmap;
 import android.graphics.drawable.BitmapDrawable;
-import com.bumptech.glide.Glide;
+import android.support.annotation.NonNull;
 import com.bumptech.glide.load.Options;
 import com.bumptech.glide.load.ResourceDecoder;
 import com.bumptech.glide.load.engine.Resource;
@@ -21,18 +21,28 @@ public class BitmapDrawableDecoder<DataType> implements ResourceDecoder<DataType
 
   private final ResourceDecoder<DataType, Bitmap> decoder;
   private final Resources resources;
-  private final BitmapPool bitmapPool;
 
   // Public API.
-  @SuppressWarnings("unused")
+  @SuppressWarnings({"unused", "WeakerAccess"})
   public BitmapDrawableDecoder(Context context, ResourceDecoder<DataType, Bitmap> decoder) {
-    this(context.getResources(), Glide.get(context).getBitmapPool(), decoder);
+    this(context.getResources(), decoder);
   }
 
-  public BitmapDrawableDecoder(Resources resources, BitmapPool bitmapPool,
+  /**
+   * @deprecated Use {@link #BitmapDrawableDecoder(Context, ResourceDecoder)}, {@code bitmapPool}
+   * is ignored.
+   */
+  @Deprecated
+  public BitmapDrawableDecoder(
+      Resources resources,
+      @SuppressWarnings("unused") BitmapPool bitmapPool,
       ResourceDecoder<DataType, Bitmap> decoder) {
+    this(resources, decoder);
+  }
+
+  public BitmapDrawableDecoder(
+      @NonNull Resources resources, @NonNull ResourceDecoder<DataType, Bitmap> decoder) {
     this.resources = Preconditions.checkNotNull(resources);
-    this.bitmapPool = Preconditions.checkNotNull(bitmapPool);
     this.decoder = Preconditions.checkNotNull(decoder);
   }
 
@@ -45,10 +55,6 @@ public boolean handles(DataType source, Options options) throws IOException {
   public Resource<BitmapDrawable> decode(DataType source, int width, int height, Options options)
       throws IOException {
     Resource<Bitmap> bitmapResource = decoder.decode(source, width, height, options);
-    if (bitmapResource == null) {
-      return null;
-    }
-
-    return LazyBitmapDrawableResource.obtain(resources, bitmapPool, bitmapResource.get());
+    return LazyBitmapDrawableResource.obtain(resources, bitmapResource);
   }
 }

library/src/main/java/com/bumptech/glide/load/resource/bitmap/DrawableTransformation.java

@@ -63,15 +63,15 @@ public Resource<Drawable> transform(Context context, Resource<Drawable> resource
       transformedBitmapResource.recycle();
       return resource;
     } else {
-      return newDrawableResource(context, transformedBitmapResource.get());
+      return newDrawableResource(context, transformedBitmapResource);
     }
   }
 
   @SuppressWarnings("unchecked")
   private Resource<Drawable> newDrawableResource(
-      Context context, Bitmap transformed) {
+      Context context, Resource<Bitmap> transformed) {
     Resource<? extends Drawable> result =
-        LazyBitmapDrawableResource.obtain(context, transformed);
+        LazyBitmapDrawableResource.obtain(context.getResources(), transformed);
     return (Resource<Drawable>) result;
   }