AsyncDrawableLoader improvements (multiple images with the same source)

gradle.properties

@@ -6,7 +6,7 @@ org.gradle.configureondemand=true
 android.enableBuildCache=true
 android.buildCacheDir=build/pre-dex-cache
 
-VERSION_NAME=3.0.1
+VERSION_NAME=3.1.0-SNAPSHOT
 
 GROUP=ru.noties.markwon
 POM_DESCRIPTION=Markwon markdown for Android

markwon-core/src/main/java/ru/noties/markwon/image/AsyncDrawable.java

@@ -32,7 +32,7 @@ public class AsyncDrawable extends Drawable {
     public AsyncDrawable(
             @NonNull String destination,
             @NonNull AsyncDrawableLoader loader,
-            @Nullable ImageSizeResolver imageSizeResolver,
+            @NonNull ImageSizeResolver imageSizeResolver,
             @Nullable ImageSize imageSize
     ) {
         this.destination = destination;
@@ -51,6 +51,45 @@ public class AsyncDrawable extends Drawable {
         return destination;
     }
 
+    /**
+     * @since 3.1.0-SNAPSHOT
+     */
+    @Nullable
+    public ImageSize getImageSize() {
+        return imageSize;
+    }
+
+    /**
+     * @since 3.1.0-SNAPSHOT
+     */
+    @NonNull
+    public ImageSizeResolver getImageSizeResolver() {
+        return imageSizeResolver;
+    }
+
+    /**
+     * @since 3.1.0-SNAPSHOT
+     */
+    public boolean hasKnownDimentions() {
+        return canvasWidth > 0;
+    }
+
+    /**
+     * @see #hasKnownDimentions()
+     * @since 3.1.0-SNAPSHOT
+     */
+    public int getLastKnownCanvasWidth() {
+        return canvasWidth;
+    }
+
+    /**
+     * @see #hasKnownDimentions()
+     * @since 3.1.0-SNAPSHOT
+     */
+    public float getLastKnowTextSize() {
+        return textSize;
+    }
+
     public Drawable getResult() {
         return result;
     }
@@ -80,7 +119,7 @@ public class AsyncDrawable extends Drawable {
                 result.setCallback(callback);
             }
 
-            loader.load(destination, this);
+            loader.load(this);
         } else {
             if (result != null) {
 
@@ -91,7 +130,7 @@ public class AsyncDrawable extends Drawable {
                     ((Animatable) result).stop();
                 }
             }
-            loader.cancel(destination);
+            loader.cancel(this);
         }
     }
 

markwon-core/src/main/java/ru/noties/markwon/image/AsyncDrawableLoader.java

@@ -3,6 +3,7 @@ package ru.noties.markwon.image;
 import android.graphics.drawable.Drawable;
 import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
+import android.util.Log;
 
 import java.util.Collection;
 import java.util.HashMap;
@@ -36,10 +37,37 @@ public abstract class AsyncDrawableLoader {
         return new AsyncDrawableLoaderNoOp();
     }
 
+    /**
+     * @since 3.1.0-SNAPSHOT
+     */
+    public abstract void load(@NonNull AsyncDrawable drawable);
 
-    public abstract void load(@NonNull String destination, @NonNull AsyncDrawable drawable);
+    /**
+     * @since 3.1.0-SNAPSHOT
+     */
+    public abstract void cancel(@NonNull AsyncDrawable drawable);
 
-    public abstract void cancel(@NonNull String destination);
+    /**
+     * @see #load(AsyncDrawable)
+     * @deprecated 3.1.0-SNAPSHOT
+     */
+    @Deprecated
+    public void load(@NonNull String destination, @NonNull AsyncDrawable drawable) {
+        load(drawable);
+    }
+
+    /**
+     * Method is deprecated because cancellation won\'t work for markdown input
+     * with multiple images with the same source
+     *
+     * @deprecated 3.1.0-SNAPSHOT
+     */
+    @Deprecated
+    public void cancel(@NonNull String destination) {
+        Log.e("MARKWON-IL", "Image loading cancellation must be triggered " +
+                "by AsyncDrawable, please use #cancel(AsyncDrawable) method instead. " +
+                "No op, nothing is cancelled for destination: " + destination);
+    }
 
     @Nullable
     public abstract Drawable placeholder();

markwon-core/src/main/java/ru/noties/markwon/image/AsyncDrawableLoaderImpl.java

@@ -11,6 +11,7 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.lang.ref.WeakReference;
 import java.util.HashMap;
+import java.util.Iterator;
 import java.util.Map;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Future;
@@ -26,7 +27,9 @@ class AsyncDrawableLoaderImpl extends AsyncDrawableLoader {
 
     private final Handler mainThread;
 
-    private final Map<String, Future<?>> requests = new HashMap<>(2);
+    // @since 3.1.0-SNAPSHOT use a hash-map with a weak AsyncDrawable as key for multiple requests
+    //  for the same destination
+    private final Map<WeakReference<AsyncDrawable>, Future<?>> requests = new HashMap<>(2);
 
     AsyncDrawableLoaderImpl(@NonNull Builder builder) {
         this.executorService = builder.executorService;
@@ -39,18 +42,138 @@ class AsyncDrawableLoaderImpl extends AsyncDrawableLoader {
     }
 
     @Override
-    public void load(@NonNull String destination, @NonNull AsyncDrawable drawable) {
+    public void load(@NonNull final AsyncDrawable drawable) {
-        // if drawable is not a link -> show loading placeholder...
+
-        requests.put(destination, execute(destination, drawable));
+        // primitive synchronization via main-thread
+        if (!isMainThread()) {
+            mainThread.post(new Runnable() {
+                @Override
+                public void run() {
+                    load(drawable);
+                }
+            });
+            return;
+        }
+
+        // okay, if by some chance requested drawable already has a future associated -> no-op
+        // as AsyncDrawable cannot change `destination` (immutable field)
+        // @since 3.1.0-SNAPSHOT
+        if (hasTaskAssociated(drawable)) {
+            return;
+        }
+
+        final WeakReference<AsyncDrawable> reference = new WeakReference<>(drawable);
+        requests.put(reference, execute(drawable.getDestination(), reference));
     }
 
     @Override
-    public void cancel(@NonNull String destination) {
+    public void cancel(@NonNull final AsyncDrawable drawable) {
-        final Future<?> request = requests.remove(destination);
+
-        if (request != null) {
+        if (!isMainThread()) {
-            request.cancel(true);
+            mainThread.post(new Runnable() {
+                @Override
+                public void run() {
+                    cancel(drawable);
+                }
+            });
+            return;
+        }
+
+        final Iterator<Map.Entry<WeakReference<AsyncDrawable>, Future<?>>> iterator =
+                requests.entrySet().iterator();
+
+        AsyncDrawable key;
+        Map.Entry<WeakReference<AsyncDrawable>, Future<?>> entry;
+
+        while (iterator.hasNext()) {
+
+            entry = iterator.next();
+            key = entry.getKey().get();
+
+            // if key is null or it contains requested AsyncDrawable -> cancel
+            if (shouldCleanUp(key) || key == drawable) {
+                entry.getValue().cancel(true);
+                iterator.remove();
             }
         }
+    }
+
+    private boolean hasTaskAssociated(@NonNull AsyncDrawable drawable) {
+
+        final Iterator<Map.Entry<WeakReference<AsyncDrawable>, Future<?>>> iterator =
+                requests.entrySet().iterator();
+
+        boolean result = false;
+
+        AsyncDrawable key;
+        Map.Entry<WeakReference<AsyncDrawable>, Future<?>> entry;
+
+        while (iterator.hasNext()) {
+
+            entry = iterator.next();
+            key = entry.getKey().get();
+
+            // clean-up
+            if (shouldCleanUp(key)) {
+                entry.getValue().cancel(true);
+                iterator.remove();
+            } else if (key == drawable) {
+                result = true;
+                // do not break, let iteration continue to possibly clean-up the rest references
+            }
+        }
+
+        return result;
+    }
+
+    private void cleanUp() {
+
+        final Iterator<Map.Entry<WeakReference<AsyncDrawable>, Future<?>>> iterator =
+                requests.entrySet().iterator();
+
+        AsyncDrawable key;
+        Map.Entry<WeakReference<AsyncDrawable>, Future<?>> entry;
+
+        while (iterator.hasNext()) {
+
+            entry = iterator.next();
+            key = entry.getKey().get();
+
+            // clean-up of already referenced or detached drawables
+            if (shouldCleanUp(key)) {
+                entry.getValue().cancel(true);
+                iterator.remove();
+            }
+        }
+    }
+
+//    @Override
+//    public void load(@NonNull String destination, @NonNull AsyncDrawable drawable) {
+//
+//        // todo: we cannot reliably identify request by the destination, as if
+//        //  markdown input has multiple images with the same destination as source
+//        //  we will be tracking only one of them (the one appears the last). We should
+//        //  move to AsyncDrawable based identification. This method also _maybe_
+//        //  should include the ImageSize (comment @since 3.1.0-SNAPSHOT)
+//
+//        requests.put(destination, execute(destination, drawable));
+//    }
+//
+//    @Override
+//    public void cancel(@NonNull String destination) {
+//
+//        // todo: as we are moving away from a single request for a destination,
+//        //  we should re-evaluate this cancellation logic, as if there are multiple images
+//        //  in markdown input all of them will be cancelled (won't delivered), even if
+//        //  only a single drawable is detached. Cancellation must also take
+//        //  the AsyncDrawable argument (comment @since 3.1.0-SNAPSHOT)
+//
+//        //
+//        final Future<?> request = requests.remove(destination);
+//        if (request != null) {
+//            request.cancel(true);
+//        }
+//    }
 
     @Nullable
     @Override
@@ -60,12 +183,7 @@ class AsyncDrawableLoaderImpl extends AsyncDrawableLoader {
                 : null;
     }
 
-    private Future<?> execute(@NonNull final String destination, @NonNull AsyncDrawable drawable) {
+    private Future<?> execute(@NonNull final String destination, @NonNull final WeakReference<AsyncDrawable> reference) {
-
-        final WeakReference<AsyncDrawable> reference = new WeakReference<AsyncDrawable>(drawable);
-
-        // todo: should we cancel pending request for the same destination?
-        //      we _could_ but there is possibility that one resource is request in multiple places
 
         // todo: error handing (simply applying errorDrawable is not a good solution
         //      as reason for an error is unclear (no scheme handler, no input data, error decoding, etc)
@@ -125,24 +243,48 @@ class AsyncDrawableLoaderImpl extends AsyncDrawableLoader {
                             : null;
                 }
 
-                if (result != null) {
                 final Drawable out = result;
+
                 mainThread.post(new Runnable() {
                     @Override
                     public void run() {
-                            final boolean canDeliver = requests.remove(destination) != null;
+
-                            if (canDeliver) {
+                        if (out != null) {
+
+                            // this doesn't work with markdown input with multiple images with the
+                            // same source (comment @since 3.1.0-SNAPSHOT)
+//                            final boolean canDeliver = requests.remove(destination) != null;
+//                            if (canDeliver) {
+//                                final AsyncDrawable asyncDrawable = reference.get();
+//                                if (asyncDrawable != null && asyncDrawable.isAttached()) {
+//                                    asyncDrawable.setResult(out);
+//                                }
+//                            }
+
+                            // todo: AsyncDrawable cannot change destination, so if it's
+                            //  attached and not garbage-collected, we can deliver the result.
+                            //  Note that there is no cache, so attach/detach of drawables
+                            //  will always request a new entry.. (comment @since 3.1.0-SNAPSHOT)
                             final AsyncDrawable asyncDrawable = reference.get();
                             if (asyncDrawable != null && asyncDrawable.isAttached()) {
                                 asyncDrawable.setResult(out);
                             }
                         }
-                        }
+
-                    });
+                        requests.remove(reference);
-                } else {
+                        cleanUp();
-                    requests.remove(destination);
-                }
                     }
                 });
             }
+        });
+    }
+
+    private static boolean shouldCleanUp(@Nullable AsyncDrawable drawable) {
+        return drawable == null || !drawable.isAttached();
+    }
+
+    @SuppressWarnings("BooleanMethodIsAlwaysInverted")
+    private static boolean isMainThread() {
+        return Looper.myLooper() == Looper.getMainLooper();
+    }
 }

markwon-core/src/main/java/ru/noties/markwon/image/AsyncDrawableLoaderNoOp.java

@@ -6,12 +6,12 @@ import android.support.annotation.Nullable;
 
 class AsyncDrawableLoaderNoOp extends AsyncDrawableLoader {
     @Override
-    public void load(@NonNull String destination, @NonNull AsyncDrawable drawable) {
+    public void load(@NonNull AsyncDrawable drawable) {
 
     }
 
     @Override
-    public void cancel(@NonNull String destination) {
+    public void cancel(@NonNull AsyncDrawable drawable) {
 
     }