Better asynchronous image loading for BookmarksFragment

Previous AsyncTask would throw a RejectedExecutionException if too many
AsyncTasks got spawned on the thread pool executor. The solution was to
create a custom Executor that properly executed the task and queue it if
necessary. Also switched to using weakreference for the view and set
timeouts on image loading so it can load faster.

app/src/main/java/acr/browser/lightning/async/AsyncExecutor.java

@@ -0,0 +1,50 @@
+package acr.browser.lightning.async;
+
+import android.support.annotation.NonNull;
+import android.util.Log;
+
+import java.util.ArrayDeque;
+import java.util.Queue;
+import java.util.concurrent.Executor;
+import java.util.concurrent.Executors;
+import java.util.concurrent.RejectedExecutionException;
+
+/**
+ * Created 9/27/2015 Anthony Restaino
+ */
+public class AsyncExecutor implements Executor {
+
+    private static final String TAG = AsyncExecutor.class.getSimpleName();
+    private static AsyncExecutor INSTANCE = new AsyncExecutor();
+    private Queue<Runnable> mQueue = new ArrayDeque<>(1);
+    private Executor mExecutor = Executors.newFixedThreadPool(4);
+
+    private AsyncExecutor() {}
+
+    public static AsyncExecutor getInstance() {
+        return INSTANCE;
+    }
+
+    public synchronized void notifyThreadFinish() {
+        if (mQueue.isEmpty()) {
+            return;
+        }
+        Runnable runnable = mQueue.remove();
+        execute(runnable);
+    }
+
+    @Override
+    protected void finalize() throws Throwable {
+        super.finalize();
+    }
+
+    @Override
+    public void execute(@NonNull Runnable command) {
+        try {
+            mExecutor.execute(command);
+        } catch (RejectedExecutionException ignored) {
+            mQueue.add(command);
+            Log.d(TAG, "Thread was enqueued");
+        }
+    }
+}

app/src/main/java/acr/browser/lightning/async/ImageDownloadTask.java

@@ -1,4 +1,4 @@
-package acr.browser.lightning.utils;
+package acr.browser.lightning.async;
 
 import android.graphics.Bitmap;
 import android.graphics.BitmapFactory;
@@ -11,33 +11,35 @@ import android.widget.ImageView;
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.InputStream;
+import java.lang.ref.WeakReference;
 import java.net.HttpURLConnection;
 import java.net.URL;
 
 import acr.browser.lightning.app.BrowserApp;
 import acr.browser.lightning.constant.Constants;
 import acr.browser.lightning.database.HistoryItem;
+import acr.browser.lightning.utils.Utils;
 
-/**
- * Created by Stefano Pacifici on 25/08/15.
- */
-public class DownloadImageTask extends AsyncTask<Void, Void, Bitmap> {
+public class ImageDownloadTask extends AsyncTask<Void, Void, Bitmap> {
 
-    private final ImageView bmImage;
+    private static final String TAG = ImageDownloadTask.class.getSimpleName();
+    private static final File mCacheDir = BrowserApp.getAppContext().getCacheDir();
+    private final WeakReference<ImageView> bmImage;
     private final HistoryItem mWeb;
-    private final File mCacheDir;
     private final String mUrl;
     private final Bitmap mDefaultBitmap;
 
-    public DownloadImageTask(@NonNull ImageView bmImage, @NonNull HistoryItem web,
-                             @NonNull Bitmap defaultBitmap) {
-        this.bmImage = bmImage;
+    public ImageDownloadTask(@NonNull ImageView bmImage, @NonNull HistoryItem web, @NonNull Bitmap defaultBitmap) {
+        // Set a tag on the ImageView so we know if the view
+        // has gone out of scope and should not be used
+        bmImage.setTag(web.getUrl().hashCode());
+        this.bmImage = new WeakReference<>(bmImage);
         this.mWeb = web;
-        this.mCacheDir = BrowserApp.getAppContext().getCacheDir();
         this.mUrl = web.getUrl();
         this.mDefaultBitmap = defaultBitmap;
     }
 
+    @Override
     protected Bitmap doInBackground(Void... params) {
         Bitmap mIcon = null;
         // unique path for each url that is bookmarked.
@@ -60,6 +62,8 @@ public class DownloadImageTask extends AsyncTask<Void, Void, Bitmap> {
                 final URL urlDownload = new URL(urlDisplay);
                 final HttpURLConnection connection = (HttpURLConnection) urlDownload.openConnection();
                 connection.setDoInput(true);
+                connection.setConnectTimeout(1000);
+                connection.setReadTimeout(1000);
                 connection.connect();
                 in = connection.getInputStream();
 
@@ -74,8 +78,8 @@ public class DownloadImageTask extends AsyncTask<Void, Void, Bitmap> {
                     Log.d(Constants.TAG, "Downloaded: " + urlDisplay);
                 }
 
-            } catch (Exception e) {
-                e.printStackTrace();
+            } catch (Exception ignored) {
+                Log.d(TAG, "Could not download: " + urlDisplay);
             } finally {
                 Utils.close(in);
                 Utils.close(fos);
@@ -89,10 +93,11 @@ public class DownloadImageTask extends AsyncTask<Void, Void, Bitmap> {
             FileOutputStream fos = null;
             try {
                 // if not, download it...
-                final URL urlDownload = new URL("https://www.google.com/s2/favicons?domain_url="
-                        + uri.toString());
+                final URL urlDownload = new URL("https://www.google.com/s2/favicons?domain_url=" + uri.toString());
                 final HttpURLConnection connection = (HttpURLConnection) urlDownload.openConnection();
                 connection.setDoInput(true);
+                connection.setConnectTimeout(1000);
+                connection.setReadTimeout(1000);
                 connection.connect();
                 in = connection.getInputStream();
 
@@ -107,7 +112,7 @@ public class DownloadImageTask extends AsyncTask<Void, Void, Bitmap> {
                 }
 
             } catch (Exception e) {
-                e.printStackTrace();
+                Log.d(TAG, "Could not download Google favicon");
             } finally {
                 Utils.close(in);
                 Utils.close(fos);
@@ -120,10 +125,16 @@ public class DownloadImageTask extends AsyncTask<Void, Void, Bitmap> {
         }
     }
 
-    protected void onPostExecute(Bitmap result) {
-        final Bitmap fav = Utils.padFavicon(result);
-        bmImage.setImageBitmap(fav);
+    @Override
+    protected void onPostExecute(Bitmap bitmap) {
+        super.onPostExecute(bitmap);
+        AsyncExecutor.getInstance().notifyThreadFinish();
+        final Bitmap fav = Utils.padFavicon(bitmap);
+        ImageView view = bmImage.get();
+        if (view != null && view.getTag().equals(mWeb.getUrl().hashCode())) {
+            view.setImageBitmap(fav);
+        }
         mWeb.setBitmap(fav);
-        // notifyBookmarkDataSetChanged();
     }
+
 }

app/src/main/java/acr/browser/lightning/fragment/BookmarksFragment.java

@@ -3,7 +3,6 @@ package acr.browser.lightning.fragment;
 import android.content.Context;
 import android.graphics.Bitmap;
 import android.graphics.PorterDuff;
-import android.os.AsyncTask;
 import android.os.Bundle;
 import android.support.annotation.IdRes;
 import android.support.annotation.NonNull;
@@ -38,13 +37,14 @@ import javax.inject.Inject;
 import acr.browser.lightning.R;
 import acr.browser.lightning.activity.BrowserActivity;
 import acr.browser.lightning.app.BrowserApp;
+import acr.browser.lightning.async.AsyncExecutor;
 import acr.browser.lightning.bus.BookmarkEvents;
 import acr.browser.lightning.bus.BrowserEvents;
 import acr.browser.lightning.database.BookmarkManager;
 import acr.browser.lightning.database.HistoryItem;
 import acr.browser.lightning.dialog.BookmarksDialogBuilder;
 import acr.browser.lightning.preference.PreferenceManager;
-import acr.browser.lightning.utils.DownloadImageTask;
+import acr.browser.lightning.async.ImageDownloadTask;
 import acr.browser.lightning.utils.ThemeUtils;
 
 /**
@@ -103,8 +103,7 @@ public class BookmarksFragment extends Fragment implements View.OnClickListener,
         public void onItemClick(AdapterView<?> parent, View view, int position, long id) {
             final HistoryItem item = mBookmarks.get(position);
             if (item.isFolder()) {
-                setBookmarkDataSet(mBookmarkManager.getBookmarksFromFolder(item.getTitle(), true),
-                        true);
+                setBookmarkDataSet(mBookmarkManager.getBookmarksFromFolder(item.getTitle(), true), true);
             } else {
                 mEventBus.post(new BookmarkEvents.Clicked(item));
             }
@@ -139,8 +138,7 @@ public class BookmarksFragment extends Fragment implements View.OnClickListener,
         backView.setOnClickListener(new View.OnClickListener() {
             @Override
             public void onClick(View v) {
-                if (mBookmarkManager == null)
-                    return;
+                if (mBookmarkManager == null) return;
                 if (!mBookmarkManager.isRootFolder()) {
                     setBookmarkDataSet(mBookmarkManager.getBookmarksFromFolder(null, true), true);
                 }
@@ -186,8 +184,7 @@ public class BookmarksFragment extends Fragment implements View.OnClickListener,
             mBookmarks.add(item);
             Collections.sort(mBookmarks, new BookmarkManager.SortIgnoreCase());
             mBookmarkAdapter.notifyDataSetChanged();
-            mEventBus
-                    .post(new BookmarkEvents.Added(item));
+            mEventBus.post(new BookmarkEvents.Added(item));
             updateBookmarkIndicator(event.url);
         }
     }
@@ -345,19 +342,19 @@ public class BookmarksFragment extends Fragment implements View.OnClickListener,
 
             HistoryItem web = mBookmarks.get(position);
             holder.txtTitle.setText(web.getTitle());
-            holder.favicon.setImageBitmap(mWebpageBitmap);
             if (web.isFolder()) {
                 holder.favicon.setImageBitmap(mFolderBitmap);
             } else if (web.getBitmap() == null) {
-                new DownloadImageTask(holder.favicon, web, mWebpageBitmap)
-                        .executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
+                holder.favicon.setImageBitmap(mWebpageBitmap);
+                new ImageDownloadTask(holder.favicon, web, mWebpageBitmap)
+                        .executeOnExecutor(AsyncExecutor.getInstance());
             } else {
                 holder.favicon.setImageBitmap(web.getBitmap());
             }
             return row;
         }
 
-        class BookmarkViewHolder {
+        private class BookmarkViewHolder {
             TextView txtTitle;
             ImageView favicon;
         }