From 3a7060777d17fa4d9ca2b68b019fd2d448f54ea6 Mon Sep 17 00:00:00 2001 From: azlatin Date: Fri, 22 Jun 2018 13:08:24 -0700 Subject: [PATCH] Create a CustomViewTarget to replace ViewTarget. The two main differences are: - It forces you to override the method where resources must be cleared. Not doing so results in recycled bitmaps being used and crashing apps. Not doing so was a common pattern among developers optimizing for lines of code instead of correctness. - No more setTag(object) use. Glide now targets 14+ which can safely use the id tag variant and avoid another class of runtime bugs caused by developers optimizing for lines of code instead of correctness by calling setTag() and overwriting Glide's data. Finally, we deprecate ViewTarget, SimpleTarget and BaseTarget. Apps should primarily be using Target, CustomViewTarget, ImageViewTarget and FutureTarget which either force the developer to implement all necessary methods, properly implement them themselves, or will not attempt to reclaim bitmaps. The deprecated classes continue to be used internally by some of the "correct" classes but can be merged down once the deprecated APIs are able to be removed. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=201729878 --- library/src/main/AndroidManifest.xml | 5 +- .../glide/request/target/BaseTarget.java | 19 +- .../request/target/CustomViewTarget.java | 485 +++++++++++++ .../glide/request/target/SimpleTarget.java | 34 +- .../bumptech/glide/request/target/Target.java | 21 +- .../glide/request/target/ViewTarget.java | 12 +- library/src/main/res/values/ids.xml | 4 + .../request/target/CustomViewTargetTest.java | 677 ++++++++++++++++++ .../glide/request/target/ViewTargetTest.java | 2 +- 9 files changed, 1222 insertions(+), 37 deletions(-) create mode 100644 library/src/main/java/com/bumptech/glide/request/target/CustomViewTarget.java create mode 100644 library/src/main/res/values/ids.xml create mode 100644 library/test/src/test/java/com/bumptech/glide/request/target/CustomViewTargetTest.java diff --git a/library/src/main/AndroidManifest.xml b/library/src/main/AndroidManifest.xml index 2e0f06b5f..2e69519f4 100644 --- a/library/src/main/AndroidManifest.xml +++ b/library/src/main/AndroidManifest.xml @@ -1,5 +1,6 @@ - - + + diff --git a/library/src/main/java/com/bumptech/glide/request/target/BaseTarget.java b/library/src/main/java/com/bumptech/glide/request/target/BaseTarget.java index 792de64a3..026a3233f 100644 --- a/library/src/main/java/com/bumptech/glide/request/target/BaseTarget.java +++ b/library/src/main/java/com/bumptech/glide/request/target/BaseTarget.java @@ -8,16 +8,23 @@ import com.bumptech.glide.request.Request; * A base {@link Target} for loading {@link com.bumptech.glide.load.engine.Resource}s that provides * basic or empty implementations for most methods. * - *

For maximum efficiency, clear this target when you have finished using or displaying the - * {@link com.bumptech.glide.load.engine.Resource} loaded into it using - * {@link com.bumptech.glide.RequestManager#clear(Target)}.

+ *

For maximum efficiency, clear this target when you have finished using or displaying the + * {@link com.bumptech.glide.load.engine.Resource} loaded into it using {@link + * com.bumptech.glide.RequestManager#clear(Target)}. * - *

For loading {@link com.bumptech.glide.load.engine.Resource}s into {@link android.view.View}s, - * {@link com.bumptech.glide.request.target.ViewTarget} or - * {@link com.bumptech.glide.request.target.ImageViewTarget} are preferable.

+ *

For loading {@link com.bumptech.glide.load.engine.Resource}s into {@link android.view.View}s, + * {@link com.bumptech.glide.request.target.ViewTarget} or {@link + * com.bumptech.glide.request.target.ImageViewTarget} are preferable. * * @param The type of resource that will be received by this target. + * @deprecated Use {@link CustomViewTarget} if loading the content into a view, the download API if + * in the background + * (http://bumptech.github.io/glide/doc/getting-started.html#background-threads), or a a fully + * implemented {@link Target} for any specialized use-cases. Using BaseView is unsafe if the + * user does not implement {@link #onLoadCleared}, resulting in recycled bitmaps being + * referenced from the UI and hard to root-cause crashes. */ +@Deprecated public abstract class BaseTarget implements Target { private Request request; diff --git a/library/src/main/java/com/bumptech/glide/request/target/CustomViewTarget.java b/library/src/main/java/com/bumptech/glide/request/target/CustomViewTarget.java new file mode 100644 index 000000000..27973e409 --- /dev/null +++ b/library/src/main/java/com/bumptech/glide/request/target/CustomViewTarget.java @@ -0,0 +1,485 @@ +package com.bumptech.glide.request.target; + +import android.content.Context; +import android.graphics.Point; +import android.graphics.drawable.Drawable; +import android.support.annotation.IdRes; +import android.support.annotation.NonNull; +import android.support.annotation.Nullable; +import android.support.annotation.VisibleForTesting; +import android.util.Log; +import android.view.Display; +import android.view.View; +import android.view.View.OnAttachStateChangeListener; +import android.view.ViewGroup.LayoutParams; +import android.view.ViewTreeObserver; +import android.view.WindowManager; +import com.bumptech.glide.request.Request; +import com.bumptech.glide.request.transition.Transition; +import com.bumptech.glide.util.Preconditions; +import com.bumptech.glide.util.Synthetic; +import java.lang.ref.WeakReference; +import java.util.ArrayList; +import java.util.List; + +/** + * A base {@link Target} for loading {@link android.graphics.Bitmap}s into {@link View}s that + * provides default implementations for most methods and can determine the size of views using a + * {@link android.view.ViewTreeObserver.OnDrawListener}. + * + * @param The specific subclass of view wrapped by this target. + * @param The resource type this target will receive. + */ +public abstract class CustomViewTarget implements Target { + private static final String TAG = "CustomViewTarget"; + @IdRes private static final int VIEW_TAG_ID = + com.bumptech.glide.R.id.glide_custom_view_target_tag; + + private final SizeDeterminer sizeDeterminer; + + protected final T view; + @Nullable private OnAttachStateChangeListener attachStateListener; + private boolean isClearedByUs; + private boolean isAttachStateListenerAdded; + @IdRes private int overrideTag = 0; + + /** Constructor that defaults {@code waitForLayout} to {@code false}. */ + public CustomViewTarget(@NonNull T view) { + this.view = Preconditions.checkNotNull(view); + sizeDeterminer = new SizeDeterminer(view); + } + + /** + * A required callback invoked when the resource is no longer valid and must be freed. + * + *

You must ensure that any current Drawable received in {@link #onResourceReady(Z, + * Transition)} is no longer used before redrawing the container (usually a View) or changing its + * visibility. Not doing so will result in crashes in your app. + * + * @param placeholder The placeholder drawable to optionally show, or null. + */ + protected abstract void onResourceCleared(@Nullable Drawable placeholder); + + /** + * An optional callback invoked when a resource load is started. + * + * @see Target#onLoadStarted(Drawable) + * @param placeholder The placeholder drawable to optionally show, or null. + */ + protected void onResourceLoading(@Nullable Drawable placeholder) {} + + @Override + public void onStart() {} + + @Override + public void onStop() {} + + @Override + public void onDestroy() {} + + /** + * Indicates that Glide should always wait for any pending layout pass before checking for the + * size an {@link View}. + * + *

By default, Glide will only wait for a pending layout pass if it's unable to resolve the + * size from the {@link LayoutParams} or valid non-zero values for {@link View#getWidth()} and + * {@link View#getHeight()}. + * + *

Because calling this method forces Glide to wait for the layout pass to occur before + * starting loads, setting this parameter to {@code true} can cause Glide to asynchronous load an + * image even if it's in the memory cache. The load will happen asynchronously because Glide has + * to wait for a layout pass to occur, which won't necessarily happen in the same frame as when + * the image is requested. As a result, using this method can resulting in flashing in some cases + * and should be used sparingly. + * + *

If the {@link LayoutParams} of the wrapped {@link View} are set to fixed sizes, they will + * still be used instead of the {@link View}'s dimensions even if this method is called. This + * parameter is a fallback only. + */ + @SuppressWarnings("WeakerAccess") // Public API + @NonNull + public final CustomViewTarget waitForLayout() { + sizeDeterminer.waitForLayout = true; + return this; + } + + /** + * Clears the {@link View}'s {@link Request} when the {@link View} is detached from its {@link + * android.view.Window} and restarts the {@link Request} when the {@link View} is re-attached from + * its {@link android.view.Window}. + * + *

This is an experimental API that may be removed in a future version. + * + *

Using this method can save memory by allowing Glide to more eagerly clear resources when + * transitioning screens or swapping adapters in scrolling views. However it also substantially + * increases the odds that images will not be in memory if users subsequently return to a screen + * where images were previously loaded. Whether or not this happens will depend on the number of + * images loaded in the new screen and the size of the memory cache. Increasing the size of the + * memory cache can improve this behavior but it largely negates the memory benefits of using this + * method. + * + *

Use this method with caution and measure your memory usage to ensure that it's actually + * improving your memory usage in the cases you care about. + */ + // Public API. + @NonNull + @SuppressWarnings({"UnusedReturnValue", "WeakerAccess"}) + public final CustomViewTarget clearOnDetach() { + if (attachStateListener != null) { + return this; + } + attachStateListener = new OnAttachStateChangeListener() { + @Override + public void onViewAttachedToWindow(View v) { + resumeMyRequest(); + } + + @Override + public void onViewDetachedFromWindow(View v) { + pauseMyRequest(); + } + }; + maybeAddAttachStateListener(); + return this; + } + + /** + * Override the android resource id to store temporary state allowing loads to be automatically + * cancelled and resources re-used in scrolling lists. + * + *

Unlike {@link ViewTarget}, it is not necessary to set a custom tag id if your app + * uses {@link View#setTag(Object)}. It is only necessary if loading several Glide resources into + * the same view, for example one foreground and one background view. + * + * @param tagId The android resource id to use. + */ + // Public API. + @SuppressWarnings({"UnusedReturnValue", "WeakerAccess"}) + public final CustomViewTarget useTagId(@IdRes int tagId) { + if (this.overrideTag != 0) { + throw new IllegalArgumentException("You cannot change the tag id once it has been set."); + } + this.overrideTag = tagId; + return this; + } + + /** Returns the wrapped {@link android.view.View}. */ + @NonNull + public final T getView() { + return view; + } + + /** + * Determines the size of the view by first checking {@link android.view.View#getWidth()} and + * {@link android.view.View#getHeight()}. If one or both are zero, it then checks the view's + * {@link LayoutParams}. If one or both of the params width and height are less than or equal to + * zero, it then adds an {@link android.view.ViewTreeObserver.OnPreDrawListener} which waits until + * the view has been measured before calling the callback with the view's drawn width and height. + * + * @param cb {@inheritDoc} + */ + @Override + public final void getSize(@NonNull SizeReadyCallback cb) { + sizeDeterminer.getSize(cb); + } + + @Override + public final void removeCallback(@NonNull SizeReadyCallback cb) { + sizeDeterminer.removeCallback(cb); + } + + @Override + public final void onLoadStarted(@Nullable Drawable placeholder) { + maybeAddAttachStateListener(); + onResourceLoading(placeholder); + } + + @Override + public final void onLoadCleared(@Nullable Drawable placeholder) { + sizeDeterminer.clearCallbacksAndListener(); + + onResourceCleared(placeholder); + if (!isClearedByUs) { + maybeRemoveAttachStateListener(); + } + } + + /** + * Stores the request using {@link View#setTag(Object)}. + * + * @param request {@inheritDoc} + */ + @Override + public final void setRequest(@Nullable Request request) { + setTag(request); + } + + /** Returns any stored request using {@link android.view.View#getTag()}. */ + @Override + @Nullable + public final Request getRequest() { + Object tag = getTag(); + if (tag != null) { + if (tag instanceof Request) { + return (Request) tag; + } else { + throw new IllegalArgumentException("You must not pass non-R.id ids to setTag(id)"); + } + } + return null; + } + + @Override + public String toString() { + return "Target for: " + view; + } + + @SuppressWarnings("WeakerAccess") + @Synthetic + final void resumeMyRequest() { + Request request = getRequest(); + if (request != null && request.isCleared()) { + request.begin(); + } + } + + @SuppressWarnings("WeakerAccess") + @Synthetic + final void pauseMyRequest() { + Request request = getRequest(); + if (request != null) { + isClearedByUs = true; + request.clear(); + isClearedByUs = false; + } + } + + private void setTag(@Nullable Object tag) { + view.setTag(overrideTag == 0 ? VIEW_TAG_ID : overrideTag, tag); + } + + @Nullable + private Object getTag() { + return view.getTag(overrideTag == 0 ? VIEW_TAG_ID : overrideTag); + } + + private void maybeAddAttachStateListener() { + if (attachStateListener == null || isAttachStateListenerAdded) { + return; + } + + view.addOnAttachStateChangeListener(attachStateListener); + isAttachStateListenerAdded = true; + } + + private void maybeRemoveAttachStateListener() { + if (attachStateListener == null || !isAttachStateListenerAdded) { + return; + } + + view.removeOnAttachStateChangeListener(attachStateListener); + isAttachStateListenerAdded = false; + } + + @VisibleForTesting + static final class SizeDeterminer { + // Some negative sizes (Target.SIZE_ORIGINAL) are valid, 0 is never valid. + private static final int PENDING_SIZE = 0; + @VisibleForTesting + @Nullable + static Integer maxDisplayLength; + private final View view; + private final List cbs = new ArrayList<>(); + @Synthetic boolean waitForLayout; + + @Nullable private SizeDeterminerLayoutListener layoutListener; + + SizeDeterminer(@NonNull View view) { + this.view = view; + } + + // Use the maximum to avoid depending on the device's current orientation. + private static int getMaxDisplayLength(@NonNull Context context) { + if (maxDisplayLength == null) { + WindowManager windowManager = + (WindowManager) context.getSystemService(Context.WINDOW_SERVICE); + Display display = Preconditions.checkNotNull(windowManager).getDefaultDisplay(); + Point displayDimensions = new Point(); + display.getSize(displayDimensions); + maxDisplayLength = Math.max(displayDimensions.x, displayDimensions.y); + } + return maxDisplayLength; + } + + private void notifyCbs(int width, int height) { + // One or more callbacks may trigger the removal of one or more additional callbacks, so we + // need a copy of the list to avoid a concurrent modification exception. One place this + // happens is when a full request completes from the in memory cache while its thumbnail is + // still being loaded asynchronously. See #2237. + for (SizeReadyCallback cb : new ArrayList<>(cbs)) { + cb.onSizeReady(width, height); + } + } + + @Synthetic + void checkCurrentDimens() { + if (cbs.isEmpty()) { + return; + } + + int currentWidth = getTargetWidth(); + int currentHeight = getTargetHeight(); + if (!isViewStateAndSizeValid(currentWidth, currentHeight)) { + return; + } + + notifyCbs(currentWidth, currentHeight); + clearCallbacksAndListener(); + } + + void getSize(@NonNull SizeReadyCallback cb) { + int currentWidth = getTargetWidth(); + int currentHeight = getTargetHeight(); + if (isViewStateAndSizeValid(currentWidth, currentHeight)) { + cb.onSizeReady(currentWidth, currentHeight); + return; + } + + // We want to notify callbacks in the order they were added and we only expect one or two + // callbacks to be added a time, so a List is a reasonable choice. + if (!cbs.contains(cb)) { + cbs.add(cb); + } + if (layoutListener == null) { + ViewTreeObserver observer = view.getViewTreeObserver(); + layoutListener = new SizeDeterminerLayoutListener(this); + observer.addOnPreDrawListener(layoutListener); + } + } + + /** + * The callback may be called anyway if it is removed by another {@link SizeReadyCallback} or + * otherwise removed while we're notifying the list of callbacks. + * + *

See #2237. + */ + void removeCallback(@NonNull SizeReadyCallback cb) { + cbs.remove(cb); + } + + void clearCallbacksAndListener() { + // Keep a reference to the layout attachStateListener and remove it here + // rather than having the observer remove itself because the observer + // we add the attachStateListener to will be almost immediately merged into + // another observer and will therefore never be alive. If we instead + // keep a reference to the attachStateListener and remove it here, we get the + // current view tree observer and should succeed. + ViewTreeObserver observer = view.getViewTreeObserver(); + if (observer.isAlive()) { + observer.removeOnPreDrawListener(layoutListener); + } + layoutListener = null; + cbs.clear(); + } + + private boolean isViewStateAndSizeValid(int width, int height) { + return isDimensionValid(width) && isDimensionValid(height); + } + + private int getTargetHeight() { + int verticalPadding = view.getPaddingTop() + view.getPaddingBottom(); + LayoutParams layoutParams = view.getLayoutParams(); + int layoutParamSize = layoutParams != null ? layoutParams.height : PENDING_SIZE; + return getTargetDimen(view.getHeight(), layoutParamSize, verticalPadding); + } + + private int getTargetWidth() { + int horizontalPadding = view.getPaddingLeft() + view.getPaddingRight(); + LayoutParams layoutParams = view.getLayoutParams(); + int layoutParamSize = layoutParams != null ? layoutParams.width : PENDING_SIZE; + return getTargetDimen(view.getWidth(), layoutParamSize, horizontalPadding); + } + + private int getTargetDimen(int viewSize, int paramSize, int paddingSize) { + // We consider the View state as valid if the View has non-null layout params and a non-zero + // layout params width and height. This is imperfect. We're making an assumption that View + // parents will obey their child's layout parameters, which isn't always the case. + int adjustedParamSize = paramSize - paddingSize; + if (adjustedParamSize > 0) { + return adjustedParamSize; + } + + // Since we always prefer layout parameters with fixed sizes, even if waitForLayout is true, + // we might as well ignore it and just return the layout parameters above if we have them. + // Otherwise we should wait for a layout pass before checking the View's dimensions. + if (waitForLayout && view.isLayoutRequested()) { + return PENDING_SIZE; + } + + // We also consider the View state valid if the View has a non-zero width and height. This + // means that the View has gone through at least one layout pass. It does not mean the Views + // width and height are from the current layout pass. For example, if a View is re-used in + // RecyclerView or ListView, this width/height may be from an old position. In some cases + // the dimensions of the View at the old position may be different than the dimensions of the + // View in the new position because the LayoutManager/ViewParent can arbitrarily decide to + // change them. Nevertheless, in most cases this should be a reasonable choice. + int adjustedViewSize = viewSize - paddingSize; + if (adjustedViewSize > 0) { + return adjustedViewSize; + } + + // Finally we consider the view valid if the layout parameter size is set to wrap_content. + // It's difficult for Glide to figure out what to do here. Although Target.SIZE_ORIGINAL is a + // coherent choice, it's extremely dangerous because original images may be much too large to + // fit in memory or so large that only a couple can fit in memory, causing OOMs. If users want + // the original image, they can always use .override(Target.SIZE_ORIGINAL). Since wrap_content + // may never resolve to a real size unless we load something, we aim for a square whose length + // is the largest screen size. That way we're loading something and that something has some + // hope of being downsampled to a size that the device can support. We also log a warning that + // tries to explain what Glide is doing and why some alternatives are preferable. + // Since WRAP_CONTENT is sometimes used as a default layout parameter, we always wait for + // layout to complete before using this fallback parameter (ConstraintLayout among others). + if (!view.isLayoutRequested() && paramSize == LayoutParams.WRAP_CONTENT) { + if (Log.isLoggable(TAG, Log.INFO)) { + Log.i(TAG, "Glide treats LayoutParams.WRAP_CONTENT as a request for an image the size of" + + " this device's screen dimensions. If you want to load the original image and are" + + " ok with the corresponding memory cost and OOMs (depending on the input size), use" + + " .override(Target.SIZE_ORIGINAL). Otherwise, use LayoutParams.MATCH_PARENT, set" + + " layout_width and layout_height to fixed dimension, or use .override() with fixed" + + " dimensions."); + } + return getMaxDisplayLength(view.getContext()); + } + + // If the layout parameters are < padding, the view size is < padding, or the layout + // parameters are set to match_parent or wrap_content and no layout has occurred, we should + // wait for layout and repeat. + return PENDING_SIZE; + } + + private boolean isDimensionValid(int size) { + return size > 0 || size == SIZE_ORIGINAL; + } + + private static final class SizeDeterminerLayoutListener + implements ViewTreeObserver.OnPreDrawListener { + private final WeakReference sizeDeterminerRef; + + SizeDeterminerLayoutListener(@NonNull SizeDeterminer sizeDeterminer) { + sizeDeterminerRef = new WeakReference<>(sizeDeterminer); + } + + @Override + public boolean onPreDraw() { + if (Log.isLoggable(TAG, Log.VERBOSE)) { + Log.v(TAG, "OnGlobalLayoutListener called attachStateListener=" + this); + } + SizeDeterminer sizeDeterminer = sizeDeterminerRef.get(); + if (sizeDeterminer != null) { + sizeDeterminer.checkCurrentDimens(); + } + return true; + } + } + } +} diff --git a/library/src/main/java/com/bumptech/glide/request/target/SimpleTarget.java b/library/src/main/java/com/bumptech/glide/request/target/SimpleTarget.java index 4c2143ab1..107330551 100644 --- a/library/src/main/java/com/bumptech/glide/request/target/SimpleTarget.java +++ b/library/src/main/java/com/bumptech/glide/request/target/SimpleTarget.java @@ -9,6 +9,7 @@ import com.bumptech.glide.util.Util; * A simple {@link com.bumptech.glide.request.target.Target} base class with default (usually no-op) * implementations of non essential methods that allows the caller to specify an exact width/height. * Typically use cases look something like this: + * * 

  * 
  * Target target =
@@ -40,27 +41,32 @@ import com.bumptech.glide.util.Util;
  * load into the same {@link View} or caller repeatedly using this class, always retain a reference
  * to the previous instance and either call {@link com.bumptech.glide.RequestManager#clear(Target)}
  * on the old instance before starting a new load or you must re-use the old instance for the new
- * load. Glide's {@link com.bumptech.glide.RequestBuilder#into(Target)} method returns the
- * {@link Target} instance you provided to make retaining a reference to the {@link Target} as easy
- * as possible. That said, you must wait until you're completely finished with the resource before
+ * load. Glide's {@link com.bumptech.glide.RequestBuilder#into(Target)} method returns the {@link
+ * Target} instance you provided to make retaining a reference to the {@link Target} as easy as
+ * possible. That said, you must wait until you're completely finished with the resource before
  * calling {@link com.bumptech.glide.RequestManager#clear(Target)} and you should always null out
  * references to any loaded resources in {@link Target#onLoadCleared(Drawable)}.
  *
- * 
Always try to provide a size when using this class. Use
- * {@link SimpleTarget#SimpleTarget(int, int)} whenever possible with values that are not
- * {@link Target#SIZE_ORIGINAL}. Using {@link Target#SIZE_ORIGINAL} is unsafe if you're loading
- * large images or are running your application on older or memory constrained devices because it
- * can cause Glide to load very large images into memory. In some cases those images may throw
- * {@link OutOfMemoryError} and in others they may exceed the texture limit for the device, which
- * will prevent them from being rendered. Providing a valid size allows Glide to downsample large
- * images, which can avoid issues with texture size or memory limitations. You don't have to worry
- * about providing a size in most cases if you use {@link ViewTarget} so prefer {@link ViewTarget}
- * over this class whenver possible.
+ * 
Always try to provide a size when using this class. Use {@link SimpleTarget#SimpleTarget(int,
+ * int)} whenever possible with values that are not {@link Target#SIZE_ORIGINAL}. Using
+ * {@link Target#SIZE_ORIGINAL} is unsafe if you're loading large images or are running your
+ * application on older or memory constrained devices because it can cause Glide to load very large
+ * images into memory. In some cases those images may throw {@link OutOfMemoryError} and in others
+ * they may exceed the texture limit for the device, which will prevent them from being rendered.
+ * Providing a valid size allows Glide to downsample large images, which can avoid issues with
+ * texture size or memory limitations. You don't have to worry about providing a size in most cases
+ * if you use {@link ViewTarget} so prefer {@link ViewTarget} over this class whenver possible.
  *
  * @see Glide's Target docs page
- *
  * @param  The type of resource that this target will receive.
+ * @deprecated Use {@link CustomViewTarget} if loading the content into a view, the download API if
+ *     in the background
+ *     (http://bumptech.github.io/glide/doc/getting-started.html#background-threads), or a a fully
+ *     implemented {@link Target} for any specialized use-cases. Using BaseView is unsafe if the
+ *     user does not implement {@link #onLoadCleared}, resulting in recycled bitmaps being
+ *     referenced from the UI and hard to root-cause crashes.
  */
+@Deprecated
 public abstract class SimpleTarget extends BaseTarget {
   private final int width;
   private final int height;
diff --git a/library/src/main/java/com/bumptech/glide/request/target/Target.java b/library/src/main/java/com/bumptech/glide/request/target/Target.java
index e9eb1a538..502234774 100644
--- a/library/src/main/java/com/bumptech/glide/request/target/Target.java
+++ b/library/src/main/java/com/bumptech/glide/request/target/Target.java
@@ -42,14 +42,14 @@ public interface Target extends LifecycleListener {
   void onLoadStarted(@Nullable Drawable placeholder);
 
   /**
-   * A lifecycle callback that is called when a load fails.
+   * A mandatory lifecycle callback that is called when a load fails.
    *
-   * 
 Note - This may be called before {@link #onLoadStarted(android.graphics.drawable.Drawable)
-   * } if the model object is null.
+   * 
Note - This may be called before {@link #onLoadStarted(android.graphics.drawable.Drawable) }
+   * if the model object is null.
    *
-   * 
You must ensure that any current Drawable received in {@link #onResourceReady(Object,
-   * Transition)} is no longer displayed before redrawing the container (usually a View) or
-   * changing its visibility.
+   * 
You must ensure that any current Drawable received in {@link #onResourceReady(Object,
+   * Transition)} is no longer used before redrawing the container (usually a View) or changing its
+   * visibility.
    *
    * @param errorDrawable The error drawable to optionally show, or null.
    */
@@ -63,11 +63,12 @@ public interface Target extends LifecycleListener {
   void onResourceReady(@NonNull R resource, @Nullable Transition transition);
 
   /**
-   * A lifecycle callback that is called when a load is cancelled and its resources are freed.
+   * A mandatory lifecycle callback that is called when a load is cancelled and its resources
+   * are freed.
    *
-   * 
You must ensure that any current Drawable received in {@link #onResourceReady(Object,
-   * Transition)} is no longer displayed before redrawing the container (usually a View) or
-   * changing its visibility.
+   * 
You must ensure that any current Drawable received in {@link #onResourceReady(Object,
+   * Transition)} is no longer used before redrawing the container (usually a View) or changing its
+   * visibility.
    *
    * @param placeholder The placeholder drawable to optionally show, or null.
    */
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 8df9ef60d..9bc6d8b2e 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
@@ -26,21 +26,25 @@ import java.util.List;
  * provides default implementations for most most methods and can determine the size of views using
  * a {@link android.view.ViewTreeObserver.OnDrawListener}.
  *
- * 
 To detect {@link View} reuse in {@link android.widget.ListView} or any {@link
+ * 
To detect {@link View} reuse in {@link android.widget.ListView} or any {@link
  * android.view.ViewGroup} that reuses views, this class uses the {@link View#setTag(Object)} method
  * to store some metadata so that if a view is reused, any previous loads or resources from previous
- * loads can be cancelled or reused. 

+ * loads can be cancelled or reused.
  *
- * 
 Any calls to {@link View#setTag(Object)}} on a View given to this class will result in
+ * 
Any calls to {@link View#setTag(Object)}} on a View given to this class will result in
  * excessive allocations and and/or {@link IllegalArgumentException}s. If you must call {@link
  * View#setTag(Object)} on a view, use {@link #setTagId(int)} to specify a custom tag for Glide to
  * use.
  *
- * 
 Subclasses must call super in {@link #onLoadCleared(Drawable)} 

+ * 
Subclasses must call super in {@link #onLoadCleared(Drawable)}
  *
  * @param  The specific subclass of view wrapped by this target.
  * @param  The resource type this target will receive.
+ * @deprecated Use {@link CustomViewTarget}. Using this class is unsafe without implementing {@link
+ *     #onLoadCleared} and results in recycled bitmaps being referenced from the UI and hard to
+ *     root-cause crashes.
  */
+@Deprecated
 public abstract class ViewTarget extends BaseTarget {
   private static final String TAG = "ViewTarget";
   private static boolean isTagUsedAtLeastOnce;
diff --git a/library/src/main/res/values/ids.xml b/library/src/main/res/values/ids.xml
new file mode 100644
index 000000000..c8c7c74dd
--- /dev/null
+++ b/library/src/main/res/values/ids.xml
@@ -0,0 +1,4 @@
+
+
+  
+
\ No newline at end of file
diff --git a/library/test/src/test/java/com/bumptech/glide/request/target/CustomViewTargetTest.java b/library/test/src/test/java/com/bumptech/glide/request/target/CustomViewTargetTest.java
new file mode 100644
index 000000000..728fbc35d
--- /dev/null
+++ b/library/test/src/test/java/com/bumptech/glide/request/target/CustomViewTargetTest.java
@@ -0,0 +1,677 @@
+package com.bumptech.glide.request.target;
+
+import static android.view.ViewGroup.LayoutParams;
+import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+import static org.mockito.Matchers.anyInt;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.inOrder;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import android.app.Activity;
+import android.content.Context;
+import android.graphics.drawable.Drawable;
+import android.os.Build;
+import android.support.annotation.NonNull;
+import android.support.annotation.Nullable;
+import android.view.View;
+import android.view.View.OnAttachStateChangeListener;
+import android.view.ViewGroup;
+import android.view.ViewTreeObserver;
+import android.view.WindowManager;
+import android.widget.FrameLayout;
+import com.bumptech.glide.request.Request;
+import com.bumptech.glide.request.transition.Transition;
+import com.bumptech.glide.tests.Util;
+import com.bumptech.glide.util.Preconditions;
+import java.util.concurrent.atomic.AtomicInteger;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InOrder;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+import org.robolectric.Robolectric;
+import org.robolectric.RobolectricTestRunner;
+import org.robolectric.RuntimeEnvironment;
+import org.robolectric.Shadows;
+import org.robolectric.android.controller.ActivityController;
+import org.robolectric.annotation.Config;
+import org.robolectric.shadows.ShadowDisplay;
+
+@RunWith(RobolectricTestRunner.class)
+@Config(sdk = 19)
+public class CustomViewTargetTest {
+  private ActivityController activity;
+  private View view;
+  private ViewGroup parent;
+  private CustomViewTarget target;
+  @Mock private SizeReadyCallback cb;
+  @Mock private Request request;
+  private int sdkVersion;
+  private AttachStateTarget attachStateTarget;
+
+  @Before
+  public void setUp() {
+    sdkVersion = Build.VERSION.SDK_INT;
+    MockitoAnnotations.initMocks(this);
+    activity = Robolectric.buildActivity(Activity.class).create().start().postCreate(null).resume();
+    view = new View(activity.get());
+    target = new TestViewTarget(view);
+    attachStateTarget = new AttachStateTarget(view);
+
+    activity.get().setContentView(view);
+    parent = (ViewGroup) view.getParent();
+  }
+
+  @After
+  public void tearDown() {
+    Util.setSdkVersionInt(sdkVersion);
+    CustomViewTarget.SizeDeterminer.maxDisplayLength = null;
+  }
+
+  @Test
+  public void testReturnsWrappedView() {
+    assertEquals(view, target.getView());
+  }
+
+  @Test
+  public void testReturnsNullFromGetRequestIfNoRequestSet() {
+    assertNull(target.getRequest());
+  }
+
+  @Test
+  public void testCanSetAndRetrieveRequest() {
+    target.setRequest(request);
+
+    assertEquals(request, target.getRequest());
+  }
+
+  @Test
+  public void testRetrievesRequestFromPreviousTargetForView() {
+    target.setRequest(request);
+
+    CustomViewTarget second = new TestViewTarget(view);
+
+    assertEquals(request, second.getRequest());
+  }
+
+  @Test
+  public void testSizeCallbackIsCalledSynchronouslyIfViewSizeSet() {
+    int dimens = 333;
+    activity.get().setContentView(view);
+    view.layout(0, 0, dimens, dimens);
+
+    target.getSize(cb);
+
+    verify(cb).onSizeReady(eq(dimens), eq(dimens));
+  }
+
+  @Test
+  public void testSizeCallbackIsCalledSynchronouslyIfLayoutParamsConcreteSizeSet() {
+    int dimens = 444;
+    LayoutParams layoutParams = new FrameLayout.LayoutParams(dimens, dimens);
+    view.setLayoutParams(layoutParams);
+    view.requestLayout();
+
+    target.getSize(cb);
+
+    verify(cb).onSizeReady(eq(dimens), eq(dimens));
+  }
+
+  @Test
+  public void getSize_withBothWrapContent_usesDisplayDimens() {
+    LayoutParams layoutParams =
+        new FrameLayout.LayoutParams(LayoutParams.WRAP_CONTENT, LayoutParams.WRAP_CONTENT);
+    view.setLayoutParams(layoutParams);
+
+    setDisplayDimens(200, 300);
+
+    activity.visible();
+    view.layout(0, 0, 0, 0);
+
+    target.getSize(cb);
+
+    verify(cb).onSizeReady(300, 300);
+  }
+
+  @Test
+  public void getSize_withWrapContentWidthAndValidHeight_usesDisplayDimenAndValidHeight() {
+    int height = 100;
+    LayoutParams params = new FrameLayout.LayoutParams(LayoutParams.WRAP_CONTENT, height);
+    view.setLayoutParams(params);
+
+    setDisplayDimens(100, 200);
+
+    activity.visible();
+    view.setRight(0);
+
+    target.getSize(cb);
+
+    verify(cb).onSizeReady(200, height);
+  }
+
+  @Test
+  public void getSize_withWrapContentHeightAndValidWidth_returnsWidthAndDisplayDimen() {
+    int width = 100;
+    LayoutParams params = new FrameLayout.LayoutParams(width, LayoutParams.WRAP_CONTENT);
+    view.setLayoutParams(params);
+    setDisplayDimens(200, 100);
+    parent.getLayoutParams().height = 200;
+
+    activity.visible();
+
+    target.getSize(cb);
+
+    verify(cb).onSizeReady(width, 200);
+  }
+
+  @Test
+  public void getSize_withWrapContentWidthAndMatchParentHeight_usesDisplayDimenWidthAndHeight() {
+    LayoutParams params =
+        new FrameLayout.LayoutParams(LayoutParams.WRAP_CONTENT, LayoutParams.MATCH_PARENT);
+    view.setLayoutParams(params);
+
+    setDisplayDimens(500, 600);
+
+    target.getSize(cb);
+
+    verify(cb, never()).onSizeReady(anyInt(), anyInt());
+
+    int height = 32;
+    parent.getLayoutParams().height = height;
+    activity.visible();
+
+    view.getViewTreeObserver().dispatchOnPreDraw();
+
+    verify(cb).onSizeReady(600, height);
+  }
+
+  @Test
+  public void getSize_withMatchParentWidthAndWrapContentHeight_usesWidthAndDisplayDimenHeight() {
+    LayoutParams params =
+        new FrameLayout.LayoutParams(LayoutParams.MATCH_PARENT, LayoutParams.WRAP_CONTENT);
+    view.setLayoutParams(params);
+
+    setDisplayDimens(300, 400);
+
+    target.getSize(cb);
+
+    verify(cb, never()).onSizeReady(anyInt(), anyInt());
+
+    int width = 32;
+    parent.getLayoutParams().width = 32;
+    activity.visible();
+    view.getViewTreeObserver().dispatchOnPreDraw();
+
+    verify(cb).onSizeReady(width, 400);
+  }
+
+  @Test
+  public void testMatchParentWidthAndHeight() {
+    LayoutParams params =
+        new FrameLayout.LayoutParams(LayoutParams.MATCH_PARENT, LayoutParams.MATCH_PARENT);
+    view.setLayoutParams(params);
+
+    target.getSize(cb);
+
+    verify(cb, never()).onSizeReady(anyInt(), anyInt());
+
+    int width = 32;
+    int height = 45;
+    parent.getLayoutParams().width = width;
+    parent.getLayoutParams().height = height;
+    activity.visible();
+    view.getViewTreeObserver().dispatchOnPreDraw();
+
+    verify(cb).onSizeReady(eq(width), eq(height));
+  }
+
+  @Test
+  public void testSizeCallbackIsCalledPreDrawIfNoDimensAndNoLayoutParams() {
+    target.getSize(cb);
+
+    int width = 12;
+    int height = 32;
+    parent.getLayoutParams().width = width;
+    parent.getLayoutParams().height = height;
+    activity.visible();
+    view.getViewTreeObserver().dispatchOnPreDraw();
+
+    verify(cb).onSizeReady(eq(width), eq(height));
+  }
+
+  @Test
+  public void testSizeCallbacksAreCalledInOrderPreDraw() {
+    SizeReadyCallback[] cbs = new SizeReadyCallback[25];
+    for (int i = 0; i < cbs.length; i++) {
+      cbs[i] = mock(SizeReadyCallback.class);
+      target.getSize(cbs[i]);
+    }
+
+    int width = 100, height = 111;
+    parent.getLayoutParams().width = width;
+    parent.getLayoutParams().height = height;
+    activity.visible();
+    view.getViewTreeObserver().dispatchOnPreDraw();
+
+    InOrder order = inOrder((Object[]) cbs);
+    for (SizeReadyCallback cb : cbs) {
+      order.verify(cb).onSizeReady(eq(width), eq(height));
+    }
+  }
+
+  @Test
+  public void testDoesNotNotifyCallbackTwiceIfAddedTwice() {
+    target.getSize(cb);
+    target.getSize(cb);
+
+    view.setLayoutParams(new FrameLayout.LayoutParams(100, 100));
+    activity.visible();
+    view.getViewTreeObserver().dispatchOnPreDraw();
+
+    verify(cb, times(1)).onSizeReady(anyInt(), anyInt());
+  }
+
+  @Test
+  public void testDoesNotAddMultipleListenersIfMultipleCallbacksAreAdded() {
+    SizeReadyCallback cb1 = mock(SizeReadyCallback.class);
+    SizeReadyCallback cb2 = mock(SizeReadyCallback.class);
+    target.getSize(cb1);
+    target.getSize(cb2);
+    view.getViewTreeObserver().dispatchOnPreDraw();
+    // assertThat(shadowObserver.getPreDrawListeners()).hasSize(1);
+  }
+
+  @Test
+  public void testDoesAddSecondListenerIfFirstListenerIsRemovedBeforeSecondRequest() {
+    SizeReadyCallback cb1 = mock(SizeReadyCallback.class);
+    target.getSize(cb1);
+
+    view.setLayoutParams(new FrameLayout.LayoutParams(100, 100));
+    activity.visible();
+    view.getViewTreeObserver().dispatchOnPreDraw();
+
+    SizeReadyCallback cb2 = mock(SizeReadyCallback.class);
+    view.setLayoutParams(
+        new FrameLayout.LayoutParams(LayoutParams.MATCH_PARENT, LayoutParams.MATCH_PARENT));
+    target.getSize(cb2);
+
+    view.setLayoutParams(new FrameLayout.LayoutParams(100, 100));
+    view.getViewTreeObserver().dispatchOnPreDraw();
+
+    verify(cb2).onSizeReady(anyInt(), anyInt());
+  }
+
+  @Test
+  public void testSizeCallbackIsNotCalledPreDrawIfNoDimensSetOnPreDraw() {
+    target.getSize(cb);
+    view.getViewTreeObserver().dispatchOnPreDraw();
+
+    verify(cb, never()).onSizeReady(anyInt(), anyInt());
+
+    activity.visible();
+    verify(cb).onSizeReady(anyInt(), anyInt());
+  }
+
+  @Test
+  public void testSizeCallbackIsCalledPreDrawIfNoDimensAndNoLayoutParamsButLayoutParamsSetLater() {
+    target.getSize(cb);
+
+    int width = 689;
+    int height = 354;
+    LayoutParams layoutParams = new FrameLayout.LayoutParams(width, height);
+    view.setLayoutParams(layoutParams);
+    view.requestLayout();
+    view.getViewTreeObserver().dispatchOnPreDraw();
+
+    verify(cb).onSizeReady(eq(width), eq(height));
+  }
+
+  @Test
+  public void testCallbackIsNotCalledTwiceIfPreDrawFiresTwice() {
+    activity.visible();
+    target.getSize(cb);
+
+    LayoutParams layoutParams = new FrameLayout.LayoutParams(1234, 4123);
+    view.setLayoutParams(layoutParams);
+    view.requestLayout();
+    view.getViewTreeObserver().dispatchOnPreDraw();
+    view.getViewTreeObserver().dispatchOnPreDraw();
+
+    verify(cb, times(1)).onSizeReady(anyInt(), anyInt());
+  }
+
+  @Test
+  public void testCallbacksFromMultipleRequestsAreNotifiedOnPreDraw() {
+    SizeReadyCallback firstCb = mock(SizeReadyCallback.class);
+    SizeReadyCallback secondCb = mock(SizeReadyCallback.class);
+    target.getSize(firstCb);
+    target.getSize(secondCb);
+
+    int width = 68;
+    int height = 875;
+    LayoutParams layoutParams = new FrameLayout.LayoutParams(width, height);
+    view.setLayoutParams(layoutParams);
+    activity.visible();
+    view.getViewTreeObserver().dispatchOnPreDraw();
+    view.getViewTreeObserver().dispatchOnPreDraw();
+
+    verify(firstCb, times(1)).onSizeReady(eq(width), eq(height));
+    verify(secondCb, times(1)).onSizeReady(eq(width), eq(height));
+  }
+
+  @Test
+  public void testDoesNotThrowOnPreDrawIfViewTreeObserverIsDead() {
+    target.getSize(cb);
+
+    int width = 1;
+    int height = 2;
+    LayoutParams layoutParams = new FrameLayout.LayoutParams(width, height);
+    view.setLayoutParams(layoutParams);
+    ViewTreeObserver vto = view.getViewTreeObserver();
+    view.requestLayout();
+    activity.visible();
+    assertFalse(vto.isAlive());
+    vto.dispatchOnPreDraw();
+
+    verify(cb).onSizeReady(eq(width), eq(height));
+  }
+
+  @Test(expected = NullPointerException.class)
+  public void testThrowsIfGivenNullView() {
+    new TestViewTarget(null);
+  }
+
+  @Test
+  public void testDecreasesDimensionsByViewPadding() {
+    activity.visible();
+    view.setLayoutParams(new FrameLayout.LayoutParams(100, 100));
+    view.setPadding(25, 25, 25, 25);
+    view.requestLayout();
+
+    target.getSize(cb);
+
+    verify(cb).onSizeReady(50, 50);
+  }
+
+  @Test
+  public void getSize_withValidWidthAndHeight_notLaidOut_notLayoutRequested_callsSizeReady() {
+    view.setRight(100);
+    view.setBottom(100);
+    target.getSize(cb);
+
+    verify(cb).onSizeReady(100, 100);
+  }
+
+  @Test
+  public void getSize_withLayoutParams_notLaidOut_doesCallSizeReady() {
+    view.setLayoutParams(new FrameLayout.LayoutParams(10, 10));
+    view.setRight(100);
+    view.setBottom(100);
+    target.getSize(cb);
+
+    verify(cb, times(1)).onSizeReady(anyInt(), anyInt());
+  }
+
+  @Test
+  public void getSize_withLayoutParams_emptyParams_notLaidOutOrLayoutRequested_callsSizeReady() {
+    view.setLayoutParams(new FrameLayout.LayoutParams(0, 0));
+    view.setRight(100);
+    view.setBottom(100);
+    target.getSize(cb);
+
+    verify(cb).onSizeReady(100, 100);
+  }
+
+  @Test
+  public void getSize_withValidWidthAndHeight_preV19_layoutRequested_callsSizeReady() {
+    Util.setSdkVersionInt(18);
+    view.setLayoutParams(new FrameLayout.LayoutParams(100, 100));
+    view.requestLayout();
+
+    target.getSize(cb);
+
+    verify(cb).onSizeReady(100, 100);
+  }
+
+  @Test
+  public void getSize_withWidthAndHeightEqualToPadding_doesNotCallSizeReady() {
+    view.setLayoutParams(new FrameLayout.LayoutParams(100, 100));
+    view.requestLayout();
+    view.setPadding(50, 50, 50, 50);
+
+    target.getSize(cb);
+
+    verify(cb, never()).onSizeReady(anyInt(), anyInt());
+  }
+
+  private void setDisplayDimens(Integer width, Integer height) {
+    WindowManager windowManager =
+        (WindowManager) RuntimeEnvironment.application.getSystemService(Context.WINDOW_SERVICE);
+    ShadowDisplay shadowDisplay =
+        Shadows.shadowOf(Preconditions.checkNotNull(windowManager).getDefaultDisplay());
+    if (width != null) {
+      shadowDisplay.setWidth(width);
+    }
+
+    if (height != null) {
+      shadowDisplay.setHeight(height);
+    }
+  }
+
+  @Test
+  public void clearOnDetach_onDetach_withNullRequest_doesNothing() {
+    attachStateTarget.clearOnDetach();
+    attachStateTarget.setRequest(null);
+    activity.visible();
+  }
+
+  // This behavior isn't clearly correct, but it doesn't seem like there's any harm to clear an
+  // already cleared request, so we might as well avoid the extra check/complexity in the code.
+  @Test
+  public void clearOnDetach_onDetach_withClearedRequest_clearsRequest() {
+    activity.visible();
+    attachStateTarget.clearOnDetach();
+    attachStateTarget.setRequest(request);
+    when(request.isCleared()).thenReturn(true);
+    parent.removeView(view);
+
+    verify(request).clear();
+  }
+
+  @Test
+  public void clearOnDetach_onDetach_withRunningRequest_pausesRequestOnce() {
+    activity.visible();
+    attachStateTarget.clearOnDetach();
+    attachStateTarget.setRequest(request);
+    parent.removeView(view);
+
+    verify(request).clear();
+  }
+
+  @Test
+  public void clearOnDetach_onDetach_afterOnLoadCleared_removesListener() {
+    activity.visible();
+    attachStateTarget.clearOnDetach();
+    attachStateTarget.onLoadCleared(/*placeholder=*/ null);
+    attachStateTarget.setRequest(request);
+    parent.removeView(view);
+
+    verify(request, never()).clear();
+  }
+
+  @Test
+  public void clearOnDetach_moreThanOnce_registersObserverOnce() {
+    activity.visible();
+    attachStateTarget.setRequest(request);
+    attachStateTarget
+        .clearOnDetach()
+        .clearOnDetach();
+    parent.removeView(view);
+
+    verify(request).clear();
+  }
+
+  @Test
+  public void clearOnDetach_onDetach_afterMultipleClearOnDetaches_removesListener() {
+    activity.visible();
+    attachStateTarget
+        .clearOnDetach()
+        .clearOnDetach()
+        .clearOnDetach();
+    attachStateTarget.onLoadCleared(/*placeholder=*/ null);
+    attachStateTarget.setRequest(request);
+    parent.removeView(view);
+
+    verify(request, never()).clear();
+  }
+
+  @Test
+  public void clearOnDetach_onDetach_afterLoadCleared_clearsRequest() {
+    activity.visible();
+    attachStateTarget.clearOnDetach();
+    attachStateTarget.setRequest(request);
+    when(request.isCleared()).thenReturn(true);
+    parent.removeView(view);
+
+    verify(request).clear();
+  }
+
+  @Test
+  public void clearOnDetach_onAttach_withNullRequest_doesNothing() {
+    attachStateTarget.clearOnDetach();
+    attachStateTarget.setRequest(null);
+    activity.visible();
+  }
+
+  @Test
+  public void clearOnDetach_onAttach_withRunningRequest_doesNotBeginRequest() {
+    attachStateTarget.clearOnDetach();
+    attachStateTarget.setRequest(request);
+    when(request.isCleared()).thenReturn(false);
+    activity.visible();
+
+    verify(request, never()).begin();
+  }
+
+  @Test
+  public void clearOnDetach_onAttach_withClearedRequest_beginsRequest() {
+    attachStateTarget.clearOnDetach();
+    attachStateTarget.setRequest(request);
+    when(request.isCleared()).thenReturn(true);
+    activity.visible();
+
+    verify(request).begin();
+  }
+
+  @Test
+  public void clearOnDetach_afterLoadClearedAndRestarted_onAttach_beingsREquest() {
+    attachStateTarget.clearOnDetach();
+    attachStateTarget.setRequest(request);
+    when(request.isCleared()).thenReturn(true);
+    attachStateTarget.onLoadCleared(/*placeholder=*/ null);
+    attachStateTarget.onLoadStarted(/*placeholder=*/ null);
+    activity.visible();
+
+    verify(request).begin();
+  }
+
+  @Test
+  public void clearOnDetach_onAttach_afterLoadCleared_doesNotBeingRequest() {
+    attachStateTarget.clearOnDetach();
+    attachStateTarget.setRequest(request);
+    when(request.isCleared()).thenReturn(true);
+    attachStateTarget.onLoadCleared(/*placeholder=*/ null);
+    activity.visible();
+
+    verify(request, never()).begin();
+  }
+
+  @Test
+  public void onLoadStarted_withoutClearOnDetach_doesNotAddListener() {
+    activity.visible();
+    target.setRequest(request);
+    attachStateTarget.onLoadStarted(/*placeholder=*/ null);
+    parent.removeView(view);
+
+    verify(request, never()).clear();
+  }
+
+  @Test
+  public void onLoadCleared_withoutClearOnDetach_doesNotRemoveListeners() {
+    AtomicInteger count = new AtomicInteger();
+    OnAttachStateChangeListener expected =
+        new OnAttachStateChangeListener() {
+          @Override
+          public void onViewAttachedToWindow(View v) {
+            count.incrementAndGet();
+          }
+
+          @Override
+          public void onViewDetachedFromWindow(View v) {}
+        };
+    view.addOnAttachStateChangeListener(expected);
+
+    attachStateTarget.onLoadCleared(/*placeholder=*/ null);
+
+    activity.visible();
+
+    assertThat(count.get()).isEqualTo(1);
+  }
+
+  private static final class AttachStateTarget extends CustomViewTarget {
+    AttachStateTarget(View view) {
+      super(view);
+    }
+
+    @Override
+    protected void onResourceCleared(@Nullable Drawable placeholder) {}
+
+    @Override
+    public void onLoadFailed(@Nullable Drawable errorDrawable) {}
+
+    @Override
+    public void onResourceReady(
+        @NonNull Object resource, @Nullable Transition transition) {}
+  }
+
+  private static final class TestViewTarget extends CustomViewTarget {
+
+    TestViewTarget(View view) {
+      super(view);
+    }
+
+    @Override
+    protected void onResourceCleared(@Nullable Drawable placeholder) {}
+
+    // We're intentionally avoiding the super call.
+    @SuppressWarnings("MissingSuperCall")
+    @Override
+    public void onResourceReady(
+        @NonNull Object resource, @Nullable Transition transition) {
+      // Avoid calling super.
+    }
+
+    // We're intentionally avoiding the super call.
+    @SuppressWarnings("MissingSuperCall")
+    @Override
+    public void onResourceLoading(@Nullable Drawable placeholder) {
+      // Avoid calling super.
+    }
+
+    // We're intentionally avoiding the super call.
+    @SuppressWarnings("MissingSuperCall")
+    @Override
+    public void onLoadFailed(@Nullable Drawable errorDrawable) {
+      // Avoid calling super.
+    }
+  }
+}
diff --git a/library/test/src/test/java/com/bumptech/glide/request/target/ViewTargetTest.java b/library/test/src/test/java/com/bumptech/glide/request/target/ViewTargetTest.java
index 0ea32764e..362746fb5 100644
--- a/library/test/src/test/java/com/bumptech/glide/request/target/ViewTargetTest.java
+++ b/library/test/src/test/java/com/bumptech/glide/request/target/ViewTargetTest.java
@@ -687,7 +687,7 @@ public class ViewTargetTest {
     private LayoutParams layoutParams;
     private boolean isLaidOut;
     private boolean isLayoutRequested;
-    private final Set attachStateListeners = new HashSet<>();
+    final Set attachStateListeners = new HashSet<>();
 
     public SizedShadowView setWidth(int width) {
       this.width = width;
-- 
GitLab