diff --git a/app/src/main/java/org/fdroid/fdroid/AppDetails.java b/app/src/main/java/org/fdroid/fdroid/AppDetails.java index a295724d0..149065263 100644 --- a/app/src/main/java/org/fdroid/fdroid/AppDetails.java +++ b/app/src/main/java/org/fdroid/fdroid/AppDetails.java @@ -426,8 +426,8 @@ public class AppDetails extends AppCompatActivity { @Override protected void onResumeFragments() { super.onResumeFragments(); + headerFragment = (AppDetailsHeaderFragment) getSupportFragmentManager().findFragmentById(R.id.header); refreshApkList(); - refreshHeader(); supportInvalidateOptionsMenu(); if (DownloaderService.isQueuedOrActive(activeDownloadUrlString)) { registerDownloaderReceivers(); @@ -564,7 +564,7 @@ public class AppDetails extends AppCompatActivity { @Override protected void onDestroy() { - cleanUpFinishedDownload(); + unregisterDownloaderReceivers(); super.onDestroy(); } @@ -615,8 +615,6 @@ public class AppDetails extends AppCompatActivity { } private void refreshHeader() { - headerFragment = (AppDetailsHeaderFragment) - getSupportFragmentManager().findFragmentById(R.id.header); if (headerFragment != null) { headerFragment.updateViews(); } @@ -1416,17 +1414,44 @@ public class AppDetails extends AppCompatActivity { public void onResume() { super.onResume(); updateViews(); + restoreProgressBarOnResume(); + } + + /** + * After resuming the fragment, decide whether or not we need to show the progress bar. + * Also, put an appropriate message depending on whether or not the download is active or + * just queued. + * + * NOTE: this can't be done in the `updateViews` method as it currently stands. The reason + * is because that method gets called all the time, for all sorts of reasons. The progress + * bar is updated with actual progress values in response to async broadcasts. If we always + * tried to force the progress bar in `updateViews`, it would override the values that were + * set by the async progress broadcasts. + */ + private void restoreProgressBarOnResume() { + if (appDetails.activeDownloadUrlString != null) { + // We don't actually know what the current progress is, so this will show an indeterminate + // progress bar until the first progress/complete event we receive. + final String message = DownloaderService.isQueued(appDetails.activeDownloadUrlString) + ? getString(R.string.download_pending) + : ""; + showIndeterminateProgress(message); + } } /** * Displays empty, indeterminate progress bar and related views. */ public void startProgress() { + showIndeterminateProgress(getString(R.string.download_pending)); + updateViews(); + } + + private void showIndeterminateProgress(String message) { setProgressVisible(true); progressBar.setIndeterminate(true); - progressSize.setText(""); + progressSize.setText(message); progressPercent.setText(""); - updateViews(); } /** diff --git a/app/src/main/java/org/fdroid/fdroid/net/DownloadCompleteService.java b/app/src/main/java/org/fdroid/fdroid/net/DownloadCompleteService.java index 6574a76fd..fa5029d94 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/DownloadCompleteService.java +++ b/app/src/main/java/org/fdroid/fdroid/net/DownloadCompleteService.java @@ -2,17 +2,14 @@ package org.fdroid.fdroid.net; import android.app.IntentService; import android.app.NotificationManager; -import android.app.PendingIntent; import android.content.Context; import android.content.Intent; import android.content.pm.PackageManager; import android.net.Uri; import android.os.Process; import android.support.v4.app.NotificationCompat; -import android.support.v4.app.TaskStackBuilder; import android.text.TextUtils; -import org.fdroid.fdroid.AppDetails; import org.fdroid.fdroid.R; import org.fdroid.fdroid.Utils; import org.fdroid.fdroid.data.App; @@ -42,12 +39,12 @@ public class DownloadCompleteService extends IntentService { if (intent != null) { final String action = intent.getAction(); if (!ACTION_NOTIFY.equals(action)) { - Utils.debugLog(TAG, "intent action is not ACTION_NOTIFY"); + Utils.debugLog(TAG, "Intent action is not ACTION_NOTIFY"); return; } String packageName = intent.getStringExtra(EXTRA_PACKAGE_NAME); if (TextUtils.isEmpty(packageName)) { - Utils.debugLog(TAG, "intent is missing EXTRA_PACKAGE_NAME"); + Utils.debugLog(TAG, "Intent is missing EXTRA_PACKAGE_NAME"); return; } @@ -64,22 +61,13 @@ public class DownloadCompleteService extends IntentService { title = String.format(getString(R.string.tap_to_install_format), app.name); } - Intent notifyIntent = new Intent(this, AppDetails.class); - notifyIntent.putExtra(AppDetails.EXTRA_APPID, packageName); - TaskStackBuilder stackBuilder = TaskStackBuilder - .create(this) - .addParentStack(AppDetails.class) - .addNextIntent(notifyIntent); int requestCode = Utils.getApkUrlNotificationId(intent.getDataString()); - PendingIntent pendingIntent = stackBuilder.getPendingIntent(requestCode, - PendingIntent.FLAG_UPDATE_CURRENT); - NotificationCompat.Builder builder = new NotificationCompat.Builder(this) .setAutoCancel(true) .setContentTitle(title) .setSmallIcon(android.R.drawable.stat_sys_download_done) - .setContentIntent(pendingIntent) + .setContentIntent(DownloaderService.createAppDetailsIntent(this, requestCode, packageName)) .setContentText(getString(R.string.tap_to_install)); NotificationManager nm = (NotificationManager) getSystemService(NOTIFICATION_SERVICE); nm.notify(Utils.getApkUrlNotificationId(intent.getDataString()), builder.build()); diff --git a/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java b/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java index 3d4bd7679..2d31a6574 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java +++ b/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java @@ -19,6 +19,7 @@ package org.fdroid.fdroid.net; import android.app.Notification; import android.app.NotificationManager; +import android.app.PendingIntent; import android.app.Service; import android.content.Context; import android.content.Intent; @@ -31,14 +32,21 @@ import android.os.Looper; import android.os.Message; import android.os.PatternMatcher; import android.os.Process; +import android.support.annotation.NonNull; +import android.support.annotation.Nullable; import android.support.v4.app.NotificationCompat; +import android.support.v4.app.TaskStackBuilder; import android.support.v4.content.LocalBroadcastManager; import android.text.TextUtils; import android.util.Log; +import org.fdroid.fdroid.AppDetails; +import org.fdroid.fdroid.FDroid; import org.fdroid.fdroid.Preferences; import org.fdroid.fdroid.R; import org.fdroid.fdroid.Utils; +import org.fdroid.fdroid.data.App; +import org.fdroid.fdroid.data.AppProvider; import org.fdroid.fdroid.data.SanitizedFile; import java.io.File; @@ -104,7 +112,7 @@ public class DownloaderService extends Service { @Override public void onCreate() { super.onCreate(); - Log.i(TAG, "onCreate"); + Utils.debugLog(TAG, "Creating downloader service."); HandlerThread thread = new HandlerThread(TAG, Process.THREAD_PRIORITY_BACKGROUND); thread.start(); @@ -117,60 +125,102 @@ public class DownloaderService extends Service { @Override public void onStart(Intent intent, int startId) { super.onStart(intent, startId); - Log.i(TAG, "onStart " + startId + " " + intent); + Utils.debugLog(TAG, "Received Intent for downloading: " + intent + " (with a startId of " + startId + ")"); String uriString = intent.getDataString(); if (uriString == null) { Log.e(TAG, "Received Intent with no URI: " + intent); return; } if (ACTION_CANCEL.equals(intent.getAction())) { - Log.i(TAG, "Removed " + intent); - Integer what = QUEUE_WHATS.remove(uriString); - if (what != null && serviceHandler.hasMessages(what)) { - // the URL is in the queue, remove it + Utils.debugLog(TAG, "Cancelling download of " + uriString); + if (isQueued(uriString)) { serviceHandler.removeMessages(what); - } else if (downloader != null && TextUtils.equals(uriString, downloader.sourceUrl.toString())) { - // the URL is being downloaded, cancel it + } else if (isActive(uriString)) { downloader.cancelDownload(); } else { - Log.e(TAG, "CANCEL called on something not queued or running: " + startId + " " + intent); + Log.e(TAG, "ACTION_CANCEL called on something not queued or running"); + } + + QUEUE_WHATS.remove(uriString); + if (isQueueEmpty()) { + stopForeground(true); } } else if (ACTION_QUEUE.equals(intent.getAction())) { - if (Preferences.get().isUpdateNotificationEnabled()) { - startForeground(NOTIFY_DOWNLOADING, createNotification(intent.getDataString()).build()); - } - Log.i(TAG, "Queued " + intent); Message msg = serviceHandler.obtainMessage(); msg.arg1 = startId; msg.obj = intent; msg.what = what++; serviceHandler.sendMessage(msg); - Log.i(TAG, "QUEUE_WHATS.put(" + uriString + ", " + msg.what); QUEUE_WHATS.put(uriString, msg.what); + Utils.debugLog(TAG, "Queued download of " + uriString + ". Now " + QUEUE_WHATS.size() + " downloads in the queue"); } else { Log.e(TAG, "Received Intent with unknown action: " + intent); } } - private NotificationCompat.Builder createNotification(String urlString) { + @Nullable + private static String getPackageNameFromIntent(@NonNull Intent intent) { + return intent.hasExtra(EXTRA_PACKAGE_NAME) ? intent.getStringExtra(EXTRA_PACKAGE_NAME) : null; + } + + private NotificationCompat.Builder createNotification(String urlString, @Nullable String packageName) { return new NotificationCompat.Builder(this) .setAutoCancel(true) - .setContentTitle(getString(R.string.downloading)) + .setContentIntent(createAppDetailsIntent(this, 0, packageName)) + .setContentTitle(getNotificationTitle(packageName)) .setSmallIcon(android.R.drawable.stat_sys_download) .setContentText(urlString) .setProgress(100, 0, true); } + /** + * If downloading an apk (i.e. <code>packageName != null</code>) then the title will indicate + * the name of the app which the apk belongs to. Otherwise, it will be a generic "Downloading..." + * message. + */ + private String getNotificationTitle(@Nullable String packageName) { + String title; + if (packageName != null) { + App app = AppProvider.Helper.findByPackageName( + getContentResolver(), packageName, new String[] {AppProvider.DataColumns.NAME}); + title = getString(R.string.downloading_apk, app.name); + } else { + title = getString(R.string.downloading); + } + return title; + } + + public static PendingIntent createAppDetailsIntent(@NonNull Context context, int requestCode, @Nullable String packageName) { + TaskStackBuilder stackBuilder; + if (packageName != null) { + Intent notifyIntent = new Intent(context.getApplicationContext(), AppDetails.class) + .putExtra(AppDetails.EXTRA_APPID, packageName); + + stackBuilder = TaskStackBuilder + .create(context.getApplicationContext()) + .addParentStack(AppDetails.class) + .addNextIntent(notifyIntent); + } else { + Intent notifyIntent = new Intent(context.getApplicationContext(), FDroid.class); + stackBuilder = TaskStackBuilder + .create(context.getApplicationContext()) + .addParentStack(FDroid.class) + .addNextIntent(notifyIntent); + } + + return stackBuilder.getPendingIntent(requestCode, PendingIntent.FLAG_UPDATE_CURRENT); + } + @Override public int onStartCommand(Intent intent, int flags, int startId) { onStart(intent, startId); - Log.i(TAG, "onStartCommand " + intent); + Utils.debugLog(TAG, "onStartCommand " + intent); return START_REDELIVER_INTENT; // if killed before completion, retry Intent } @Override public void onDestroy() { - Log.i(TAG, "onDestroy"); + Utils.debugLog(TAG, "Destroying downloader service. Will move to background and stop our Looper."); stopForeground(true); serviceLooper.quit(); //NOPMD - this is copied from IntentService, no super call needed } @@ -204,29 +254,39 @@ public class DownloaderService extends Service { File downloadDir = new File(Utils.getApkCacheDir(this), uri.getHost() + "-" + uri.getPort()); downloadDir.mkdirs(); final SanitizedFile localFile = new SanitizedFile(downloadDir, uri.getLastPathSegment()); + final String packageName = getPackageNameFromIntent(intent); sendBroadcast(uri, Downloader.ACTION_STARTED, localFile); + + if (Preferences.get().isUpdateNotificationEnabled()) { + Notification notification = createNotification(intent.getDataString(), getPackageNameFromIntent(intent)).build(); + startForeground(NOTIFY_DOWNLOADING, notification); + } + try { downloader = DownloaderFactory.create(this, uri, localFile); downloader.setListener(new Downloader.DownloaderProgressListener() { @Override public void sendProgress(URL sourceUrl, int bytesRead, int totalBytes) { - Intent intent = new Intent(Downloader.ACTION_PROGRESS); - intent.setData(uri); - intent.putExtra(Downloader.EXTRA_BYTES_READ, bytesRead); - intent.putExtra(Downloader.EXTRA_TOTAL_BYTES, totalBytes); - localBroadcastManager.sendBroadcast(intent); + if (isActive(uri.toString())) { + Intent intent = new Intent(Downloader.ACTION_PROGRESS); + intent.setData(uri); + intent.putExtra(Downloader.EXTRA_BYTES_READ, bytesRead); + intent.putExtra(Downloader.EXTRA_TOTAL_BYTES, totalBytes); + localBroadcastManager.sendBroadcast(intent); - NotificationManager nm = (NotificationManager) getSystemService(NOTIFICATION_SERVICE); - Notification notification = createNotification(uri.toString()) - .setProgress(totalBytes, bytesRead, false) - .build(); - nm.notify(NOTIFY_DOWNLOADING, notification); + if (Preferences.get().isUpdateNotificationEnabled()) { + NotificationManager nm = (NotificationManager) getSystemService(NOTIFICATION_SERVICE); + Notification notification = createNotification(uri.toString(), packageName) + .setProgress(totalBytes, bytesRead, false) + .build(); + nm.notify(NOTIFY_DOWNLOADING, notification); + } + } } }); downloader.download(); sendBroadcast(uri, Downloader.ACTION_COMPLETE, localFile); - DownloadCompleteService.notify(this, intent.getStringExtra(EXTRA_PACKAGE_NAME), - intent.getDataString()); + DownloadCompleteService.notify(this, packageName, intent.getDataString()); } catch (InterruptedException e) { sendBroadcast(uri, Downloader.ACTION_INTERRUPTED, localFile); } catch (IOException e) { @@ -237,6 +297,10 @@ public class DownloaderService extends Service { if (downloader != null) { downloader.close(); } + // May have already been removed in response to a cancel intent, but that wont cause + // problems if we ask to remove it again. + QUEUE_WHATS.remove(uri.toString()); + stopForeground(true); } downloader = null; } @@ -266,11 +330,11 @@ public class DownloaderService extends Service { * @see #cancel(Context, String) */ public static void queue(Context context, String packageName, String urlString) { - Log.i(TAG, "queue " + urlString); + Utils.debugLog(TAG, "Preparing " + urlString + " to go into the download queue"); Intent intent = new Intent(context, DownloaderService.class); intent.setAction(ACTION_QUEUE); intent.setData(Uri.parse(urlString)); - if (!TextUtils.isEmpty(EXTRA_PACKAGE_NAME)) { + if (!TextUtils.isEmpty(packageName)) { intent.putExtra(EXTRA_PACKAGE_NAME, packageName); } context.startService(intent); @@ -286,7 +350,7 @@ public class DownloaderService extends Service { * @see #queue(Context, String, String) */ public static void cancel(Context context, String urlString) { - Log.i(TAG, "cancel " + urlString); + Utils.debugLog(TAG, "Preparing cancellation of " + urlString + " download"); Intent intent = new Intent(context, DownloaderService.class); intent.setAction(ACTION_CANCEL); intent.setData(Uri.parse(urlString)); @@ -294,18 +358,36 @@ public class DownloaderService extends Service { } /** - * Check if a URL is waiting in the queue for downloading or if actively - * being downloaded. This is useful for checking whether to re-register - * {@link android.content.BroadcastReceiver}s in - * {@link android.app.Activity#onResume()} + * Check if a URL is waiting in the queue for downloading or if actively being downloaded. + * This is useful for checking whether to re-register {@link android.content.BroadcastReceiver}s + * in {@link android.app.Activity#onResume()}. + * @see DownloaderService#isQueued(String) + * @see DownloaderService#isActive(String) */ public static boolean isQueuedOrActive(String urlString) { + return isQueued(urlString) || isActive(urlString); + } + + public static boolean isQueueEmpty() { + return QUEUE_WHATS.isEmpty(); + } + + /** + * Check if a URL is waiting in the queue for downloading. + */ + public static boolean isQueued(String urlString) { if (TextUtils.isEmpty(urlString)) { return false; } Integer what = QUEUE_WHATS.get(urlString); - return (what != null && serviceHandler.hasMessages(what)) - || (downloader != null && TextUtils.equals(urlString, downloader.sourceUrl.toString())); + return what != null && serviceHandler.hasMessages(what); + } + + /** + * Check if a URL is actively being downloaded. + */ + public static boolean isActive(String urlString) { + return downloader != null && QUEUE_WHATS.containsKey(urlString) && TextUtils.equals(urlString, downloader.sourceUrl.toString()); } /** diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index e3969b4c6..20d0988a3 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -367,10 +367,12 @@ <string name="uninstall_confirm">Do you want to uninstall this app?</string> <string name="tap_to_install">Download completed, tap to install</string> <string name="download_error">Download unsuccessful</string> + <string name="download_pending">Waiting to start download…</string> <string name="perms_new_perm_prefix">New: </string> <string name="perms_description_app">Provided by %1$s.</string> <string name="downloading">Downloading…</string> + <string name="downloading_apk">Downloading %1$s</string> <string name="interval_never">Never</string> <string name="interval_1h">Hourly</string>