Skip to content

Commit

Permalink
Pass through exception messages to RequestFutureTarget.
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@sjudd
sjudd committed Oct 22, 2017
1 parent cb1276d commit 65048a4
Show file tree
Hide file tree
Showing 6 changed files with 237 additions and 55 deletions.
130 changes: 101 additions & 29 deletions library/src/main/java/com/bumptech/glide/RequestBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -476,18 +476,27 @@ public RequestBuilder<TranscodeType> clone() {
* @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) {
throw new IllegalArgumentException("You must call #load() before calling #into()");
}

options = options.autoClone();
Request request = buildRequest(target, options);
Request request = buildRequest(target, targetListener, options);

Request previous = target.getRequest();
if (request.isEquivalentTo(previous)) {
Expand Down Expand Up @@ -557,7 +566,10 @@ public Target<TranscodeType> into(ImageView view) {
}
}

return into(glideContext.buildImageViewTarget(view, transcodeClass), requestOptions);
return into(
glideContext.buildImageViewTarget(view, transcodeClass),
/*targetListener=*/ null,
requestOptions);
}

/**
Expand Down Expand Up @@ -617,12 +629,12 @@ public FutureTarget<TranscodeType> submit(int width, int height) {
@Override
public void run() {
if (!target.isCancelled()) {
into(target);
into(target, target);
}
}
});
} else {
into(target);
into(target, target);
}

return target;
Expand Down Expand Up @@ -717,15 +729,30 @@ private Priority getThumbnailPriority(Priority current) {
}
}

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;
Expand All @@ -737,6 +764,7 @@ private Request buildRequestRecursive(Target<TranscodeType> target,
Request mainRequest =
buildThumbnailRequestRecursive(
target,
targetListener,
parentCoordinator,
transitionOptions,
priority,
Expand All @@ -758,6 +786,7 @@ private Request buildRequestRecursive(Target<TranscodeType> target,

Request errorRequest = errorBuilder.buildRequestRecursive(
target,
targetListener,
errorRequestCoordinator,
errorBuilder.transitionOptions,
errorBuilder.requestOptions.getPriority(),
Expand All @@ -768,10 +797,15 @@ private Request buildRequestRecursive(Target<TranscodeType> target,
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) {
Expand Down Expand Up @@ -800,13 +834,22 @@ private Request buildThumbnailRequestRecursive(Target<TranscodeType> target,
}

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,
Expand All @@ -819,27 +862,55 @@ private Request buildThumbnailRequestRecursive(Target<TranscodeType> target,
} 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,
Expand All @@ -850,6 +921,7 @@ private Request obtainRequest(Target<TranscodeType> target,
overrideHeight,
priority,
target,
targetListener,
requestListener,
requestCoordinator,
glideContext.getEngine(),
Expand Down
1 change: 1 addition & 0 deletions library/src/main/java/com/bumptech/glide/load/engine/DecodeJob.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,7 @@ public void run() {
}
// 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) {
Expand Down
67 changes: 59 additions & 8 deletions library/src/main/java/com/bumptech/glide/request/RequestFutureTarget.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,14 @@
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;
Expand Down Expand Up @@ -47,6 +52,7 @@
* @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();

Expand All @@ -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.
Expand Down Expand Up @@ -162,19 +169,15 @@ public void onLoadStarted(Drawable placeholder) {
*/
@Override
public synchronized void onLoadFailed(Drawable errorDrawable) {
loadFailed = true;
waiter.notifyAll(this);
// Ignored, synchronized for backwards compatibility.
}

/**
* A callback that should never be invoked directly.
*/
@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)
Expand All @@ -186,7 +189,7 @@ private synchronized R doGet(Long timeoutMillis)
if (isCancelled) {
throw new CancellationException();
} else if (loadFailed) {
throw new ExecutionException(new IllegalStateException("Load failed"));
throw new ExecutionException(exception);
} else if (resultReceived) {
return resource;
}
Expand All @@ -200,7 +203,7 @@ private synchronized R doGet(Long timeoutMillis)
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) {
Expand Down Expand Up @@ -240,6 +243,25 @@ public void onDestroy() {
// 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 {

Expand All @@ -251,4 +273,33 @@ public void notifyAll(Object toNotify) {
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);
}
}
}
Loading

0 comments on commit 65048a4

Please sign in to comment.