提交 65048a4a 编写于 作者: S Sam Judd

Pass through exception messages to RequestFutureTarget.

We may want to more generically handle multiple RequestListeners in the
future, but for now we can at least avoid swallowing exceptions in 
RequestFutureTarget without allocating new Lists every time a RequestListener is added.
上级 cb1276d5
......@@ -476,10 +476,19 @@ public class RequestBuilder<TranscodeType> implements Cloneable {
* @see RequestManager#clear(Target)
*/
public <Y extends Target<TranscodeType>> Y into(@NonNull Y target) {
return into(target, getMutableOptions());
return into(target, /*targetListener=*/ null);
}
private <Y extends Target<TranscodeType>> Y into(@NonNull Y target, RequestOptions options) {
private <Y extends Target<TranscodeType>> Y into(
@NonNull Y target,
@Nullable RequestListener<TranscodeType> targetListener) {
return into(target, targetListener, getMutableOptions());
}
private <Y extends Target<TranscodeType>> Y into(
@NonNull Y target,
@Nullable RequestListener<TranscodeType> targetListener,
RequestOptions options) {
Util.assertMainThread();
Preconditions.checkNotNull(target);
if (!isModelSet) {
......@@ -487,7 +496,7 @@ public class RequestBuilder<TranscodeType> implements Cloneable {
}
options = options.autoClone();
Request request = buildRequest(target, options);
Request request = buildRequest(target, targetListener, options);
Request previous = target.getRequest();
if (request.isEquivalentTo(previous)) {
......@@ -557,7 +566,10 @@ public class RequestBuilder<TranscodeType> implements Cloneable {
}
}
return into(glideContext.buildImageViewTarget(view, transcodeClass), requestOptions);
return into(
glideContext.buildImageViewTarget(view, transcodeClass),
/*targetListener=*/ null,
requestOptions);
}
/**
......@@ -617,12 +629,12 @@ public class RequestBuilder<TranscodeType> implements Cloneable {
@Override
public void run() {
if (!target.isCancelled()) {
into(target);
into(target, target);
}
}
});
} else {
into(target);
into(target, target);
}
return target;
......@@ -717,15 +729,30 @@ public class RequestBuilder<TranscodeType> implements Cloneable {
}
}
private Request buildRequest(Target<TranscodeType> target, RequestOptions requestOptions) {
return buildRequestRecursive(target, null, transitionOptions, requestOptions.getPriority(),
requestOptions.getOverrideWidth(), requestOptions.getOverrideHeight(), requestOptions);
}
private Request buildRequestRecursive(Target<TranscodeType> target,
private Request buildRequest(
Target<TranscodeType> target,
@Nullable RequestListener<TranscodeType> targetListener,
RequestOptions requestOptions) {
return buildRequestRecursive(
target,
targetListener,
/*requestCoordinator=*/ null,
transitionOptions,
requestOptions.getPriority(),
requestOptions.getOverrideWidth(),
requestOptions.getOverrideHeight(),
requestOptions);
}
private Request buildRequestRecursive(
Target<TranscodeType> target,
@Nullable RequestListener<TranscodeType> targetListener,
@Nullable RequestCoordinator parentCoordinator,
TransitionOptions<?, ? super TranscodeType> transitionOptions,
Priority priority, int overrideWidth, int overrideHeight, RequestOptions requestOptions) {
Priority priority,
int overrideWidth,
int overrideHeight,
RequestOptions requestOptions) {
// Build the ErrorRequestCoordinator first if necessary so we can update parentCoordinator.
ErrorRequestCoordinator errorRequestCoordinator = null;
......@@ -737,6 +764,7 @@ public class RequestBuilder<TranscodeType> implements Cloneable {
Request mainRequest =
buildThumbnailRequestRecursive(
target,
targetListener,
parentCoordinator,
transitionOptions,
priority,
......@@ -758,6 +786,7 @@ public class RequestBuilder<TranscodeType> implements Cloneable {
Request errorRequest = errorBuilder.buildRequestRecursive(
target,
targetListener,
errorRequestCoordinator,
errorBuilder.transitionOptions,
errorBuilder.requestOptions.getPriority(),
......@@ -768,10 +797,15 @@ public class RequestBuilder<TranscodeType> implements Cloneable {
return errorRequestCoordinator;
}
private Request buildThumbnailRequestRecursive(Target<TranscodeType> target,
@Nullable RequestCoordinator parentCoordinator,
TransitionOptions<?, ? super TranscodeType> transitionOptions,
Priority priority, int overrideWidth, int overrideHeight, RequestOptions requestOptions) {
private Request buildThumbnailRequestRecursive(
Target<TranscodeType> target,
RequestListener<TranscodeType> targetListener,
@Nullable RequestCoordinator parentCoordinator,
TransitionOptions<?, ? super TranscodeType> transitionOptions,
Priority priority,
int overrideWidth,
int overrideHeight,
RequestOptions requestOptions) {
if (thumbnailBuilder != null) {
// Recursive case: contains a potentially recursive thumbnail request builder.
if (isThumbnailBuilt) {
......@@ -800,13 +834,22 @@ public class RequestBuilder<TranscodeType> implements Cloneable {
}
ThumbnailRequestCoordinator coordinator = new ThumbnailRequestCoordinator(parentCoordinator);
Request fullRequest = obtainRequest(target, requestOptions, coordinator,
transitionOptions, priority, overrideWidth, overrideHeight);
Request fullRequest =
obtainRequest(
target,
targetListener,
requestOptions,
coordinator,
transitionOptions,
priority,
overrideWidth,
overrideHeight);
isThumbnailBuilt = true;
// Recursively generate thumbnail requests.
Request thumbRequest =
thumbnailBuilder.buildRequestRecursive(
target,
targetListener,
coordinator,
thumbTransitionOptions,
thumbPriority,
......@@ -819,27 +862,55 @@ public class RequestBuilder<TranscodeType> implements Cloneable {
} else if (thumbSizeMultiplier != null) {
// Base case: thumbnail multiplier generates a thumbnail request, but cannot recurse.
ThumbnailRequestCoordinator coordinator = new ThumbnailRequestCoordinator(parentCoordinator);
Request fullRequest = obtainRequest(target, requestOptions, coordinator, transitionOptions,
priority, overrideWidth, overrideHeight);
Request fullRequest =
obtainRequest(
target,
targetListener,
requestOptions,
coordinator,
transitionOptions,
priority,
overrideWidth,
overrideHeight);
RequestOptions thumbnailOptions = requestOptions.clone()
.sizeMultiplier(thumbSizeMultiplier);
Request thumbnailRequest = obtainRequest(target, thumbnailOptions, coordinator,
transitionOptions, getThumbnailPriority(priority), overrideWidth, overrideHeight);
Request thumbnailRequest =
obtainRequest(
target,
targetListener,
thumbnailOptions,
coordinator,
transitionOptions,
getThumbnailPriority(priority),
overrideWidth,
overrideHeight);
coordinator.setRequests(fullRequest, thumbnailRequest);
return coordinator;
} else {
// Base case: no thumbnail.
return obtainRequest(target, requestOptions, parentCoordinator, transitionOptions, priority,
overrideWidth, overrideHeight);
return obtainRequest(
target,
targetListener,
requestOptions,
parentCoordinator,
transitionOptions,
priority,
overrideWidth,
overrideHeight);
}
}
private Request obtainRequest(Target<TranscodeType> target,
RequestOptions requestOptions, RequestCoordinator requestCoordinator,
TransitionOptions<?, ? super TranscodeType> transitionOptions, Priority priority,
int overrideWidth, int overrideHeight) {
private Request obtainRequest(
Target<TranscodeType> target,
RequestListener<TranscodeType> targetListener,
RequestOptions requestOptions,
RequestCoordinator requestCoordinator,
TransitionOptions<?, ? super TranscodeType> transitionOptions,
Priority priority,
int overrideWidth,
int overrideHeight) {
return SingleRequest.obtain(
context,
glideContext,
......@@ -850,6 +921,7 @@ public class RequestBuilder<TranscodeType> implements Cloneable {
overrideHeight,
priority,
target,
targetListener,
requestListener,
requestCoordinator,
glideContext.getEngine(),
......
......@@ -235,6 +235,7 @@ class DecodeJob<R> implements DataFetcherGenerator.FetcherReadyCallback,
}
// When we're encoding we've already notified our callback and it isn't safe to do so again.
if (stage != Stage.ENCODE) {
exceptions.add(e);
notifyFailed();
}
if (!isCancelled) {
......
......@@ -3,9 +3,14 @@ package com.bumptech.glide.request;
import android.graphics.drawable.Drawable;
import android.os.Handler;
import android.support.annotation.Nullable;
import com.bumptech.glide.load.DataSource;
import com.bumptech.glide.load.engine.GlideException;
import com.bumptech.glide.request.target.SizeReadyCallback;
import com.bumptech.glide.request.target.Target;
import com.bumptech.glide.request.transition.Transition;
import com.bumptech.glide.util.Util;
import java.io.PrintStream;
import java.io.PrintWriter;
import java.util.concurrent.CancellationException;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
......@@ -47,6 +52,7 @@ import java.util.concurrent.TimeoutException;
* @param <R> The type of the resource that will be loaded.
*/
public class RequestFutureTarget<R> implements FutureTarget<R>,
RequestListener<R>,
Runnable {
private static final Waiter DEFAULT_WAITER = new Waiter();
......@@ -62,6 +68,7 @@ public class RequestFutureTarget<R> implements FutureTarget<R>,
private boolean isCancelled;
private boolean resultReceived;
private boolean loadFailed;
@Nullable private GlideException exception;
/**
* Constructor for a RequestFutureTarget. Should not be used directly.
......@@ -162,8 +169,7 @@ public class RequestFutureTarget<R> implements FutureTarget<R>,
*/
@Override
public synchronized void onLoadFailed(Drawable errorDrawable) {
loadFailed = true;
waiter.notifyAll(this);
// Ignored, synchronized for backwards compatibility.
}
/**
......@@ -171,10 +177,7 @@ public class RequestFutureTarget<R> implements FutureTarget<R>,
*/
@Override
public synchronized void onResourceReady(R resource, Transition<? super R> transition) {
// We might get a null result.
resultReceived = true;
this.resource = resource;
waiter.notifyAll(this);
// Ignored, synchronized for backwards compatibility.
}
private synchronized R doGet(Long timeoutMillis)
......@@ -186,7 +189,7 @@ public class RequestFutureTarget<R> implements FutureTarget<R>,
if (isCancelled) {
throw new CancellationException();
} else if (loadFailed) {
throw new ExecutionException(new IllegalStateException("Load failed"));
throw new ExecutionException(exception);
} else if (resultReceived) {
return resource;
}
......@@ -200,7 +203,7 @@ public class RequestFutureTarget<R> implements FutureTarget<R>,
if (Thread.interrupted()) {
throw new InterruptedException();
} else if (loadFailed) {
throw new ExecutionException(new IllegalStateException("Load failed"));
throw new GlideExecutionException(exception);
} else if (isCancelled) {
throw new CancellationException();
} else if (!resultReceived) {
......@@ -240,6 +243,25 @@ public class RequestFutureTarget<R> implements FutureTarget<R>,
// Do nothing.
}
@Override
public synchronized boolean onLoadFailed(
@Nullable GlideException e, Object model, Target<R> target, boolean isFirstResource) {
loadFailed = true;
exception = e;
waiter.notifyAll(this);
return false;
}
@Override
public synchronized boolean onResourceReady(
R resource, Object model, Target<R> target, DataSource dataSource, boolean isFirstResource) {
// We might get a null result.
resultReceived = true;
this.resource = resource;
waiter.notifyAll(this);
return false;
}
// Visible for testing.
static class Waiter {
......@@ -251,4 +273,33 @@ public class RequestFutureTarget<R> implements FutureTarget<R>,
toNotify.notifyAll();
}
}
private static class GlideExecutionException extends ExecutionException {
private final GlideException cause;
GlideExecutionException(GlideException cause) {
super();
this.cause = cause;
}
@Override
public void printStackTrace() {
printStackTrace(System.err);
}
@Override
public void printStackTrace(PrintStream s) {
super.printStackTrace(s);
s.print("Caused by: ");
cause.printStackTrace(s);
}
@Override
public void printStackTrace(PrintWriter s) {
super.printStackTrace(s);
s.print("Caused by: ");
cause.printStackTrace(s);
}
}
}
......@@ -85,6 +85,8 @@ public final class SingleRequest<R> implements Request,
private final String tag = String.valueOf(super.hashCode());
private final StateVerifier stateVerifier = StateVerifier.newInstance();
@Nullable
private RequestListener<R> targetListener;
private RequestCoordinator requestCoordinator;
private Context context;
private GlideContext glideContext;
......@@ -119,6 +121,7 @@ public final class SingleRequest<R> implements Request,
int overrideHeight,
Priority priority,
Target<R> target,
RequestListener<R> targetListener,
RequestListener<R> requestListener,
RequestCoordinator requestCoordinator,
Engine engine,
......@@ -138,6 +141,7 @@ public final class SingleRequest<R> implements Request,
overrideHeight,
priority,
target,
targetListener,
requestListener,
requestCoordinator,
engine,
......@@ -160,6 +164,7 @@ public final class SingleRequest<R> implements Request,
int overrideHeight,
Priority priority,
Target<R> target,
RequestListener<R> targetListener,
RequestListener<R> requestListener,
RequestCoordinator requestCoordinator,
Engine engine,
......@@ -173,6 +178,7 @@ public final class SingleRequest<R> implements Request,
this.overrideHeight = overrideHeight;
this.priority = priority;
this.target = target;
this.targetListener = targetListener;
this.requestListener = requestListener;
this.requestCoordinator = requestCoordinator;
this.engine = engine;
......@@ -197,6 +203,7 @@ public final class SingleRequest<R> implements Request,
overrideHeight = -1;
target = null;
requestListener = null;
targetListener = null;
requestCoordinator = null;
animationFactory = null;
loadStatus = null;
......@@ -544,8 +551,10 @@ public final class SingleRequest<R> implements Request,
isCallingCallbacks = true;
try {
if (requestListener == null
|| !requestListener.onResourceReady(result, model, target, dataSource, isFirstResource)) {
if ((requestListener == null
|| !requestListener.onResourceReady(result, model, target, dataSource, isFirstResource))
&& (targetListener == null
|| !targetListener.onResourceReady(result, model, target, dataSource, isFirstResource))) {
Transition<? super R> animation =
animationFactory.build(dataSource, isFirstResource);
target.onResourceReady(result, animation);
......@@ -581,8 +590,10 @@ public final class SingleRequest<R> implements Request,
isCallingCallbacks = true;
try {
//TODO: what if this is a thumbnail request?
if (requestListener == null
|| !requestListener.onLoadFailed(e, model, target, isFirstReadyResource())) {
if ((requestListener == null
|| !requestListener.onLoadFailed(e, model, target, isFirstReadyResource()))
&& (targetListener == null
|| !targetListener.onLoadFailed(e, model, target, isFirstReadyResource()))) {
setErrorPlaceholder();
}
} finally {
......
......@@ -13,6 +13,7 @@ import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import android.os.Handler;
import com.bumptech.glide.load.DataSource;
import com.bumptech.glide.request.target.SizeReadyCallback;
import java.util.concurrent.CancellationException;
import java.util.concurrent.ExecutionException;
......@@ -61,7 +62,12 @@ public class RequestFutureTargetTest {
@Test
public void testReturnsTrueFromIsDoneIfDone() {
future.onResourceReady(new Object(), null);
future.onResourceReady(
/*resource=*/ new Object(),
/*model=*/ null,
/*target=*/future,
DataSource.DATA_DISK_CACHE,
true /*isFirstResource*/);
assertTrue(future.isDone());
}
......@@ -106,7 +112,12 @@ public class RequestFutureTargetTest {
@Test
public void testDoesNotClearRequestIfCancelledAfterDone() {
future.onResourceReady(new Object(), null);
future.onResourceReady(
/*resource=*/ new Object(),
/*model=*/ null,
/*target=*/future,
DataSource.DATA_DISK_CACHE,
true /*isFirstResource*/);
future.cancel(true);
verify(request, never()).clear();
......@@ -120,7 +131,12 @@ public class RequestFutureTargetTest {
@Test
public void testReturnsFalseFromIsCancelledIfCancelledAfterDone() {
future.onResourceReady(new Object(), null);
future.onResourceReady(
/*resource=*/ new Object(),
/*model=*/ null,
/*target=*/future,
DataSource.DATA_DISK_CACHE,
true /*isFirstResource*/);
future.cancel(true);
assertFalse(future.isCancelled());
......@@ -134,7 +150,12 @@ public class RequestFutureTargetTest {
@Test
public void testReturnsFalseFromCancelIfDone() {
future.onResourceReady(new Object(), null);
future.onResourceReady(
/*resource=*/ new Object(),
/*model=*/ null,
/*target=*/future,
DataSource.DATA_DISK_CACHE,
true /*isFirstResource*/);
assertFalse(future.cancel(true));
}
......@@ -142,7 +163,12 @@ public class RequestFutureTargetTest {
public void testReturnsResourceOnGetIfAlreadyDone()
throws ExecutionException, InterruptedException {
Object expected = new Object();
future.onResourceReady(expected, null);
future.onResourceReady(
/*resource=*/ expected,
/*model=*/ null,
/*target=*/future,
DataSource.DATA_DISK_CACHE,
true /*isFirstResource*/);
assertEquals(expected, future.get());
}
......@@ -151,7 +177,12 @@ public class RequestFutureTargetTest {
public void testReturnsResourceOnGetWithTimeoutIfAlreadyDone()
throws InterruptedException, ExecutionException, TimeoutException {
Object expected = new Object();
future.onResourceReady(expected, null);
future.onResourceReady(
/*resource=*/ expected,
/*model=*/ null,
/*target=*/future,
DataSource.DATA_DISK_CACHE,
true /*isFirstResource*/);
assertEquals(expected, future.get(100, TimeUnit.MILLISECONDS));
}
......@@ -173,21 +204,21 @@ public class RequestFutureTargetTest {
@Test(expected = ExecutionException.class)
public void testThrowsExecutionExceptionOnGetIfExceptionBeforeGet()
throws ExecutionException, InterruptedException {
future.onLoadFailed(null);
future.onLoadFailed(/*e=*/ null, /*model=*/ null, future, /*isFirstResource=*/ true);
future.get();
}
@Test(expected = ExecutionException.class)
public void testThrowsExecutionExceptionOnGetIfExceptionWithNullValueBeforeGet()
throws ExecutionException, InterruptedException, TimeoutException {
future.onLoadFailed(null);
future.onLoadFailed(/*e=*/ null, /*model=*/ null, future, /*isFirstResource=*/ true);
future.get(100, TimeUnit.MILLISECONDS);
}
@Test(expected = ExecutionException.class)
public void testThrowsExecutionExceptionOnGetIfExceptionBeforeGetWithTimeout()
throws ExecutionException, InterruptedException, TimeoutException {
future.onLoadFailed(null);
future.onLoadFailed(/*e=*/ null, /*model=*/ null, future, /*isFirstResource=*/ true);
future.get(100, TimeUnit.MILLISECONDS);
}
......@@ -208,7 +239,12 @@ public class RequestFutureTargetTest {
public void testGetSucceedsOnMainThreadIfDone()
throws ExecutionException, InterruptedException {
future = new RequestFutureTarget<>(handler, width, height, true, waiter);
future.onResourceReady(new Object(), null);
future.onResourceReady(
/*resource=*/ new Object(),
/*model=*/ null,
/*target=*/future,
DataSource.DATA_DISK_CACHE,
true /*isFirstResource*/);
future.get();
}
......@@ -232,7 +268,7 @@ public class RequestFutureTargetTest {
doAnswer(new Answer<Void>() {
@Override
public Void answer(InvocationOnMock invocationOnMock) throws Throwable {
future.onLoadFailed(null);
future.onLoadFailed(/*e=*/ null, /*model=*/ null, future, /*isFirstResource=*/ true);
return null;
}
}).when(waiter).waitForTimeout(eq(future), anyLong());
......@@ -266,13 +302,18 @@ public class RequestFutureTargetTest {
@Test
public void testNotifiesAllWhenLoadFails() {
future.onLoadFailed(null);
future.onLoadFailed(/*e=*/ null, /*model=*/ null, future, /*isFirstResource=*/ true);
verify(waiter).notifyAll(eq(future));
}
@Test
public void testNotifiesAllWhenResourceReady() {
future.onResourceReady(null, null);
future.onResourceReady(
/*resource=*/ new Object(),
/*model=*/ null,
/*target=*/future,
DataSource.DATA_DISK_CACHE,
true /*isFirstResource*/);
verify(waiter).notifyAll(eq(future));
}
......@@ -297,7 +338,12 @@ public class RequestFutureTargetTest {
doAnswer(new Answer<Void>() {
@Override
public Void answer(InvocationOnMock invocationOnMock) throws Throwable {
future.onResourceReady(expected, null);
future.onResourceReady(
/*resource=*/ expected,
/*model=*/ null,
/*target=*/future,
DataSource.DATA_DISK_CACHE,
true /*isFirstResource*/);
return null;
}
}).when(waiter).waitForTimeout(eq(future), anyLong());
......
......@@ -1002,6 +1002,7 @@ public class SingleRequestTest {
overrideHeight,
priority,
target,
/*targetListener=*/ null,
requestListener,
requestCoordinator,
engine,
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册