Handle notifications in MultiFetcher after cancellation.

We can’t guarantee that every fetcher implementation will strictly avoid notifying after they’ve been cancelled. This also improves the behavior in VolleyStreamFetcher so that it attempts to avoid notifying after cancel, although it still doesn’t make any strict guarantee. Fixes #2879
bumptech · Feb 8, 2018 · a690f38 · a690f38
1 parent 914060e
commit a690f38
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 9 deletions.
diff --git a/...ation/volley/src/main/java/com/bumptech/glide/integration/volley/VolleyStreamFetcher.java b/...ation/volley/src/main/java/com/bumptech/glide/integration/volley/VolleyStreamFetcher.java
@@ -133,13 +133,17 @@ protected VolleyError parseNetworkError(VolleyError volleyError) {
       if (Log.isLoggable(TAG, Log.DEBUG)) {
         Log.d(TAG, "Volley failed to retrieve response", volleyError);
       }
-      callback.onLoadFailed(volleyError);
+      if (!isCanceled()) {
+        callback.onLoadFailed(volleyError);
+      }
       return super.parseNetworkError(volleyError);
     }
 
     @Override
     protected Response<byte[]> parseNetworkResponse(NetworkResponse response) {
-      callback.onDataReady(new ByteArrayInputStream(response.data));
+      if (!isCanceled()) {
+        callback.onDataReady(new ByteArrayInputStream(response.data));
+      }
       return Response.success(response.data, HttpHeaderParser.parseCacheHeaders(response));
     }
 

diff --git a/library/src/main/java/com/bumptech/glide/load/model/MultiModelLoader.java b/library/src/main/java/com/bumptech/glide/load/model/MultiModelLoader.java
@@ -52,7 +52,7 @@ public LoadData<Data> buildLoadData(@NonNull Model model, int width, int height,
         }
       }
     }
-    return !fetchers.isEmpty()
+    return !fetchers.isEmpty() && sourceKey != null
         ? new LoadData<>(sourceKey, new MultiFetcher<>(fetchers, exceptionListPool)) : null;
   }
 
@@ -80,8 +80,10 @@ static class MultiFetcher<Data> implements DataFetcher<Data>, DataCallback<Data>
     private DataCallback<? super Data> callback;
     @Nullable
     private List<Throwable> exceptions;
+    private boolean isCancelled;
 
-    MultiFetcher(@NonNull List<DataFetcher<Data>> fetchers,
+    MultiFetcher(
+        @NonNull List<DataFetcher<Data>> fetchers,
         @NonNull Pool<List<Throwable>> throwableListPool) {
       this.throwableListPool = throwableListPool;
       Preconditions.checkNotEmpty(fetchers);
@@ -90,15 +92,16 @@ static class MultiFetcher<Data> implements DataFetcher<Data>, DataCallback<Data>
     }
 
     @Override
-    public void loadData(@NonNull Priority priority, @NonNull DataCallback<? super Data> callback) {
+    public synchronized void loadData(
+        @NonNull Priority priority, @NonNull DataCallback<? super Data> callback) {
       this.priority = priority;
       this.callback = callback;
       exceptions = throwableListPool.acquire();
       fetchers.get(currentIndex).loadData(priority, this);
     }
 
     @Override
-    public void cleanup() {
+    public synchronized void cleanup() {
       if (exceptions != null) {
         throwableListPool.release(exceptions);
       }
@@ -109,7 +112,8 @@ public void cleanup() {
     }
 
     @Override
-    public void cancel() {
+    public synchronized void cancel() {
+      isCancelled = true;
       for (DataFetcher<Data> fetcher : fetchers) {
         fetcher.cancel();
       }
@@ -128,7 +132,11 @@ public DataSource getDataSource() {
     }
 
     @Override
-    public void onDataReady(@Nullable Data data) {
+    public synchronized void onDataReady(@Nullable Data data) {
+      if (isCancelled) {
+        return;
+      }
+
       if (data != null) {
         callback.onDataReady(data);
       } else {
@@ -137,12 +145,20 @@ public void onDataReady(@Nullable Data data) {
     }
 
     @Override
-    public void onLoadFailed(@NonNull Exception e) {
+    public synchronized void onLoadFailed(@NonNull Exception e) {
+      if (isCancelled) {
+        return;
+      }
+
       Preconditions.checkNotNull(exceptions).add(e);
       startNextOrFail();
     }
 
     private void startNextOrFail() {
+      if (isCancelled) {
+        return;
+      }
+
       if (currentIndex < fetchers.size() - 1) {
         currentIndex++;
         loadData(priority, callback);