diff --git a/src/org/fdroid/fdroid/AppDetails.java b/src/org/fdroid/fdroid/AppDetails.java index e56e6ce79..fca4c0b10 100644 --- a/src/org/fdroid/fdroid/AppDetails.java +++ b/src/org/fdroid/fdroid/AppDetails.java @@ -433,13 +433,18 @@ public class AppDetails extends ListActivity implements ProgressListener { AppProvider.getContentUri(app.id), true, myAppObserver); - if (downloadHandler != null) { - downloadHandler.setProgressListener(this); + if (downloadHandler != null) { + if (downloadHandler.isComplete()) { + downloadCompleteInstallApk(); + } else { + downloadHandler.setProgressListener(this); - // Show the progress dialog, if for no other reason than to prevent them attempting - // to download again (i.e. we force them to touch 'cancel' before they can access - // the rest of the activity). - updateProgressDialog(); + // Show the progress dialog, if for no other reason than to prevent them attempting + // to download again (i.e. we force them to touch 'cancel' before they can access + // the rest of the activity). + Log.d(TAG, "Showing dialog to user after resuming app details view, because a download was previously in progress"); + updateProgressDialog(); + } } updateViews(); @@ -447,6 +452,29 @@ public class AppDetails extends ListActivity implements ProgressListener { MenuManager.create(this).invalidateOptionsMenu(); } + /** + * Remove progress listener, suppress progress dialog, set downloadHandler to null. + */ + private void cleanUpFinishedDownload() { + if (downloadHandler != null) { + downloadHandler.removeProgressListener(); + removeProgressDialog(); + downloadHandler = null; + } + } + + /** + * Once the download completes successfully, call this method to start the install process + * with the file that was downloaded. + */ + private void downloadCompleteInstallApk() { + if (downloadHandler != null) { + assert downloadHandler.isComplete(); + installApk(downloadHandler.localFile(), downloadHandler.getApk().id); + cleanUpFinishedDownload(); + } + } + @Override protected void onPause() { if (myAppObserver != null) { @@ -491,6 +519,7 @@ public class AppDetails extends ListActivity implements ProgressListener { if (downloadHandler != null) { if (!inProcessOfChangingConfiguration) { downloadHandler.cancel(); + cleanUpFinishedDownload(); } } inProcessOfChangingConfiguration = false; @@ -1024,6 +1053,7 @@ public class AppDetails extends ListActivity implements ProgressListener { updateProgressDialog(); } } + private void installApk(File file, String packageName) { setProgressBarIndeterminateVisibility(true); @@ -1120,7 +1150,7 @@ public class AppDetails extends ListActivity implements ProgressListener { Log.d(TAG, "User clicked 'cancel' on download, attempting to interrupt download thread."); if (downloadHandler != null) { downloadHandler.cancel(); - downloadHandler = null; + cleanUpFinishedDownload(); } else { Log.e(TAG, "Tried to cancel, but the downloadHandler doesn't exist."); } @@ -1175,6 +1205,18 @@ public class AppDetails extends ListActivity implements ProgressListener { @Override public void onProgress(Event event) { + if (downloadHandler == null || !downloadHandler.isEventFromThis(event)) { + // Choose not to respond to events from previous downloaders. + // We don't even care if we receive "cancelled" events or the like, because + // we dealt with cancellations in the onCancel listener of the dialog, + // rather than waiting to receive the event here. We try and be careful in + // the download thread to make sure that we check for cancellations before + // sending events, but it is not possible to be perfect, because the interruption + // which triggers the download can happen after the check to see if + Log.d(TAG, "Discarding downloader event \"" + event.type + "\" as it is from an old (probably cancelled) downloader."); + return; + } + boolean finished = false; if (event.type.equals(Downloader.EVENT_PROGRESS)) { updateProgressDialog(event.progress, event.total); @@ -1188,7 +1230,7 @@ public class AppDetails extends ListActivity implements ProgressListener { Toast.makeText(this, text, Toast.LENGTH_LONG).show(); finished = true; } else if (event.type.equals(ApkDownloader.EVENT_APK_DOWNLOAD_COMPLETE)) { - installApk(downloadHandler.localFile(), downloadHandler.getApk().id); + downloadCompleteInstallApk(); finished = true; } diff --git a/src/org/fdroid/fdroid/net/ApkDownloader.java b/src/org/fdroid/fdroid/net/ApkDownloader.java index 2dffe9507..baddfd5ef 100644 --- a/src/org/fdroid/fdroid/net/ApkDownloader.java +++ b/src/org/fdroid/fdroid/net/ApkDownloader.java @@ -48,6 +48,9 @@ public class ApkDownloader implements AsyncDownloadWrapper.Listener { public static final int ERROR_DOWNLOAD_FAILED = 102; public static final int ERROR_UNKNOWN = 103; + private static final String EVENT_SOURCE_ID = "sourceId"; + private static long downloadIdCounter = 0; + /** * Used as a key to pass data through with an error event, explaining the type of event. */ @@ -61,6 +64,9 @@ public class ApkDownloader implements AsyncDownloadWrapper.Listener { private AsyncDownloadWrapper dlWrapper = null; private int progress = 0; private int totalSize = 0; + private boolean isComplete = false; + + private long id = ++downloadIdCounter; public void setProgressListener(ProgressListener listener) { this.listener = listener; @@ -83,6 +89,15 @@ public class ApkDownloader implements AsyncDownloadWrapper.Listener { return localFile; } + /** + * When stopping/starting downloaders multiple times (on different threads), it can + * get weird whereby different threads are sending progress events. It is important + * to be able to see which downloader these progress events are coming from. + */ + public boolean isEventFromThis(Event event) { + return event.getData().containsKey(EVENT_SOURCE_ID) && event.getData().getLong(EVENT_SOURCE_ID) == id; + } + public String getRemoteAddress() { return repoAddress + "/" + curApk.apkName.replace(" ", "%20"); } @@ -131,6 +146,15 @@ public class ApkDownloader implements AsyncDownloadWrapper.Listener { } } + private void sendCompleteMessage() { + isComplete = true; + sendMessage(EVENT_APK_DOWNLOAD_COMPLETE); + } + + public boolean isComplete() { + return this.isComplete; + } + /** * If the download successfully spins up a new thread to start downloading, then we return * true, otherwise false. This is useful, e.g. when we use a cached version, and so don't @@ -140,7 +164,7 @@ public class ApkDownloader implements AsyncDownloadWrapper.Listener { // Can we use the cached version? if (verifyOrDeleteCachedVersion()) { - sendMessage(EVENT_APK_DOWNLOAD_COMPLETE); + sendCompleteMessage(); return false; } @@ -181,6 +205,8 @@ public class ApkDownloader implements AsyncDownloadWrapper.Listener { progress = event.progress; } + event.getData().putLong(EVENT_SOURCE_ID, id); + if (listener != null) { listener.onProgress(event); } @@ -214,7 +240,7 @@ public class ApkDownloader implements AsyncDownloadWrapper.Listener { } Log.d("FDroid", "Download finished: " + localFile); - sendMessage(EVENT_APK_DOWNLOAD_COMPLETE); + sendCompleteMessage(); } @Override @@ -232,7 +258,6 @@ public class ApkDownloader implements AsyncDownloadWrapper.Listener { * listener (to prevent */ public void cancel() { - removeProgressListener(); if (dlWrapper != null) { dlWrapper.attemptCancel(); }