From bfb12bc7c170e5d09958aebe44eeacb0437674ab Mon Sep 17 00:00:00 2001 From: len Date: Sun, 24 Apr 2016 23:32:49 +0200 Subject: [PATCH] Minor changes to fix a possible crash in the downloads view --- .../tachiyomi/ui/download/DownloadFragment.kt | 104 ++++++++++++-- .../tachiyomi/ui/download/DownloadHolder.kt | 2 +- .../ui/download/DownloadPresenter.kt | 132 ++---------------- 3 files changed, 108 insertions(+), 130 deletions(-) diff --git a/app/src/main/java/eu/kanade/tachiyomi/ui/download/DownloadFragment.kt b/app/src/main/java/eu/kanade/tachiyomi/ui/download/DownloadFragment.kt index 9212b83b6..ff0a0738e 100644 --- a/app/src/main/java/eu/kanade/tachiyomi/ui/download/DownloadFragment.kt +++ b/app/src/main/java/eu/kanade/tachiyomi/ui/download/DownloadFragment.kt @@ -10,7 +10,13 @@ import eu.kanade.tachiyomi.ui.main.MainActivity import eu.kanade.tachiyomi.widget.NpaLinearLayoutManager import kotlinx.android.synthetic.main.fragment_download_queue.* import nucleus.factory.RequiresPresenter +import rx.Observable import rx.Subscription +import rx.android.schedulers.AndroidSchedulers +import rx.schedulers.Schedulers +import rx.subscriptions.CompositeSubscription +import java.util.* +import java.util.concurrent.TimeUnit /** * Fragment that shows the currently active downloads. @@ -40,9 +46,14 @@ class DownloadFragment : BaseRxFragment() { private var clearButton: MenuItem? = null /** - * Subscription to know if the download queue is running. + * Subscription list to be cleared during [onPause]. */ - private var queueStatusSubscription: Subscription? = null + private val resumeSubscriptions by lazy { CompositeSubscription() } + + /** + * Map of subscriptions for active downloads. + */ + private val progressSubscriptions by lazy { HashMap() } /** * Whether the download queue is running or not. @@ -66,8 +77,7 @@ class DownloadFragment : BaseRxFragment() { setHasOptionsMenu(true) } - override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, - savedState: Bundle?): View? { + override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedState: Bundle?): View { return inflater.inflate(R.layout.fragment_download_queue, container, false) } @@ -123,15 +133,91 @@ class DownloadFragment : BaseRxFragment() { override fun onResume() { super.onResume() - queueStatusSubscription = presenter.downloadManager.runningSubject + presenter.downloadManager.runningSubject + .observeOn(AndroidSchedulers.mainThread()) .subscribe { onQueueStatusChange(it) } + .apply { resumeSubscriptions.add(this) } + + presenter.getStatusObservable() + .observeOn(AndroidSchedulers.mainThread()) + .subscribe { onStatusChange(it) } + .apply { resumeSubscriptions.add(this) } + + presenter.getProgressObservable() + .observeOn(AndroidSchedulers.mainThread()) + .subscribe { onUpdateDownloadedPages(it) } + .apply { resumeSubscriptions.add(this) } } override fun onPause() { - queueStatusSubscription?.unsubscribe() + for (subscription in progressSubscriptions.values) { + subscription.unsubscribe() + } + progressSubscriptions.clear() + resumeSubscriptions.clear() super.onPause() } + /** + * Called when the status of a download changes. + * + * @param download the download whose status has changed. + */ + private fun onStatusChange(download: Download) { + when (download.status) { + Download.DOWNLOADING -> { + observeProgress(download) + // Initial update of the downloaded pages + onUpdateDownloadedPages(download) + } + Download.DOWNLOADED -> { + unsubscribeProgress(download) + onUpdateProgress(download) + onUpdateDownloadedPages(download) + } + Download.ERROR -> unsubscribeProgress(download) + } + } + + /** + * Observe the progress of a download and notify the view. + * + * @param download the download to observe its progress. + */ + private fun observeProgress(download: Download) { + val subscription = Observable.interval(50, TimeUnit.MILLISECONDS, Schedulers.newThread()) + // Get the sum of percentages for all the pages. + .flatMap { + Observable.from(download.pages) + .map { it.progress } + .reduce { x, y -> x + y } + } + // Keep only the latest emission to avoid backpressure. + .onBackpressureLatest() + .observeOn(AndroidSchedulers.mainThread()) + .subscribe { progress -> + // Update the view only if the progress has changed. + if (download.totalProgress != progress) { + download.totalProgress = progress + onUpdateProgress(download) + } + } + + // Avoid leaking subscriptions + progressSubscriptions.remove(download)?.unsubscribe() + + progressSubscriptions.put(download, subscription) + } + + /** + * Unsubscribes the given download from the progress subscriptions. + * + * @param download the download to unsubscribe. + */ + private fun unsubscribeProgress(download: Download) { + progressSubscriptions.remove(download)?.unsubscribe() + } + /** * Called when the queue's status has changed. Updates the visibility of the buttons. * @@ -157,16 +243,16 @@ class DownloadFragment : BaseRxFragment() { } /** - * Called from the presenter when the status of a download changes. + * Called when the progress of a download changes. * - * @param download the download whose status has changed. + * @param download the download whose progress has changed. */ fun onUpdateProgress(download: Download) { getHolder(download)?.notifyProgress() } /** - * Called from the presenter when a page of a download is downloaded. + * Called when a page of a download is downloaded. * * @param download the download whose page has been downloaded. */ diff --git a/app/src/main/java/eu/kanade/tachiyomi/ui/download/DownloadHolder.kt b/app/src/main/java/eu/kanade/tachiyomi/ui/download/DownloadHolder.kt index 75ab822ca..a78c40245 100644 --- a/app/src/main/java/eu/kanade/tachiyomi/ui/download/DownloadHolder.kt +++ b/app/src/main/java/eu/kanade/tachiyomi/ui/download/DownloadHolder.kt @@ -10,7 +10,7 @@ import kotlinx.android.synthetic.main.item_download.view.* * All the elements from the layout file "item_download" are available in this class. * * @param view the inflated view for this holder. - * @constructor creates a new library holder. + * @constructor creates a new download holder. */ class DownloadHolder(private val view: View) : RecyclerView.ViewHolder(view) { diff --git a/app/src/main/java/eu/kanade/tachiyomi/ui/download/DownloadPresenter.kt b/app/src/main/java/eu/kanade/tachiyomi/ui/download/DownloadPresenter.kt index 509ffc8ad..46ca86bda 100644 --- a/app/src/main/java/eu/kanade/tachiyomi/ui/download/DownloadPresenter.kt +++ b/app/src/main/java/eu/kanade/tachiyomi/ui/download/DownloadPresenter.kt @@ -6,12 +6,7 @@ import eu.kanade.tachiyomi.data.download.model.Download import eu.kanade.tachiyomi.data.download.model.DownloadQueue import eu.kanade.tachiyomi.ui.base.presenter.BasePresenter import rx.Observable -import rx.Subscription -import rx.android.schedulers.AndroidSchedulers -import rx.schedulers.Schedulers import timber.log.Timber -import java.util.* -import java.util.concurrent.TimeUnit import javax.inject.Inject /** @@ -19,6 +14,13 @@ import javax.inject.Inject */ class DownloadPresenter : BasePresenter() { + companion object { + /** + * Id of the restartable that returns the download queue. + */ + const val GET_DOWNLOAD_QUEUE = 1 + } + /** * Download manager. */ @@ -30,28 +32,6 @@ class DownloadPresenter : BasePresenter() { val downloadQueue: DownloadQueue get() = downloadManager.queue - /** - * Map of subscriptions for active downloads. - */ - private val progressSubscriptions by lazy { HashMap() } - - /** - * Subscription for status changes on downloads. - */ - private var statusSubscription: Subscription? = null - - /** - * Subscription for downloaded pages for active downloads. - */ - private var pageProgressSubscription: Subscription? = null - - companion object { - /** - * Id of the restartable that returns the download queue. - */ - const val GET_DOWNLOAD_QUEUE = 1 - } - override fun onCreate(savedState: Bundle?) { super.onCreate(savedState) @@ -65,102 +45,14 @@ class DownloadPresenter : BasePresenter() { } } - override fun onTakeView(view: DownloadFragment) { - super.onTakeView(view) - - statusSubscription = downloadQueue.getStatusObservable() + fun getStatusObservable(): Observable { + return downloadQueue.getStatusObservable() .startWith(downloadQueue.getActiveDownloads()) - .observeOn(AndroidSchedulers.mainThread()) - .subscribe { processStatus(it, view) } + } - add(statusSubscription) - - pageProgressSubscription = downloadQueue.getProgressObservable() + fun getProgressObservable(): Observable { + return downloadQueue.getProgressObservable() .onBackpressureBuffer() - .observeOn(AndroidSchedulers.mainThread()) - .subscribe { view.onUpdateDownloadedPages(it) } - - add(pageProgressSubscription) - } - - override fun onDropView() { - destroySubscriptions() - super.onDropView() - } - - /** - * Process the status of a download when its status has changed and notify the view. - * - * @param download the download whose status has changed. - * @param view the view. - */ - private fun processStatus(download: Download, view: DownloadFragment) { - when (download.status) { - Download.DOWNLOADING -> { - observeProgress(download, view) - // Initial update of the downloaded pages - view.onUpdateDownloadedPages(download) - } - Download.DOWNLOADED -> { - unsubscribeProgress(download) - view.onUpdateProgress(download) - view.onUpdateDownloadedPages(download) - } - Download.ERROR -> unsubscribeProgress(download) - } - } - - /** - * Observe the progress of a download and notify the view. - * - * @param download the download to observe its progress. - * @param view the view. - */ - private fun observeProgress(download: Download, view: DownloadFragment) { - val subscription = Observable.interval(50, TimeUnit.MILLISECONDS, Schedulers.newThread()) - // Get the sum of percentages for all the pages. - .flatMap { - Observable.from(download.pages) - .map { it.progress } - .reduce { x, y -> x + y } - } - // Keep only the latest emission to avoid backpressure. - .onBackpressureLatest() - .observeOn(AndroidSchedulers.mainThread()) - .subscribe { progress -> - // Update the view only if the progress has changed. - if (download.totalProgress != progress) { - download.totalProgress = progress - view.onUpdateProgress(download) - } - } - - // Avoid leaking subscriptions - progressSubscriptions.remove(download)?.unsubscribe() - - progressSubscriptions.put(download, subscription) - } - - /** - * Unsubscribes the given download from the progress subscriptions. - * - * @param download the download to unsubscribe. - */ - private fun unsubscribeProgress(download: Download) { - progressSubscriptions.remove(download)?.unsubscribe() - } - - /** - * Destroys all the subscriptions of the presenter. - */ - private fun destroySubscriptions() { - for (subscription in progressSubscriptions.values) { - subscription.unsubscribe() - } - progressSubscriptions.clear() - - remove(pageProgressSubscription) - remove(statusSubscription) } /**