Skip to content

Commit

Permalink
Avoid cancelling preload requests until the primary request completes.
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 369340778
  • Loading branch information
sjudd authored and glide-copybara-robot committed Apr 20, 2021
1 parent a7b254c commit 4733d1d
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 7 deletions.
7 changes: 7 additions & 0 deletions library/src/main/java/com/bumptech/glide/RequestBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -859,6 +859,13 @@ public FutureTarget<TranscodeType> submit(int width, int height) {
* <p>Pre-loading is useful for making sure that resources you are going to to want in the near
* future are available quickly.
*
* <p>Note - Any thumbnail request that does not complete before the primary request will be
* cancelled and may not be preloaded successfully. Cancellation of outstanding thumbnails after
* the primary request succeeds is a common behavior of all Glide requests. We do not try to
* prevent that behavior here. If you absolutely need all thumbnails to be preloaded individually,
* make separate preload() requests for each thumbnail (you can still combine them into one call
* when loading the image(s) into the UI in a subsequent request).
*
* @param width The desired width in pixels, or {@link Target#SIZE_ORIGINAL}. This will be
* overridden by {@link com.bumptech.glide.request.RequestOptions#override(int, int)} if
* previously called.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import com.bumptech.glide.RequestManager;
import com.bumptech.glide.request.Request;
import com.bumptech.glide.request.transition.Transition;
import com.bumptech.glide.util.Synthetic;

Expand Down Expand Up @@ -53,7 +54,17 @@ private PreloadTarget(RequestManager requestManager, int width, int height) {

@Override
public void onResourceReady(@NonNull Z resource, @Nullable Transition<? super Z> transition) {
HANDLER.obtainMessage(MESSAGE_CLEAR, this).sendToTarget();
// If a thumbnail request is set and the thumbnail completes, we don't want to cancel the
// primary load. Instead we wait until the primary request (the one set on the target) says
// that it is complete.
// Note - Any thumbnail request that does not complete before the primary request will be
// cancelled and may not be preloaded successfully. Cancellation of outstanding thumbnails after
// the primary request succeeds is a common behavior of all Glide requests and we're not trying
// to override it here.
Request request = getRequest();
if (request != null && request.isComplete()) {
HANDLER.obtainMessage(MESSAGE_CLEAR, this).sendToTarget();
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@

import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.robolectric.annotation.LooperMode.Mode.LEGACY;
import static org.mockito.Mockito.when;
import static org.robolectric.Shadows.shadowOf;

import android.os.Looper;
import com.bumptech.glide.RequestManager;
import com.bumptech.glide.request.Request;
import org.junit.Before;
Expand All @@ -14,9 +17,7 @@
import org.mockito.MockitoAnnotations;
import org.robolectric.RobolectricTestRunner;
import org.robolectric.annotation.Config;
import org.robolectric.annotation.LooperMode;

@LooperMode(LEGACY)
@RunWith(RobolectricTestRunner.class)
@Config(sdk = 18)
public class PreloadTargetTest {
Expand All @@ -26,6 +27,7 @@ public class PreloadTargetTest {
@Before
public void setUp() {
MockitoAnnotations.initMocks(this);
shadowOf(Looper.getMainLooper()).pause();
}

@Test
Expand All @@ -39,13 +41,58 @@ public void testCallsSizeReadyWithGivenDimensions() {
verify(cb).onSizeReady(eq(width), eq(height));
}

// This isn't really supposed to happen, but just to double check...
@Test
public void testClearsTargetInOnResourceReady() {
public void onResourceReady_withNullRequest_doesNotClearTarget() {
PreloadTarget<Object> target = PreloadTarget.obtain(requestManager, 100, 100);
target.setRequest(null);

callOnResourceReadyAndRunUiRunnables(target);

verify(requestManager, never()).clear(target);
}

@Test
public void onResourceReady_withNotYetCompleteRequest_doesNotClearTarget() {
Request request = mock(Request.class);
when(request.isComplete()).thenReturn(false);

PreloadTarget<Object> target = PreloadTarget.obtain(requestManager, 100, 100);
target.setRequest(request);

callOnResourceReadyAndRunUiRunnables(target);

verify(requestManager, never()).clear(target);
}

@Test
public void onResourceReady_withCompleteRequest_postsToClearTarget() {
Request request = mock(Request.class);
when(request.isComplete()).thenReturn(true);

PreloadTarget<Object> target = PreloadTarget.obtain(requestManager, 100, 100);
target.setRequest(request);

callOnResourceReadyAndRunUiRunnables(target);

verify(requestManager).clear(target);
}

@Test
public void onResourceReady_withCompleteRequest_doesNotImmediatelyClearTarget() {
Request request = mock(Request.class);
when(request.isComplete()).thenReturn(true);

PreloadTarget<Object> target = PreloadTarget.obtain(requestManager, 100, 100);
target.setRequest(request);
target.onResourceReady(new Object(), null);

verify(requestManager).clear(eq(target));
target.onResourceReady(new Object(), /* transition= */ null);

verify(requestManager, never()).clear(target);
}

private void callOnResourceReadyAndRunUiRunnables(Target<Object> target) {
target.onResourceReady(new Object(), /* transition= */ null);
shadowOf(Looper.getMainLooper()).idle();
}
}

0 comments on commit 4733d1d

Please sign in to comment.