the-btfash-foundation/Markwon
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove weak-reference from async-drawable-loader

Browse Source
This commit is contained in:
Dimitry Ivanov 2019-06-28 21:31:28 +03:00
parent c210dc56ca
commit 4f979ea4c6
1 changed files with 20 additions and 171 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -4,14 +4,13 @@ import android.graphics.drawable.Drawable;
import android.net.Uri; import android.net.Uri;
import android.os.Handler; import android.os.Handler;
import android.os.Looper; import android.os.Looper;
import android.os.SystemClock;
import android.support.annotation.NonNull; import android.support.annotation.NonNull;
import android.support.annotation.Nullable; import android.support.annotation.Nullable;
import java.io.IOException; import java.io.IOException;
import java.io.InputStream; import java.io.InputStream;
import java.lang.ref.WeakReference;
import java.util.HashMap; import java.util.HashMap;
import java.util.Iterator;
import java.util.Map; import java.util.Map;
import java.util.concurrent.ExecutorService; import java.util.concurrent.ExecutorService;
import java.util.concurrent.Future; import java.util.concurrent.Future;
@ -24,12 +23,12 @@ class AsyncDrawableLoaderImpl extends AsyncDrawableLoader {
private final MediaDecoder defaultMediaDecoder; private final MediaDecoder defaultMediaDecoder;
private final DrawableProvider placeholderDrawableProvider; private final DrawableProvider placeholderDrawableProvider;
private final DrawableProvider errorDrawableProvider; private final DrawableProvider errorDrawableProvider;
private final Handler handler = new Handler(Looper.getMainLooper());
private final Handler mainThread;
// @since 3.1.0-SNAPSHOT use a hash-map with a weak AsyncDrawable as key for multiple requests // @since 3.1.0-SNAPSHOT use a hash-map with an AsyncDrawable (allows multiple drawables
// for the same destination // referencing the same source)
private final Map<WeakReference<AsyncDrawable>, Future<?>> requests = new HashMap<>(2); private final Map<AsyncDrawable, Future<?>> requests = new HashMap<>(2);
AsyncDrawableLoaderImpl(@NonNull Builder builder) { AsyncDrawableLoaderImpl(@NonNull Builder builder) {
this.executorService = builder.executorService; this.executorService = builder.executorService;
@ -38,143 +37,27 @@ class AsyncDrawableLoaderImpl extends AsyncDrawableLoader {
this.defaultMediaDecoder = builder.defaultMediaDecoder; this.defaultMediaDecoder = builder.defaultMediaDecoder;
this.placeholderDrawableProvider = builder.placeholderDrawableProvider; this.placeholderDrawableProvider = builder.placeholderDrawableProvider;
this.errorDrawableProvider = builder.errorDrawableProvider; this.errorDrawableProvider = builder.errorDrawableProvider;
this.mainThread = new Handler(Looper.getMainLooper());
} }
@Override @Override
public void load(@NonNull final AsyncDrawable drawable) { public void load(@NonNull final AsyncDrawable drawable) {
final Future<?> future = requests.get(drawable);
// primitive synchronization via main-thread if (future == null) {
if (!isMainThread()) { requests.put(drawable, execute(drawable));
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 @Override
public void cancel(@NonNull final AsyncDrawable drawable) { public void cancel(@NonNull final AsyncDrawable drawable) {
if (!isMainThread()) { final Future<?> future = requests.remove(drawable);
mainThread.post(new Runnable() { if (future != null) {
@Override future.cancel(true);
public void run() {
cancel(drawable);
}
});
return;
} }
final Iterator<Map.Entry<WeakReference<AsyncDrawable>, Future<?>>> iterator = handler.removeCallbacksAndMessages(drawable);
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 @Nullable
@Override @Override
public Drawable placeholder() { public Drawable placeholder() {
@ -183,15 +66,10 @@ class AsyncDrawableLoaderImpl extends AsyncDrawableLoader {
: null; : null;
} }
private Future<?> execute(@NonNull final String destination, @NonNull final WeakReference<AsyncDrawable> reference) { @NonNull
private Future<?> execute(@NonNull final AsyncDrawable drawable) {
// todo: error handing (simply applying errorDrawable is not a good solution final String destination = drawable.getDestination();
// as reason for an error is unclear (no scheme handler, no input data, error decoding, etc)
// todo: more efficient ImageMediaDecoder... BitmapFactory.decodeStream is a bit not optimal
// for big images for sure. We _could_ introduce internal Drawable that will check for
// image bounds (but we will need to cache inputStream in order to inspect and optimize
// input image...)
return executorService.submit(new Runnable() { return executorService.submit(new Runnable() {
@Override @Override
@ -245,46 +123,17 @@ class AsyncDrawableLoaderImpl extends AsyncDrawableLoader {
final Drawable out = result; final Drawable out = result;
mainThread.post(new Runnable() { handler.postAtTime(new Runnable() {
@Override @Override
public void run() { public void run() {
if (requests.remove(drawable) != null) {
if (out != null) { if (out != null && drawable.isAttached()) {
drawable.setResult(out);
// 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);
cleanUp();
} }
}); }, drawable, SystemClock.uptimeMillis());
} }
}); });
} }
private static boolean shouldCleanUp(@Nullable AsyncDrawable drawable) {
return drawable == null || !drawable.isAttached();
}
@SuppressWarnings("BooleanMethodIsAlwaysInverted")
private static boolean isMainThread() {
return Looper.myLooper() == Looper.getMainLooper();
}
} }