From d38058497ec88020d401e00ca3eff8121e63c52f Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Tue, 29 Mar 2016 11:52:17 +0200 Subject: [PATCH 1/4] move ApkDownloader.EXTRA_URL to Intent.setDataString() This is part of the move to standardizing all internal broadcasts to use the Intent's Uri field as the standard place for a download URL, and then using that in IntentFilters to do matching. --- app/src/main/java/org/fdroid/fdroid/net/ApkDownloader.java | 4 ++-- .../main/java/org/fdroid/fdroid/views/swap/SwapAppsView.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/net/ApkDownloader.java b/app/src/main/java/org/fdroid/fdroid/net/ApkDownloader.java index 0fa6811b6..a8c76093c 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/ApkDownloader.java +++ b/app/src/main/java/org/fdroid/fdroid/net/ApkDownloader.java @@ -22,6 +22,7 @@ package org.fdroid.fdroid.net; import android.content.Context; import android.content.Intent; +import android.net.Uri; import android.os.Bundle; import android.support.annotation.NonNull; import android.support.v4.content.LocalBroadcastManager; @@ -54,7 +55,6 @@ public class ApkDownloader implements AsyncDownloader.Listener { public static final String EVENT_ERROR = "apkDownloadError"; public static final String ACTION_STATUS = "apkDownloadStatus"; - public static final String EXTRA_URL = "apkDownloadUrl"; public static final int ERROR_HASH_MISMATCH = 101; @@ -224,8 +224,8 @@ public class ApkDownloader implements AsyncDownloader.Listener { } Intent intent = new Intent(ACTION_STATUS); + intent.setData(Uri.parse(Utils.getApkUrl(repoAddress, curApk))); intent.putExtras(event.getData()); - intent.putExtra(EXTRA_URL, Utils.getApkUrl(repoAddress, curApk)); LocalBroadcastManager.getInstance(context).sendBroadcast(intent); } diff --git a/app/src/main/java/org/fdroid/fdroid/views/swap/SwapAppsView.java b/app/src/main/java/org/fdroid/fdroid/views/swap/SwapAppsView.java index 664884a20..b13f4f1e5 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/swap/SwapAppsView.java +++ b/app/src/main/java/org/fdroid/fdroid/views/swap/SwapAppsView.java @@ -278,7 +278,7 @@ public class SwapAppsView extends ListView implements // once for each ViewHolder in order to get the repository address for the // apkToInstall. This way, we can wait until we receive an incoming intent (if // at all) and then lazily load the apk to install. - String broadcastUrl = intent.getStringExtra(ApkDownloader.EXTRA_URL); + String broadcastUrl = intent.getDataString(); if (TextUtils.equals(Utils.getApkUrl(apk.repoAddress, apk), broadcastUrl)) { resetView(); } From 91edad0c31a245cb0be773d1febd2f47280a14e6 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Tue, 29 Mar 2016 15:39:18 +0200 Subject: [PATCH 2/4] remove EVENT_DATA_ERROR_TYPE for DownloaderService reorg As part of the process of moving the APK downloading to an IntentService, I'm removing and incrementally reorganizing the existing events so that the code continues to be functional as it is reorganized. We might want to include more detail in a download error to expose to the user, but I think instead what will be more fruitful is to hide details on errors where there is nothing the user can do except retry. Then if there are errors where the user can do something about it, then F-Droid should instead offer them the option of doing that, and not just show an error message and walk away. --- .../java/org/fdroid/fdroid/AppDetails.java | 8 +------- .../org/fdroid/fdroid/ProgressListener.java | 4 ---- .../org/fdroid/fdroid/net/ApkDownloader.java | 18 ++++-------------- 3 files changed, 5 insertions(+), 25 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/AppDetails.java b/app/src/main/java/org/fdroid/fdroid/AppDetails.java index 6785e52ea..aa96ab0a8 100644 --- a/app/src/main/java/org/fdroid/fdroid/AppDetails.java +++ b/app/src/main/java/org/fdroid/fdroid/AppDetails.java @@ -985,14 +985,8 @@ public class AppDetails extends AppCompatActivity implements ProgressListener, A boolean finished = false; switch (event.type) { case ApkDownloader.EVENT_ERROR: - final int res; - if (event.getData().getInt(ApkDownloader.EVENT_DATA_ERROR_TYPE) == ApkDownloader.ERROR_HASH_MISMATCH) { - res = R.string.corrupt_download; - } else { - res = R.string.details_notinstalled; - } // this must be on the main UI thread - Toast.makeText(this, res, Toast.LENGTH_LONG).show(); + Toast.makeText(this, R.string.details_notinstalled, Toast.LENGTH_LONG).show(); cleanUpFinishedDownload(); finished = true; break; diff --git a/app/src/main/java/org/fdroid/fdroid/ProgressListener.java b/app/src/main/java/org/fdroid/fdroid/ProgressListener.java index a559ae141..b2f4f252b 100644 --- a/app/src/main/java/org/fdroid/fdroid/ProgressListener.java +++ b/app/src/main/java/org/fdroid/fdroid/ProgressListener.java @@ -29,10 +29,6 @@ public interface ProgressListener { this(type, NO_VALUE, NO_VALUE, null); } - public Event(String type, Bundle data) { - this(type, NO_VALUE, NO_VALUE, data); - } - public Event(String type, int progress, int total, Bundle data) { this.type = type; this.progress = progress; diff --git a/app/src/main/java/org/fdroid/fdroid/net/ApkDownloader.java b/app/src/main/java/org/fdroid/fdroid/net/ApkDownloader.java index a8c76093c..105c13fda 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/ApkDownloader.java +++ b/app/src/main/java/org/fdroid/fdroid/net/ApkDownloader.java @@ -27,10 +27,12 @@ import android.os.Bundle; import android.support.annotation.NonNull; import android.support.v4.content.LocalBroadcastManager; import android.util.Log; +import android.widget.Toast; import org.fdroid.fdroid.Hasher; import org.fdroid.fdroid.Preferences; import org.fdroid.fdroid.ProgressListener; +import org.fdroid.fdroid.R; import org.fdroid.fdroid.Utils; import org.fdroid.fdroid.compat.FileCompat; import org.fdroid.fdroid.data.Apk; @@ -56,16 +58,9 @@ public class ApkDownloader implements AsyncDownloader.Listener { public static final String ACTION_STATUS = "apkDownloadStatus"; - public static final int ERROR_HASH_MISMATCH = 101; - private static final String EVENT_SOURCE_ID = "sourceId"; private static long downloadIdCounter; - /** - * Used as a key to pass data through with an error event, explaining the type of event. - */ - public static final String EVENT_DATA_ERROR_TYPE = "apkDownloadErrorType"; - @NonNull private final App app; @NonNull private final Apk curApk; @NonNull private final Context context; @@ -208,12 +203,6 @@ public class ApkDownloader implements AsyncDownloader.Listener { sendProgressEvent(new ProgressListener.Event(type)); } - private void sendError(int errorType) { - Bundle data = new Bundle(1); - data.putInt(EVENT_DATA_ERROR_TYPE, errorType); - sendProgressEvent(new Event(EVENT_ERROR, data)); - } - // TODO: Completely remove progress listener, only use broadcasts... private void sendProgressEvent(Event event) { @@ -246,7 +235,8 @@ public class ApkDownloader implements AsyncDownloader.Listener { public void onDownloadComplete() { if (!verifyOrDelete(localFile)) { - sendError(ERROR_HASH_MISMATCH); + sendProgressEvent(new Event(EVENT_ERROR)); + Toast.makeText(context, R.string.corrupt_download, Toast.LENGTH_LONG).show(); return; } From 260b5bb9fb9af99d0e23af0fce7a0f7f65b5daa3 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Tue, 29 Mar 2016 17:41:27 +0200 Subject: [PATCH 3/4] remove unused AsyncDownloadWrapper.MSG_DATA This is all wired up, but the data is never ultimately used. --- .../java/org/fdroid/fdroid/net/ApkDownloader.java | 7 +++---- .../org/fdroid/fdroid/net/AsyncDownloadWrapper.java | 11 ++--------- .../java/org/fdroid/fdroid/net/AsyncDownloader.java | 2 +- 3 files changed, 6 insertions(+), 14 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/net/ApkDownloader.java b/app/src/main/java/org/fdroid/fdroid/net/ApkDownloader.java index 105c13fda..54b3bcc61 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/ApkDownloader.java +++ b/app/src/main/java/org/fdroid/fdroid/net/ApkDownloader.java @@ -23,7 +23,6 @@ package org.fdroid.fdroid.net; import android.content.Context; import android.content.Intent; import android.net.Uri; -import android.os.Bundle; import android.support.annotation.NonNull; import android.support.v4.content.LocalBroadcastManager; import android.util.Log; @@ -193,7 +192,8 @@ public class ApkDownloader implements AsyncDownloader.Listener { dlWrapper.download(); return true; } catch (IOException e) { - onErrorDownloading(e.getLocalizedMessage()); + e.printStackTrace(); + onErrorDownloading(); } return false; @@ -219,8 +219,7 @@ public class ApkDownloader implements AsyncDownloader.Listener { } @Override - public void onErrorDownloading(String localisedExceptionDetails) { - Log.e(TAG, "Download failed: " + localisedExceptionDetails); + public void onErrorDownloading() { delete(localFile); } diff --git a/app/src/main/java/org/fdroid/fdroid/net/AsyncDownloadWrapper.java b/app/src/main/java/org/fdroid/fdroid/net/AsyncDownloadWrapper.java index ff559e201..9d67c0b46 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/AsyncDownloadWrapper.java +++ b/app/src/main/java/org/fdroid/fdroid/net/AsyncDownloadWrapper.java @@ -1,6 +1,5 @@ package org.fdroid.fdroid.net; -import android.os.Bundle; import android.os.Handler; import android.os.Message; import android.util.Log; @@ -13,7 +12,6 @@ class AsyncDownloadWrapper extends Handler implements AsyncDownloader { private static final int MSG_DOWNLOAD_COMPLETE = 2; private static final int MSG_ERROR = 4; - private static final String MSG_DATA = "data"; private final Downloader downloader; private DownloadThread downloadThread; @@ -61,7 +59,7 @@ class AsyncDownloadWrapper extends Handler implements AsyncDownloader { listener.onDownloadComplete(); break; case MSG_ERROR: - listener.onErrorDownloading(message.getData().getString(MSG_DATA)); + listener.onErrorDownloading(); break; } } @@ -76,12 +74,7 @@ class AsyncDownloadWrapper extends Handler implements AsyncDownloader { // ignored } catch (IOException e) { Log.e(TAG, "I/O exception in download thread", e); - Bundle data = new Bundle(1); - data.putString(MSG_DATA, e.getLocalizedMessage()); - Message message = new Message(); - message.arg1 = MSG_ERROR; - message.setData(data); - AsyncDownloadWrapper.this.sendMessage(message); + sendMessage(MSG_ERROR); } } diff --git a/app/src/main/java/org/fdroid/fdroid/net/AsyncDownloader.java b/app/src/main/java/org/fdroid/fdroid/net/AsyncDownloader.java index 0c70895f9..0cf19d4b4 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/AsyncDownloader.java +++ b/app/src/main/java/org/fdroid/fdroid/net/AsyncDownloader.java @@ -5,7 +5,7 @@ import org.fdroid.fdroid.ProgressListener; public interface AsyncDownloader { interface Listener extends ProgressListener { - void onErrorDownloading(String localisedExceptionDetails); + void onErrorDownloading(); void onDownloadComplete(); } From 29788254ddde594ab96ba23d35dbc11053aacf96 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Tue, 29 Mar 2016 20:40:42 +0200 Subject: [PATCH 4/4] use background thread for swap icons instead of AsyncTask An AsyncTask ties into the UI thread for things like onPostExecute(). If it is run within an AsyncTask, then it freaks out because it can't tie into the UI thread. We don't need it to do that here anyway, so just use a plain Thread, and set the priority to background. --- .../fdroid/views/swap/SwapWorkflowActivity.java | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/views/swap/SwapWorkflowActivity.java b/app/src/main/java/org/fdroid/fdroid/views/swap/SwapWorkflowActivity.java index a3b521f21..711226362 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/swap/SwapWorkflowActivity.java +++ b/app/src/main/java/org/fdroid/fdroid/views/swap/SwapWorkflowActivity.java @@ -702,16 +702,13 @@ public class SwapWorkflowActivity extends AppCompatActivity { lrm.copyApksToRepo(); broadcast(TYPE_STATUS, getString(R.string.copying_icons)); // run the icon copy without progress, its not a blocker - // TODO: Fix lint error about this being run from a worker thread, says it should be - // run on a main thread. - new AsyncTask() { - + new Thread() { @Override - protected Void doInBackground(Void... params) { + public void run() { + android.os.Process.setThreadPriority(android.os.Process.THREAD_PRIORITY_BACKGROUND); lrm.copyIconsToRepo(); - return null; } - }.execute(); + }.start(); broadcast(TYPE_COMPLETE); } catch (Exception e) {