diff --git a/instrumentation/src/androidTest/java/com/bumptech/glide/MultiRequestTest.java b/instrumentation/src/androidTest/java/com/bumptech/glide/MultiRequestTest.java new file mode 100644 index 0000000000..b04cc220fe --- /dev/null +++ b/instrumentation/src/androidTest/java/com/bumptech/glide/MultiRequestTest.java @@ -0,0 +1,163 @@ +package com.bumptech.glide; + +import static com.google.common.truth.Truth.assertThat; + +import android.content.Context; +import android.graphics.Bitmap; +import android.graphics.Bitmap.CompressFormat; +import android.graphics.Bitmap.Config; +import android.graphics.Canvas; +import android.graphics.Color; +import android.graphics.drawable.Drawable; +import androidx.annotation.NonNull; +import androidx.annotation.Nullable; +import androidx.test.core.app.ApplicationProvider; +import androidx.test.ext.junit.runners.AndroidJUnit4; +import com.bumptech.glide.load.DataSource; +import com.bumptech.glide.load.engine.GlideException; +import com.bumptech.glide.load.engine.executor.GlideExecutor; +import com.bumptech.glide.request.Request; +import com.bumptech.glide.request.RequestListener; +import com.bumptech.glide.request.target.CustomTarget; +import com.bumptech.glide.request.target.Target; +import com.bumptech.glide.request.transition.Transition; +import com.bumptech.glide.test.ModelGeneratorRule; +import com.bumptech.glide.testutil.ConcurrencyHelper; +import com.bumptech.glide.testutil.TearDownGlide; +import java.io.BufferedOutputStream; +import java.io.File; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.OutputStream; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.junit.runner.RunWith; + +@RunWith(AndroidJUnit4.class) +public class MultiRequestTest { + private final Context context = ApplicationProvider.getApplicationContext(); + @Rule public final TearDownGlide tearDownGlide = new TearDownGlide(); + @Rule public final ModelGeneratorRule modelGeneratorRule = new ModelGeneratorRule(); + @Rule public final TemporaryFolder temporaryFolder = new TemporaryFolder(); + private final ConcurrencyHelper concurrency = new ConcurrencyHelper(); + + @Test + public void thumbnail_onResourceReady_forPrimary_isComplete_whenRequestListenerIsCalled() + throws IOException, InterruptedException { + + // Make sure the requests complete in the same order + Glide.init( + context, + new GlideBuilder() + .setSourceExecutor(GlideExecutor.newSourceBuilder().setThreadCount(1).build())); + + AtomicBoolean isPrimaryRequestComplete = new AtomicBoolean(false); + CountDownLatch countDownLatch = new CountDownLatch(1); + + RequestBuilder request = + Glide.with(context) + .load(newImageFile()) + .thumbnail(Glide.with(context).load(newImageFile())) + .listener( + new RequestListener<>() { + @Override + public boolean onLoadFailed( + @Nullable GlideException e, + Object model, + Target target, + boolean isFirstResource) { + return false; + } + + @Override + public boolean onResourceReady( + Drawable resource, + Object model, + Target target, + DataSource dataSource, + boolean isFirstResource) { + isPrimaryRequestComplete.set(target.getRequest().isComplete()); + countDownLatch.countDown(); + return false; + } + }); + concurrency.runOnMainThread(() -> request.into(new DoNothingTarget())); + + assertThat(countDownLatch.await(3, TimeUnit.SECONDS)).isTrue(); + assertThat(isPrimaryRequestComplete.get()).isTrue(); + } + + @Test + public void thumbnail_onLoadFailed_forPrimary_isNotRunningOrComplete_whenRequestListenerIsCalled() + throws IOException, InterruptedException { + + // Make sure the requests complete in the same order + Glide.init( + context, + new GlideBuilder() + .setSourceExecutor(GlideExecutor.newSourceBuilder().setThreadCount(1).build())); + + AtomicBoolean isNeitherRunningNorComplete = new AtomicBoolean(false); + CountDownLatch countDownLatch = new CountDownLatch(1); + + int missingResourceId = 123; + RequestBuilder requestBuilder = + Glide.with(context) + .load(missingResourceId) + .thumbnail(Glide.with(context).load(newImageFile())) + .listener( + new RequestListener<>() { + @Override + public boolean onLoadFailed( + @Nullable GlideException e, + Object model, + Target target, + boolean isFirstResource) { + Request request = target.getRequest(); + isNeitherRunningNorComplete.set(!request.isComplete() && !request.isRunning()); + countDownLatch.countDown(); + return false; + } + + @Override + public boolean onResourceReady( + Drawable resource, + Object model, + Target target, + DataSource dataSource, + boolean isFirstResource) { + return false; + } + }); + concurrency.runOnMainThread(() -> requestBuilder.into(new DoNothingTarget())); + + assertThat(countDownLatch.await(3, TimeUnit.SECONDS)).isTrue(); + assertThat(isNeitherRunningNorComplete.get()).isTrue(); + } + + private File newImageFile() throws IOException { + Bitmap bitmap = Bitmap.createBitmap(100, 100, Config.ARGB_8888); + Canvas canvas = new Canvas(bitmap); + canvas.drawColor(Color.RED); + File result = temporaryFolder.newFile(); + try (OutputStream os = new BufferedOutputStream(new FileOutputStream(result))) { + bitmap.compress(CompressFormat.JPEG, 75, os); + } + return result; + } + + // We don't store or do anything with the resource, so we don't need to do anything to release it + // in onLoadCleared. + private static final class DoNothingTarget extends CustomTarget { + @Override + public void onResourceReady( + @NonNull Drawable resource, @Nullable Transition transition) {} + + @Override + public void onLoadCleared(@Nullable Drawable placeholder) {} + } +} diff --git a/library/src/main/java/com/bumptech/glide/request/SingleRequest.java b/library/src/main/java/com/bumptech/glide/request/SingleRequest.java index e59bd259d7..839a4bd778 100644 --- a/library/src/main/java/com/bumptech/glide/request/SingleRequest.java +++ b/library/src/main/java/com/bumptech/glide/request/SingleRequest.java @@ -523,14 +523,14 @@ private boolean isFirstReadyResource() { } @GuardedBy("requestLock") - private void notifyLoadSuccess() { + private void notifyRequestCoordinatorLoadSucceeded() { if (requestCoordinator != null) { requestCoordinator.onRequestSuccess(this); } } @GuardedBy("requestLock") - private void notifyLoadFailed() { + private void notifyRequestCoordinatorLoadFailed() { if (requestCoordinator != null) { requestCoordinator.onRequestFailed(this); } @@ -639,6 +639,8 @@ private void onResourceReady( + " ms"); } + notifyRequestCoordinatorLoadSucceeded(); + isCallingCallbacks = true; try { boolean anyListenerHandledUpdatingTarget = false; @@ -660,7 +662,6 @@ private void onResourceReady( isCallingCallbacks = false; } - notifyLoadSuccess(); GlideTrace.endSectionAsync(TAG, cookie); } @@ -694,6 +695,8 @@ private void onLoadFailed(GlideException e, int maxLogLevel) { loadStatus = null; status = Status.FAILED; + notifyRequestCoordinatorLoadFailed(); + isCallingCallbacks = true; try { // TODO: what if this is a thumbnail request? @@ -715,7 +718,6 @@ private void onLoadFailed(GlideException e, int maxLogLevel) { isCallingCallbacks = false; } - notifyLoadFailed(); GlideTrace.endSectionAsync(TAG, cookie); } } diff --git a/library/test/src/test/java/com/bumptech/glide/request/SingleRequestTest.java b/library/test/src/test/java/com/bumptech/glide/request/SingleRequestTest.java index 123f38bcb2..16c8169e35 100644 --- a/library/test/src/test/java/com/bumptech/glide/request/SingleRequestTest.java +++ b/library/test/src/test/java/com/bumptech/glide/request/SingleRequestTest.java @@ -33,6 +33,7 @@ import com.bumptech.glide.load.engine.Engine; import com.bumptech.glide.load.engine.GlideException; import com.bumptech.glide.load.engine.Resource; +import com.bumptech.glide.request.target.CustomTarget; import com.bumptech.glide.request.target.SizeReadyCallback; import com.bumptech.glide.request.target.Target; import com.bumptech.glide.request.transition.Transition; @@ -46,6 +47,7 @@ import java.util.List; import java.util.Map; import java.util.concurrent.Executor; +import java.util.concurrent.atomic.AtomicBoolean; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -649,6 +651,89 @@ public void testRequestListenerIsCalledWithFirstImageIfRequestCoordinatorReturns eq(builder.result), any(Number.class), isAListTarget(), isADataSource(), eq(false)); } + @Test + public void onResourceReady_notifiesRequestCoordinator_beforeCallingRequestListeners() { + AtomicBoolean isRequestCoordinatorVerified = new AtomicBoolean(); + SingleRequest request = + builder + .setTarget(new DoNothingTarget()) + .addRequestListener( + new RequestListener<>() { + @Override + public boolean onLoadFailed( + @Nullable GlideException e, + Object model, + Target target, + boolean isFirstResource) { + return false; + } + + @Override + public boolean onResourceReady( + List resource, + Object model, + Target target, + DataSource dataSource, + boolean isFirstResource) { + verify(builder.requestCoordinator).onRequestSuccess(target.getRequest()); + isRequestCoordinatorVerified.set(true); + return false; + } + }) + .build(); + builder.target.setRequest(request); + request.onResourceReady( + builder.resource, DataSource.DATA_DISK_CACHE, /* isLoadedFromAlternateCacheKey= */ false); + + assertThat(isRequestCoordinatorVerified.get()).isTrue(); + } + + @Test + public void onLoadFailed_notifiesRequestCoordinator_beforeCallingRequestListeners() { + AtomicBoolean isRequestCoordinatorVerified = new AtomicBoolean(); + SingleRequest request = + builder + .setTarget(new DoNothingTarget()) + .addRequestListener( + new RequestListener<>() { + @Override + public boolean onLoadFailed( + @Nullable GlideException e, + Object model, + Target target, + boolean isFirstResource) { + verify(builder.requestCoordinator).onRequestFailed(target.getRequest()); + isRequestCoordinatorVerified.set(true); + return false; + } + + @Override + public boolean onResourceReady( + List resource, + Object model, + Target target, + DataSource dataSource, + boolean isFirstResource) { + return false; + } + }) + .build(); + builder.target.setRequest(request); + request.onLoadFailed(new GlideException("test")); + + assertThat(isRequestCoordinatorVerified.get()).isTrue(); + } + + // We don't need to clear a resource since we're not using it to being with. + private static final class DoNothingTarget extends CustomTarget { + @Override + public void onResourceReady( + @NonNull List resource, @Nullable Transition transition) {} + + @Override + public void onLoadCleared(@Nullable Drawable placeholder) {} + } + @Test public void testRequestListenerIsCalledWithNotIsFirstRequestIfRequestCoordinatorParentReturnsResourceSet() {