Skip to content

Commit

Permalink
Stop GifDrawable in onFrameReady if all callbacks are Drawables.
Browse files Browse the repository at this point in the history
When we use TransitionDrawable, the GifDrawable’s callback is always
non-null and always the TransitionDrawable, even if the
TransitionDrawable was removed from the view hierarchy. Checking up
the callback chain allows us to ensure that there’s a non-Drawable 
callback eventually.
  • Loading branch information
sjudd committed Nov 19, 2017
1 parent bcd6cc2 commit 9d87dea
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -327,9 +327,18 @@ public int getOpacity() {
return PixelFormat.TRANSPARENT;
}

// See #1087.
private Callback findCallback() {
Callback callback = getCallback();
while (callback instanceof Drawable) {
callback = ((Drawable) callback).getCallback();
}
return callback;
}

@Override
public void onFrameReady() {
if (getCallback() == null) {
if (findCallback() == null) {
stop();
invalidateSelf();
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import android.app.Application;
import android.graphics.Bitmap;
import android.graphics.Canvas;
import android.graphics.Color;
Expand All @@ -24,10 +25,11 @@
import android.graphics.PorterDuffColorFilter;
import android.graphics.Rect;
import android.graphics.drawable.Drawable;
import android.graphics.drawable.TransitionDrawable;
import android.os.Build;
import android.view.View;
import com.bumptech.glide.gifdecoder.GifDecoder;
import com.bumptech.glide.load.Transformation;
import com.bumptech.glide.load.engine.bitmap_recycle.BitmapPool;
import com.bumptech.glide.load.resource.gif.GifDrawableTest.BitmapTrackingShadowCanvas;
import com.bumptech.glide.tests.GlideShadowLooper;
import com.bumptech.glide.tests.Util;
Expand Down Expand Up @@ -59,10 +61,10 @@ public class GifDrawableTest {
private int initialSdkVersion;

@Mock private Drawable.Callback cb;
@Mock private BitmapPool bitmapPool;
@Mock private GifFrameLoader frameLoader;
@Mock private Paint paint;
@Mock private Transformation<Bitmap> transformation;
private Application context;

private static Paint isAPaint() {
return isA(Paint.class);
Expand All @@ -75,6 +77,7 @@ private static Rect isARect() {
@Before
public void setUp() {
MockitoAnnotations.initMocks(this);
context = RuntimeEnvironment.application;
frameWidth = 120;
frameHeight = 450;
firstFrame = Bitmap.createBitmap(frameWidth, frameHeight, Bitmap.Config.RGB_565);
Expand Down Expand Up @@ -574,6 +577,30 @@ public void testThrowsIfCreatedWithNullState() {
new GifDrawable(null);
}

@Test
public void onFrameReady_whenAttachedToDrawableCallbackButNotViewCallback_stops() {
TransitionDrawable topLevel = new TransitionDrawable(new Drawable[] { drawable });
drawable.setCallback(topLevel);
topLevel.setCallback(null);

drawable.start();
drawable.onFrameReady();

assertThat(drawable.isRunning()).isFalse();
}

@Test
public void onFrameReady_whenAttachedtoDrawableCallbackWithViewCallbackParent_doesNotStop() {
TransitionDrawable topLevel = new TransitionDrawable(new Drawable[] { drawable });
drawable.setCallback(topLevel);
topLevel.setCallback(new View(context));

drawable.start();
drawable.onFrameReady();

assertThat(drawable.isRunning()).isTrue();
}

private void verifyRanLoops(int loopCount, int frameCount) {
// 1 for invalidate in start().
verify(cb, times(1 + loopCount * frameCount)).invalidateDrawable(eq(drawable));
Expand Down

0 comments on commit 9d87dea

Please sign in to comment.