DownloadObserver must die

9 files changed, 156 insertions, 238 deletions

diff --git a/core/src/main/java/de/danoeh/antennapod/core/asynctask/DownloadObserver.java b/core/src/main/java/de/danoeh/antennapod/core/asynctask/DownloadObserver.java
deleted file mode 100644
index 2ce506c22..000000000
--- a/core/src/main/java/de/danoeh/antennapod/core/asynctask/DownloadObserver.java
+++ /dev/null
@@ -1,191 +0,0 @@
-package de.danoeh.antennapod.core.asynctask;
-
-import android.app.Activity;
-import android.content.*;
-import android.os.Handler;
-import android.os.IBinder;
-import android.util.Log;
-
-import org.apache.commons.lang3.Validate;
-
-import de.danoeh.antennapod.core.BuildConfig;
-import de.danoeh.antennapod.core.service.download.DownloadService;
-import de.danoeh.antennapod.core.service.download.Downloader;
-
-import java.util.ArrayList;
-import java.util.List;
-import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.concurrent.atomic.AtomicInteger;
-
-/**
- * Provides access to the DownloadService's list of items that are currently being downloaded.
- * The DownloadObserver object should be created in the activity's onCreate() method. resume() and pause()
- * should be called in the activity's onResume() and onPause() methods
- */
-public class DownloadObserver {
-    private static final String TAG = "DownloadObserver";
-
-    /**
-     * Time period between update notifications.
-     */
-    public static final int WAITING_INTERVAL_MS = 3000;
-
-    private volatile Activity activity;
-    private final Handler handler;
-    private final Callback callback;
-
-    private DownloadService downloadService = null;
-    private AtomicBoolean mIsBound = new AtomicBoolean(false);
-
-    private Thread refresherThread;
-    private AtomicBoolean refresherThreadRunning = new AtomicBoolean(false);
-
-    private AtomicInteger users = new AtomicInteger(0);
-
-
-    /**
-     * Creates a new download observer.
-     *
-     * @param activity Used for registering receivers
-     * @param handler  All callback methods are executed on this handler. The handler MUST run on the GUI thread.
-     * @param callback Callback methods for posting content updates
-     * @throws java.lang.IllegalArgumentException if one of the arguments is null.
-     */
-    public DownloadObserver(Activity activity, Handler handler, Callback callback) {
-        Validate.notNull(activity);
-        Validate.notNull(handler);
-        Validate.notNull(callback);
-
-        this.activity = activity;
-        this.handler = handler;
-        this.callback = callback;
-    }
-
-    public void onResume() {
-        Log.d(TAG, "DownloadObserver resumed");
-        if(users.getAndIncrement() == 0) {
-            activity.registerReceiver(contentChangedReceiver, new IntentFilter(DownloadService.ACTION_DOWNLOADS_CONTENT_CHANGED));
-            connectToDownloadService();
-        }
-    }
-
-    public void onPause() {
-        Log.d(TAG, "DownloadObserver paused");
-        if(users.decrementAndGet() > 0) {
-            return;
-        }
-        try {
-            activity.unregisterReceiver(contentChangedReceiver);
-        } catch (IllegalArgumentException e) {
-            e.printStackTrace();
-        }
-        try {
-            activity.unbindService(mConnection);
-        } catch (IllegalArgumentException e) {
-            e.printStackTrace();
-        }
-        stopRefresher();
-    }
-
-    private BroadcastReceiver contentChangedReceiver = new BroadcastReceiver() {
-        @Override
-        public void onReceive(Context context, Intent intent) {
-            // reconnect to DownloadService if connection has been closed
-            if (downloadService == null) {
-                connectToDownloadService();
-            }
-            if (downloadService != null) {
-                callback.onContentChanged(downloadService.getDownloads());
-            } else {
-                // the service is gone, there are no more downloads.
-                callback.onContentChanged(new ArrayList<Downloader>());
-            }
-            startRefresher();
-        }
-    };
-
-    public interface Callback {
-        void onContentChanged(List<Downloader> downloaderList);
-    }
-
-    private void connectToDownloadService() {
-        activity.bindService(new Intent(activity, DownloadService.class), mConnection, 0);
-    }
-
-    private ServiceConnection mConnection = new ServiceConnection() {
-        public void onServiceDisconnected(ComponentName className) {
-            downloadService = null;
-            mIsBound.set(false);
-            stopRefresher();
-            Log.i(TAG, "Closed connection with DownloadService.");
-        }
-
-        public void onServiceConnected(ComponentName name, IBinder service) {
-            downloadService = ((DownloadService.LocalBinder) service)
-                    .getService();
-            mIsBound.set(true);
-            if (BuildConfig.DEBUG)
-                Log.d(TAG, "Connection to service established");
-            List<Downloader> downloaderList = downloadService.getDownloads();
-            if (downloaderList != null && !downloaderList.isEmpty()) {
-                callback.onContentChanged(downloaderList);
-                startRefresher();
-            }
-        }
-    };
-
-    private void stopRefresher() {
-        if (refresherThread != null) {
-            refresherThread.interrupt();
-        }
-    }
-
-    private void startRefresher() {
-        if (refresherThread == null || refresherThread.isInterrupted()) {
-            refresherThread = new Thread(new RefresherThread());
-            refresherThread.start();
-        }
-    }
-
-    private class RefresherThread implements Runnable {
-
-        public void run() {
-            refresherThreadRunning.set(true);
-            while (!Thread.interrupted()) {
-                try {
-                    Thread.sleep(WAITING_INTERVAL_MS);
-                } catch (InterruptedException e) {
-                    Log.d(TAG, "Refresher thread was interrupted");
-                }
-                if (mIsBound.get()) {
-                    postUpdate();
-                }
-            }
-            refresherThreadRunning.set(false);
-        }
-
-        private void postUpdate() {
-            handler.post(new Runnable() {
-                @Override
-                public void run() {
-                    if (downloadService != null) {
-                        List<Downloader> downloaderList = downloadService.getDownloads();
-                        callback.onContentChanged(downloaderList);
-                        if (downloaderList == null || downloaderList.isEmpty()) {
-                            Thread.currentThread().interrupt();
-                        }
-                    } else {
-                        callback.onContentChanged(new ArrayList<Downloader>());
-                    }
-                }
-            });
-        }
-    }
-
-    public void setActivity(Activity activity) {
-        Validate.notNull(activity);
-        this.activity = activity;
-    }
-
-}
-
diff --git a/core/src/main/java/de/danoeh/antennapod/core/event/DownloadEvent.java b/core/src/main/java/de/danoeh/antennapod/core/event/DownloadEvent.java
new file mode 100644
index 000000000..124fd3e64
--- /dev/null
+++ b/core/src/main/java/de/danoeh/antennapod/core/event/DownloadEvent.java
@@ -0,0 +1,28 @@
+package de.danoeh.antennapod.core.event;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import de.danoeh.antennapod.core.service.download.Downloader;
+
+public class DownloadEvent {
+
+    public final DownloaderUpdate update;
+
+    private DownloadEvent(DownloaderUpdate downloader) {
+        this.update = downloader;
+    }
+
+    public static DownloadEvent refresh(List<Downloader> list) {
+        list = new ArrayList<>(list);
+        DownloaderUpdate update = new DownloaderUpdate(list);
+        return new DownloadEvent(update);
+    }
+
+    @Override
+    public String toString() {
+        return "DownloadEvent{" +
+                "update=" + update +
+                '}';
+    }
+}
diff --git a/core/src/main/java/de/danoeh/antennapod/core/event/DownloaderUpdate.java b/core/src/main/java/de/danoeh/antennapod/core/event/DownloaderUpdate.java
new file mode 100644
index 000000000..dcb033267
--- /dev/null
+++ b/core/src/main/java/de/danoeh/antennapod/core/event/DownloaderUpdate.java
@@ -0,0 +1,53 @@
+package de.danoeh.antennapod.core.event;
+
+import java.util.Arrays;
+import java.util.List;
+
+import de.danoeh.antennapod.core.feed.Feed;
+import de.danoeh.antennapod.core.feed.FeedMedia;
+import de.danoeh.antennapod.core.service.download.Downloader;
+import de.danoeh.antennapod.core.util.LongList;
+
+public class DownloaderUpdate {
+
+    /* Downloaders that are currently running */
+    public final List<Downloader> downloaders;
+
+    /**
+     * IDs of feeds that are currently being downloaded
+     * Often used to show some progress wheel in the action bar
+     */
+    public final long[] feedIds;
+
+    /**
+     * IDs of feed media that are currently being downloaded
+     * Can be used to show and update download progress bars
+     */
+    public final long[] mediaIds;
+
+    public DownloaderUpdate(List<Downloader> downloaders) {
+        this.downloaders = downloaders;
+        LongList feedIds1 = new LongList(), mediaIds1 = new LongList();
+        for(Downloader d1 : downloaders) {
+            int type = d1.getDownloadRequest().getFeedfileType();
+            long id = d1.getDownloadRequest().getFeedfileId();
+            if(type == Feed.FEEDFILETYPE_FEED) {
+                feedIds1.add(id);
+            } else if(type == FeedMedia.FEEDFILETYPE_FEEDMEDIA) {
+                mediaIds1.add(id);
+            }
+        }
+
+        this.feedIds = feedIds1.toArray();
+        this.mediaIds = mediaIds1.toArray();
+    }
+
+    @Override
+    public String toString() {
+        return "DownloaderUpdate{" +
+                "downloaders=" + downloaders +
+                ", feedIds=" + Arrays.toString(feedIds) +
+                ", mediaIds=" + Arrays.toString(mediaIds) +
+                '}';
+    }
+}
diff --git a/core/src/main/java/de/danoeh/antennapod/core/event/FeedItemEvent.java b/core/src/main/java/de/danoeh/antennapod/core/event/FeedItemEvent.java
index a4a79187e..7ff241456 100644
--- a/core/src/main/java/de/danoeh/antennapod/core/event/FeedItemEvent.java
+++ b/core/src/main/java/de/danoeh/antennapod/core/event/FeedItemEvent.java
@@ -6,6 +6,7 @@ import android.support.annotation.NonNull;
 import org.apache.commons.lang3.builder.ToStringBuilder;
 import org.apache.commons.lang3.builder.ToStringStyle;
 
+import java.util.Arrays;
 import java.util.List;
 
 import de.danoeh.antennapod.core.feed.FeedItem;
@@ -32,6 +33,10 @@ public class FeedItemEvent {
         return new FeedItemEvent(Action.UPDATE, items);
     }
 
+    public static FeedItemEvent updated(FeedItem... items) {
+        return new FeedItemEvent(Action.UPDATE, Arrays.asList(items));
+    }
+
     @Override
     public String toString() {
         return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE)
diff --git a/core/src/main/java/de/danoeh/antennapod/core/feed/EventDistributor.java b/core/src/main/java/de/danoeh/antennapod/core/feed/EventDistributor.java
index 2667a2e12..3425e8a8e 100644
--- a/core/src/main/java/de/danoeh/antennapod/core/feed/EventDistributor.java
+++ b/core/src/main/java/de/danoeh/antennapod/core/feed/EventDistributor.java
@@ -26,7 +26,6 @@ public class EventDistributor extends Observable {
 	public static final int UNREAD_ITEMS_UPDATE = 2;
 	public static final int DOWNLOADLOG_UPDATE = 8;
 	public static final int PLAYBACK_HISTORY_UPDATE = 16;
-	public static final int DOWNLOAD_QUEUED = 32;
 	public static final int DOWNLOAD_HANDLED = 64;
     public static final int PLAYER_STATUS_UPDATE = 128;
 
@@ -88,10 +87,6 @@ public class EventDistributor extends Observable {
         Validate.isInstanceOf(EventListener.class, observer);
 	}
 
-	public void sendDownloadQueuedBroadcast() {
-		addEvent(DOWNLOAD_QUEUED);
-	}
-
 	public void sendUnreadItemsUpdateBroadcast() {
 		addEvent(UNREAD_ITEMS_UPDATE);
 	}
@@ -108,10 +103,6 @@ public class EventDistributor extends Observable {
 		addEvent(DOWNLOADLOG_UPDATE);
 	}
 
-	public void sendDownloadHandledBroadcast() {
-		addEvent(DOWNLOAD_HANDLED);
-	}
-
     public void sendPlayerStatusUpdateBroadcast() { addEvent(PLAYER_STATUS_UPDATE); }
 
 	public static abstract class EventListener implements Observer {
diff --git a/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadService.java b/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadService.java
index 0698107a7..505061ec4 100644
--- a/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadService.java
+++ b/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadService.java
@@ -53,7 +53,8 @@ import javax.xml.parsers.ParserConfigurationException;
 
 import de.danoeh.antennapod.core.ClientConfig;
 import de.danoeh.antennapod.core.R;
-import de.danoeh.antennapod.core.feed.EventDistributor;
+import de.danoeh.antennapod.core.event.DownloadEvent;
+import de.danoeh.antennapod.core.event.FeedItemEvent;
 import de.danoeh.antennapod.core.feed.Feed;
 import de.danoeh.antennapod.core.feed.FeedImage;
 import de.danoeh.antennapod.core.feed.FeedItem;
@@ -74,6 +75,7 @@ import de.danoeh.antennapod.core.syndication.handler.UnsupportedFeedtypeExceptio
 import de.danoeh.antennapod.core.util.ChapterUtils;
 import de.danoeh.antennapod.core.util.DownloadError;
 import de.danoeh.antennapod.core.util.InvalidFeedException;
+import de.greenrobot.event.EventBus;
 
 /**
  * Manages the download of feedfiles in the app. Downloads can be enqueued viathe startService intent.
@@ -102,12 +104,6 @@ public class DownloadService extends Service {
     public static final String EXTRA_DOWNLOAD_URL = "downloadUrl";
 
     /**
-     * Sent by the DownloadService when the content of the downloads list
-     * changes.
-     */
-    public static final String ACTION_DOWNLOADS_CONTENT_CHANGED = "action.de.danoeh.antennapod.core.service.downloadsContentChanged";
-
-    /**
      * Extra for ACTION_ENQUEUE_DOWNLOAD intent.
      */
     public static final String EXTRA_REQUEST = "request";
@@ -155,6 +151,8 @@ public class DownloadService extends Service {
     private static final int SCHED_EX_POOL_SIZE = 1;
     private ScheduledThreadPoolExecutor schedExecutor;
 
+    private Handler postHandler = new Handler();
+
     private final IBinder mBinder = new LocalBinder();
 
     public class LocalBinder extends Binder {
@@ -180,10 +178,7 @@ public class DownloadService extends Service {
                     final int type = status.getFeedfileType();
                     if (successful) {
                         if (type == Feed.FEEDFILETYPE_FEED) {
-                            handleCompletedFeedDownload(downloader
-                                    .getDownloadRequest());
-                        } else if (type == FeedImage.FEEDFILETYPE_FEEDIMAGE) {
-                            handleCompletedImageDownload(status, downloader.getDownloadRequest());
+                            handleCompletedFeedDownload(downloader.getDownloadRequest());
                         } else if (type == FeedMedia.FEEDFILETYPE_FEEDMEDIA) {
                             handleCompletedFeedMediaDownload(status, downloader.getDownloadRequest());
                         }
@@ -202,9 +197,22 @@ public class DownloadService extends Service {
                                 Log.e(TAG, "Download failed");
                                 saveDownloadStatus(status);
                                 handleFailedDownload(status, downloader.getDownloadRequest());
+
+                                // to make lists reload the failed item, we fake an item update
+                                if(type == FeedMedia.FEEDFILETYPE_FEEDMEDIA) {
+                                    long id = status.getFeedfileId();
+                                    FeedMedia media = DBReader.getFeedMedia(id);
+                                    EventBus.getDefault().post(FeedItemEvent.updated(media.getItem()));
+                                }
+                            }
+                        } else {
+                            // if FeedMedia download has been canceled, fake FeedItem update
+                            // so that lists reload that it
+                            if(status.getFeedfileType() == FeedMedia.FEEDFILETYPE_FEEDMEDIA) {
+                                FeedMedia media = DBReader.getFeedMedia(status.getFeedfileId());
+                                EventBus.getDefault().post(FeedItemEvent.updated(media.getItem()));
                             }
                         }
-                        sendDownloadHandledIntent();
                         queryDownloadsAsync();
                     }
                 } catch (InterruptedException e) {
@@ -306,6 +314,9 @@ public class DownloadService extends Service {
             updateReport();
         }
 
+        postHandler.removeCallbacks(postDownloaderTask);
+        EventBus.getDefault().postSticky(DownloadEvent.refresh(Collections.emptyList()));
+
         stopForeground(true);
         NotificationManager nm = (NotificationManager) getSystemService(NOTIFICATION_SERVICE);
         nm.cancel(NOTIFICATION_ID);
@@ -407,15 +418,14 @@ public class DownloadService extends Service {
                 } else {
                     Log.e(TAG, "Could not cancel download with url " + url);
                 }
-                sendBroadcast(new Intent(ACTION_DOWNLOADS_CONTENT_CHANGED));
+                postDownloaders();
 
             } else if (StringUtils.equals(intent.getAction(), ACTION_CANCEL_ALL_DOWNLOADS)) {
                 for (Downloader d : downloads) {
                     d.cancel();
                     Log.d(TAG, "Cancelled all downloads");
                 }
-                sendBroadcast(new Intent(ACTION_DOWNLOADS_CONTENT_CHANGED));
-
+                postDownloaders();
             }
             queryDownloads();
         }
@@ -434,13 +444,14 @@ public class DownloadService extends Service {
         if (downloader != null) {
             numberOfDownloads.incrementAndGet();
             // smaller rss feeds before bigger media files
-            if(request.getFeedfileId() == Feed.FEEDFILETYPE_FEED) {
+            if(request.getFeedfileType() == Feed.FEEDFILETYPE_FEED) {
                 downloads.add(0, downloader);
             } else {
                 downloads.add(downloader);
             }
             downloadExecutor.submit(downloader);
-            sendBroadcast(new Intent(ACTION_DOWNLOADS_CONTENT_CHANGED));
+
+            postDownloaders();
         }
 
         queryDownloads();
@@ -471,7 +482,7 @@ public class DownloadService extends Service {
                 boolean rc = downloads.remove(d);
                 Log.d(TAG, "Result of downloads.remove: " + rc);
                 DownloadRequester.getInstance().removeDownload(d.getDownloadRequest());
-                sendBroadcast(new Intent(ACTION_DOWNLOADS_CONTENT_CHANGED));
+                postDownloaders();
             }
         });
     }
@@ -487,10 +498,6 @@ public class DownloadService extends Service {
         DBWriter.addDownloadStatus(status);
     }
 
-    private void sendDownloadHandledIntent() {
-        EventDistributor.getInstance().sendDownloadHandledBroadcast();
-    }
-
     /**
      * Creates a notification at the end of the service lifecycle to notify the
      * user about the number of completed downloads. A report will only be
@@ -767,8 +774,6 @@ public class DownloadService extends Service {
                             numberOfDownloads.decrementAndGet();
                         }
 
-                        sendDownloadHandledIntent();
-
                         queryDownloadsAsync();
                     }
                 });
@@ -1039,7 +1044,6 @@ public class DownloadService extends Service {
             image.setDownloaded(true);
 
             saveDownloadStatus(status);
-            sendDownloadHandledIntent();
             DBWriter.setFeedImage(image);
             numberOfDownloads.decrementAndGet();
             queryDownloadsAsync();
@@ -1064,8 +1068,7 @@ public class DownloadService extends Service {
 
         @Override
         public void run() {
-            FeedMedia media = DBReader.getFeedMedia(
-                    request.getFeedfileId());
+            FeedMedia media = DBReader.getFeedMedia(request.getFeedfileId());
             if (media == null) {
                 throw new IllegalStateException(
                         "Could not find downloaded media object in database");
@@ -1121,7 +1124,6 @@ public class DownloadService extends Service {
             }
 
             saveDownloadStatus(status);
-            sendDownloadHandledIntent();
 
             if(GpodnetPreferences.loggedIn()) {
                 FeedItem item = media.getItem();
@@ -1174,16 +1176,24 @@ public class DownloadService extends Service {
         }
     }
 
-    public List<Downloader> getDownloads() {
-        if (downloads == null) {
-            // this is unusual, but it should be OK, we'll return
-            // an empty list to make it easy for people
-            return new ArrayList<Downloader>();
+
+    private long lastPost = 0;
+
+    final Runnable postDownloaderTask = new Runnable() {
+        @Override
+        public void run() {
+            List<Downloader> list = Collections.unmodifiableList(downloads);
+            EventBus.getDefault().postSticky(DownloadEvent.refresh(list));
+            postHandler.postDelayed(postDownloaderTask, 1500);
         }
+    };
 
-        // return a copy of downloads, but the copy doesn't need to be synchronized.
-        synchronized (downloads) {
-            return new ArrayList<Downloader>(downloads);
+    private void postDownloaders() {
+        long now = System.currentTimeMillis();
+        if(now - lastPost >= 250) {
+            postHandler.removeCallbacks(postDownloaderTask);
+            postDownloaderTask.run();
+            lastPost = now;
         }
     }
 
diff --git a/core/src/main/java/de/danoeh/antennapod/core/storage/DBWriter.java b/core/src/main/java/de/danoeh/antennapod/core/storage/DBWriter.java
index 14683506e..49a62da8c 100644
--- a/core/src/main/java/de/danoeh/antennapod/core/storage/DBWriter.java
+++ b/core/src/main/java/de/danoeh/antennapod/core/storage/DBWriter.java
@@ -766,6 +766,7 @@ public class DBWriter {
             adapter.open();
             adapter.setSingleFeedItem(item);
             adapter.close();
+            EventBus.getDefault().post(FeedItemEvent.updated(item));
         });
     }
 
diff --git a/core/src/main/java/de/danoeh/antennapod/core/storage/DownloadRequester.java b/core/src/main/java/de/danoeh/antennapod/core/storage/DownloadRequester.java
index b13f7a0cb..318060abc 100644
--- a/core/src/main/java/de/danoeh/antennapod/core/storage/DownloadRequester.java
+++ b/core/src/main/java/de/danoeh/antennapod/core/storage/DownloadRequester.java
@@ -15,7 +15,6 @@ import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 
 import de.danoeh.antennapod.core.BuildConfig;
-import de.danoeh.antennapod.core.feed.EventDistributor;
 import de.danoeh.antennapod.core.feed.Feed;
 import de.danoeh.antennapod.core.feed.FeedFile;
 import de.danoeh.antennapod.core.feed.FeedMedia;
@@ -85,7 +84,7 @@ public class DownloadRequester {
         Intent launchIntent = new Intent(context, DownloadService.class);
         launchIntent.putExtra(DownloadService.EXTRA_REQUEST, request);
         context.startService(launchIntent);
-        EventDistributor.getInstance().sendDownloadQueuedBroadcast();
+
         return true;
     }
 
diff --git a/core/src/main/java/de/danoeh/antennapod/core/util/LongList.java b/core/src/main/java/de/danoeh/antennapod/core/util/LongList.java
index 10ffd4bec..6ed8b820e 100644
--- a/core/src/main/java/de/danoeh/antennapod/core/util/LongList.java
+++ b/core/src/main/java/de/danoeh/antennapod/core/util/LongList.java
@@ -177,6 +177,28 @@ public final class LongList {
     }
 
     /**
+     * Removes values from this list.
+     *
+     * @param values  values to remove
+     */
+    public void removeAll(long[] values) {
+        for(long value : values) {
+            remove(value);
+        }
+    }
+
+    /**
+     * Removes values from this list.
+     *
+     * @param list List with values to remove
+     */
+    public void removeAll(LongList list) {
+        for(long value : list.values) {
+            remove(value);
+        }
+    }
+
+    /**
      * Removes an element at a given index, shifting elements at greater
      * indicies down one.
      *