diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml
index 66c0361e7..73972078c 100644
--- a/app/src/main/AndroidManifest.xml
+++ b/app/src/main/AndroidManifest.xml
@@ -471,7 +471,8 @@
android:permission="android.permission.BIND_JOB_SERVICE" />
* The canonical URL for the APK file to download is also used as the unique ID to * represent the download itself throughout F-Droid. This follows the model @@ -87,57 +81,55 @@ import static vendored.org.apache.commons.codec.digest.MessageDigestAlgorithms.S * @see forced to vendor Apache Commons Codec */ @SuppressWarnings("LineLength") -public class InstallManagerService extends Service { +public class InstallManagerService { private static final String TAG = "InstallManagerService"; - private static final String ACTION_INSTALL = "org.fdroid.fdroid.installer.action.INSTALL"; private static final String ACTION_CANCEL = "org.fdroid.fdroid.installer.action.CANCEL"; - private static final String EXTRA_APP = "org.fdroid.fdroid.installer.extra.APP"; - private static final String EXTRA_APK = "org.fdroid.fdroid.installer.extra.APK"; + @SuppressLint("StaticFieldLeak") // we are using ApplicationContext, so hopefully that's fine + private static InstallManagerService instance; - private static SharedPreferences pendingInstalls; + private final Context context; + private final LocalBroadcastManager localBroadcastManager; + private final AppUpdateStatusManager appUpdateStatusManager; - private LocalBroadcastManager localBroadcastManager; - private AppUpdateStatusManager appUpdateStatusManager; - private boolean running = false; - - /** - * This service does not use binding, so no need to implement this method - */ - @Override - public IBinder onBind(Intent intent) { - return null; + public static InstallManagerService getInstance(Context context) { + if (instance == null) { + instance = new InstallManagerService(context.getApplicationContext()); + } + return instance; } - @Override - public void onCreate() { - super.onCreate(); - localBroadcastManager = LocalBroadcastManager.getInstance(this); - appUpdateStatusManager = AppUpdateStatusManager.getInstance(this); - running = true; - pendingInstalls = getPendingInstalls(this); + public InstallManagerService(Context context) { + this.context = context; + this.localBroadcastManager = LocalBroadcastManager.getInstance(context); + this.appUpdateStatusManager = AppUpdateStatusManager.getInstance(context); + // cancel intent can't use LocalBroadcastManager, because it comes from system process + IntentFilter cancelFilter = new IntentFilter(); + cancelFilter.addAction(ACTION_CANCEL); + context.registerReceiver(new BroadcastReceiver() { + @Override + public void onReceive(Context context, Intent intent) { + Log.d(TAG, "Received cancel intent: " + intent); + if (!ACTION_CANCEL.equals(intent.getAction())) return; + cancel(context, intent.getStringExtra(DownloaderService.EXTRA_CANONICAL_URL)); + } + }, cancelFilter); } - /** - * If this {@link Service} is stopped, then all of the various - * {@link BroadcastReceiver}s need to unregister themselves if they get - * called. There can be multiple {@code BroadcastReceiver}s registered, - * so it can't be done with a simple call here. So {@link #running} is the - * signal to all the existing {@code BroadcastReceiver}s to unregister. - */ - @Override - public void onDestroy() { - running = false; - super.onDestroy(); + private void onCancel(String canonicalUrl) { + DownloaderService.cancel(canonicalUrl); + Apk apk = appUpdateStatusManager.getApk(canonicalUrl); + if (apk != null) { + Utils.debugLog(TAG, "also canceling OBB downloads"); + DownloaderService.cancel(apk.getPatchObbUrl()); + DownloaderService.cancel(apk.getMainObbUrl()); + } } /** * This goes through a series of checks to make sure that the incoming - * {@link Intent} is still valid. The default {@link Intent#getAction() action} - * in the logic is {@link #ACTION_INSTALL} since it is the most complicate - * case. Since the {@code Intent} will be redelivered by Android if the - * app was killed, this needs to check that it still makes sense to handle. + * {@link Intent} is still valid. *
* For example, if F-Droid is killed while installing, it might not receive * the message that the install completed successfully. The checks need to be @@ -145,93 +137,91 @@ public class InstallManagerService extends Service { * with the same {@link PackageInfo#versionCode}, which happens sometimes, * and is allowed by Android. */ - @Override - public int onStartCommand(Intent intent, int flags, int startId) { - Utils.debugLog(TAG, "onStartCommand " + intent); + private void queue(String canonicalUrl, String packageName, @NonNull App app, @NonNull Apk apk) { + Utils.debugLog(TAG, "queue " + packageName); - String canonicalUrl = intent.getDataString(); if (TextUtils.isEmpty(canonicalUrl)) { Utils.debugLog(TAG, "empty canonicalUrl, nothing to do"); - return START_NOT_STICKY; + return; } - String action = intent.getAction(); - - if (ACTION_CANCEL.equals(action)) { - DownloaderService.cancel(this, canonicalUrl); - Apk apk = appUpdateStatusManager.getApk(canonicalUrl); - if (apk != null) { - Utils.debugLog(TAG, "also canceling OBB downloads"); - DownloaderService.cancel(this, apk.getPatchObbUrl()); - DownloaderService.cancel(this, apk.getMainObbUrl()); - } - return START_NOT_STICKY; - } else if (ACTION_INSTALL.equals(action)) { - if (!isPendingInstall(canonicalUrl)) { - Log.i(TAG, "Ignoring INSTALL that is not Pending Install: " + intent); - return START_NOT_STICKY; - } - } else { - Log.i(TAG, "Ignoring unknown intent action: " + intent); - return START_NOT_STICKY; - } - - if (!intent.hasExtra(EXTRA_APP) || !intent.hasExtra(EXTRA_APK)) { - Utils.debugLog(TAG, canonicalUrl + " did not include both an App and Apk instance, ignoring"); - return START_NOT_STICKY; - } - - if ((flags & START_FLAG_REDELIVERY) == START_FLAG_REDELIVERY - && !DownloaderService.isQueuedOrActive(canonicalUrl)) { - Utils.debugLog(TAG, canonicalUrl + " finished downloading while InstallManagerService was killed."); - appUpdateStatusManager.removeApk(canonicalUrl); - return START_NOT_STICKY; - } - - App app = intent.getParcelableExtra(EXTRA_APP); - Apk apk = intent.getParcelableExtra(EXTRA_APK); - if (app == null || apk == null) { - Utils.debugLog(TAG, "Intent had null EXTRA_APP and/or EXTRA_APK: " + intent); - return START_NOT_STICKY; - } - - PackageInfo packageInfo = Utils.getPackageInfo(this, apk.packageName); - if ((flags & START_FLAG_REDELIVERY) == START_FLAG_REDELIVERY - && packageInfo != null && packageInfo.versionCode == apk.versionCode + PackageInfo packageInfo = Utils.getPackageInfo(context, packageName); + if (packageInfo != null && packageInfo.versionCode == apk.versionCode && TextUtils.equals(packageInfo.versionName, apk.versionName)) { - Log.i(TAG, "INSTALL Intent no longer valid since its installed, ignoring: " + intent); - return START_NOT_STICKY; + Log.i(TAG, "Install action no longer valid since its installed, ignoring: " + packageName); + return; } appUpdateStatusManager.addApk(app, apk, AppUpdateStatusManager.Status.Downloading, null); - registerPackageDownloaderReceivers(canonicalUrl); getMainObb(canonicalUrl, apk); getPatchObb(canonicalUrl, apk); - File apkFilePath = ApkCache.getApkDownloadPath(this, apk.getCanonicalUrl()); + File apkFilePath = ApkCache.getApkDownloadPath(context, apk.getCanonicalUrl()); long apkFileSize = apkFilePath.length(); if (!apkFilePath.exists() || apkFileSize < apk.size) { Utils.debugLog(TAG, "download " + canonicalUrl + " " + apkFilePath); - DownloaderService.queue(this, apk.repoId, canonicalUrl, apk.getDownloadUrl(), apk.apkFile); + DownloaderService.queue(context, canonicalUrl, app, apk); } else if (ApkCache.apkIsCached(apkFilePath, apk)) { Utils.debugLog(TAG, "skip download, we have it, straight to install " + canonicalUrl + " " + apkFilePath); - sendBroadcast(intent.getData(), DownloaderService.ACTION_STARTED, apkFilePath); - sendBroadcast(intent.getData(), DownloaderService.ACTION_COMPLETE, apkFilePath); + Uri canonicalUri = Uri.parse(canonicalUrl); + onDownloadStarted(canonicalUri); + onDownloadComplete(canonicalUri, apkFilePath, app, apk); } else { Utils.debugLog(TAG, "delete and download again " + canonicalUrl + " " + apkFilePath); apkFilePath.delete(); - DownloaderService.queue(this, apk.repoId, canonicalUrl, apk.getDownloadUrl(), apk.apkFile); + DownloaderService.queue(context, canonicalUrl, app, apk); } - - return START_REDELIVER_INTENT; // if killed before completion, retry Intent } - private void sendBroadcast(Uri uri, String action, File file) { - Intent intent = new Intent(action); - intent.setData(uri); - intent.putExtra(DownloaderService.EXTRA_DOWNLOAD_PATH, file.getAbsolutePath()); - localBroadcastManager.sendBroadcast(intent); + public void onDownloadStarted(Uri canonicalUri) { + // App should currently be in the "PendingDownload" state, so this changes it to "Downloading". + appUpdateStatusManager.updateApk(canonicalUri.toString(), + AppUpdateStatusManager.Status.Downloading, getDownloadCancelIntent(canonicalUri)); + } + + public void onDownloadProgress(Uri canonicalUri, App app, Apk apk, long bytesRead, long totalBytes) { + if (appUpdateStatusManager.get(canonicalUri.toString()) == null) { + // if our app got killed, we need to re-add the APK here + appUpdateStatusManager.addApk(app, apk, AppUpdateStatusManager.Status.Downloading, + getDownloadCancelIntent(canonicalUri)); + } + appUpdateStatusManager.updateApkProgress(canonicalUri.toString(), totalBytes, bytesRead); + } + + public void onDownloadComplete(Uri canonicalUri, File file, App intentApp, Apk intentApk) { + String canonicalUrl = canonicalUri.toString(); + Uri localApkUri = Uri.fromFile(file); + + Utils.debugLog(TAG, "download completed of " + canonicalUri + " to " + localApkUri); + appUpdateStatusManager.updateApk(canonicalUrl, + AppUpdateStatusManager.Status.ReadyToInstall, null); + + App app = appUpdateStatusManager.getApp(canonicalUrl); + Apk apk = appUpdateStatusManager.getApk(canonicalUrl); + if (app == null || apk == null) { + // These may be null if our app was killed and the download job restarted. + // Then, we can take the objects we saved in the intent which survive app death. + app = intentApp; + apk = intentApk; + } + if (app != null && apk != null) { + registerInstallReceiver(canonicalUrl); + InstallerService.install(context, localApkUri, canonicalUri, app, apk); + } else { + Log.e(TAG, "Could not install " + canonicalUrl + " because no app or apk available."); + } + } + + public void onDownloadFailed(Uri canonicalUri, String errorMsg) { + appUpdateStatusManager.setDownloadError(canonicalUri.toString(), errorMsg); + } + + private PendingIntent getDownloadCancelIntent(Uri canonicalUri) { + Intent intentObject = new Intent(ACTION_CANCEL); + intentObject.putExtra(DownloaderService.EXTRA_CANONICAL_URL, canonicalUri.toString()); + return PendingIntent.getBroadcast(context, 0, intentObject, + PendingIntent.FLAG_UPDATE_CURRENT | PendingIntent.FLAG_IMMUTABLE); } private void getMainObb(final String canonicalUrl, Apk apk) { @@ -257,10 +247,6 @@ public class InstallManagerService extends Service { final BroadcastReceiver downloadReceiver = new BroadcastReceiver() { @Override public void onReceive(Context context, Intent intent) { - if (!running) { - localBroadcastManager.unregisterReceiver(this); - return; - } String action = intent.getAction(); if (DownloaderService.ACTION_STARTED.equals(action)) { Utils.debugLog(TAG, action + " " + intent); @@ -306,91 +292,19 @@ public class InstallManagerService extends Service { } } }; - DownloaderService.queue(this, repoId, obbUrlString, obbUrlString, null); + DownloaderService.queue(context, repoId, obbUrlString, obbUrlString); localBroadcastManager.registerReceiver(downloadReceiver, DownloaderService.getIntentFilter(obbUrlString)); } - /** - * Register a {@link BroadcastReceiver} for tracking download progress for a - * give {@code canonicalUrl}. There can be multiple of these registered at a time. - */ - private void registerPackageDownloaderReceivers(String canonicalUrl) { - - BroadcastReceiver downloadReceiver = new BroadcastReceiver() { - @Override - public void onReceive(Context context, Intent intent) { - if (!running) { - localBroadcastManager.unregisterReceiver(this); - return; - } - Uri canonicalUri = intent.getData(); - String canonicalUrl = intent.getDataString(); - - switch (intent.getAction()) { - case DownloaderService.ACTION_STARTED: - // App should currently be in the "PendingDownload" state, so this changes it to "Downloading". - Intent intentObject = new Intent(context, InstallManagerService.class); - intentObject.setAction(ACTION_CANCEL); - intentObject.setData(canonicalUri); - PendingIntent action = - PendingIntent.getService(context, 0, intentObject, PendingIntent.FLAG_IMMUTABLE); - appUpdateStatusManager.updateApk(canonicalUrl, - AppUpdateStatusManager.Status.Downloading, action); - break; - case DownloaderService.ACTION_PROGRESS: - long bytesRead = intent.getLongExtra(DownloaderService.EXTRA_BYTES_READ, 0); - long totalBytes = intent.getLongExtra(DownloaderService.EXTRA_TOTAL_BYTES, 0); - appUpdateStatusManager.updateApkProgress(canonicalUrl, totalBytes, bytesRead); - break; - case DownloaderService.ACTION_COMPLETE: - File localFile = new File(intent.getStringExtra(DownloaderService.EXTRA_DOWNLOAD_PATH)); - Uri localApkUri = Uri.fromFile(localFile); - - Utils.debugLog(TAG, "download completed of " - + intent.getStringExtra(DownloaderService.EXTRA_MIRROR_URL) + " to " + localApkUri); - appUpdateStatusManager.updateApk(canonicalUrl, - AppUpdateStatusManager.Status.ReadyToInstall, null); - - localBroadcastManager.unregisterReceiver(this); - registerInstallReceiver(canonicalUrl); - - App app = appUpdateStatusManager.getApp(canonicalUrl); - Apk apk = appUpdateStatusManager.getApk(canonicalUrl); - Log.e(TAG, "InstallManagerService: INSTALL : " + apk); // TODO remove - if (apk != null) { - InstallerService.install(context, localApkUri, canonicalUri, app, apk); - } - break; - case DownloaderService.ACTION_INTERRUPTED: - case DownloaderService.ACTION_CONNECTION_FAILED: - appUpdateStatusManager.setDownloadError(canonicalUrl, - intent.getStringExtra(DownloaderService.EXTRA_ERROR_MESSAGE)); - localBroadcastManager.unregisterReceiver(this); - break; - default: - throw new RuntimeException("intent action not handled!"); - } - } - }; - - localBroadcastManager.registerReceiver(downloadReceiver, - DownloaderService.getIntentFilter(canonicalUrl)); - } - /** * Register a {@link BroadcastReceiver} for tracking install progress for a * give {@link Uri}. There can be multiple of these registered at a time. */ private void registerInstallReceiver(String canonicalUrl) { - BroadcastReceiver installReceiver = new BroadcastReceiver() { @Override public void onReceive(Context context, Intent intent) { - if (!running) { - localBroadcastManager.unregisterReceiver(this); - return; - } String canonicalUrl = intent.getDataString(); App app; Apk apk; @@ -406,7 +320,7 @@ public class InstallManagerService extends Service { if (apkComplete != null && apkComplete.isApk()) { try { - PackageManagerCompat.setInstaller(context, getPackageManager(), apkComplete.packageName); + PackageManagerCompat.setInstaller(context, context.getPackageManager(), apkComplete.packageName); } catch (SecurityException e) { // Will happen if we fell back to DefaultInstaller for some reason. } @@ -438,7 +352,7 @@ public class InstallManagerService extends Service { }; localBroadcastManager.registerReceiver(installReceiver, - Installer.getInstallIntentFilter(canonicalUrl)); + Installer.getInstallIntentFilter(Uri.parse(canonicalUrl))); } /** @@ -456,57 +370,11 @@ public class InstallManagerService extends Service { public static void queue(Context context, @NonNull App app, @NonNull Apk apk) { String canonicalUrl = apk.getCanonicalUrl(); AppUpdateStatusManager.getInstance(context).addApk(app, apk, AppUpdateStatusManager.Status.PendingInstall, null); - putPendingInstall(context, canonicalUrl, apk.packageName); Utils.debugLog(TAG, "queue " + app.packageName + " " + apk.versionCode + " from " + canonicalUrl); - Intent intent = new Intent(context, InstallManagerService.class); - intent.setAction(ACTION_INSTALL); - intent.setData(Uri.parse(canonicalUrl)); - intent.putExtra(EXTRA_APP, app); - intent.putExtra(EXTRA_APK, apk); - context.startService(intent); + InstallManagerService.getInstance(context).queue(canonicalUrl, apk.packageName, app, apk); } public static void cancel(Context context, String canonicalUrl) { - removePendingInstall(context, canonicalUrl); - Intent intent = new Intent(context, InstallManagerService.class); - intent.setAction(ACTION_CANCEL); - intent.setData(Uri.parse(canonicalUrl)); - context.startService(intent); - } - - /** - * Is the APK that matches the provided {@code hash} still waiting to be - * installed? This restarts the install process for this APK if it was - * interrupted somehow, like if F-Droid was killed before the download - * completed, or the device lost power in the middle of the install - * process. - */ - public boolean isPendingInstall(String canonicalUrl) { - return pendingInstalls.contains(canonicalUrl); - } - - /** - * Mark a given APK as in the process of being installed, with - * the {@code canonicalUrl} of the download used as the unique ID, - * and the file hash used to verify that things are the same. - * - * @see #isPendingInstall(String) - */ - public static void putPendingInstall(Context context, String canonicalUrl, String packageName) { - if (pendingInstalls == null) { - pendingInstalls = getPendingInstalls(context); - } - pendingInstalls.edit().putString(canonicalUrl, packageName).apply(); - } - - public static void removePendingInstall(Context context, String canonicalUrl) { - if (pendingInstalls == null) { - pendingInstalls = getPendingInstalls(context); - } - pendingInstalls.edit().remove(canonicalUrl).apply(); - } - - private static SharedPreferences getPendingInstalls(Context context) { - return context.getSharedPreferences("pending-installs", Context.MODE_PRIVATE); + InstallManagerService.getInstance(context).onCancel(canonicalUrl); } } 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 8e9923fd3..1913d1175 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java +++ b/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java @@ -22,28 +22,26 @@ import android.content.Context; import android.content.Intent; import android.content.IntentFilter; import android.net.Uri; -import android.os.Handler; -import android.os.HandlerThread; -import android.os.IBinder; -import android.os.Looper; -import android.os.Message; import android.os.PatternMatcher; -import android.os.Process; import android.text.TextUtils; import android.util.Log; -import android.util.LogPrinter; + +import androidx.annotation.NonNull; +import androidx.core.app.JobIntentService; import org.fdroid.database.FDroidDatabase; import org.fdroid.database.Repository; import org.fdroid.download.Downloader; import org.fdroid.download.NotFoundException; -import org.fdroid.fdroid.BuildConfig; import org.fdroid.fdroid.FDroidApp; -import org.fdroid.fdroid.ProgressListener; import org.fdroid.fdroid.Utils; +import org.fdroid.fdroid.data.Apk; +import org.fdroid.fdroid.data.App; import org.fdroid.fdroid.data.DBHelper; import org.fdroid.fdroid.data.SanitizedFile; import org.fdroid.fdroid.installer.ApkCache; +import org.fdroid.fdroid.installer.InstallManagerService; +import org.fdroid.fdroid.installer.Installer; import org.fdroid.index.v2.FileV1; import java.io.File; @@ -68,16 +66,13 @@ import androidx.localbroadcastmanager.content.LocalBroadcastManager; * through {@link #queue(Context, long, String, String, FileV1)} calls. The * service is started as needed, it handles each {@code Intent} using a worker * thread, and stops itself when it runs out of work. Requests can be canceled - * using {@link #cancel(Context, String)}. If this service is killed during - * operation, it will receive the queued {@link #queue(Context, long, String, String, FileV1)} - * and {@link #cancel(Context, String)} requests again due to - * {@link Service#START_REDELIVER_INTENT}. Bad requests will be ignored, + * using {@link #cancel(Context, String)}. Bad requests will be ignored, * including on restart after killing via {@link Service#START_NOT_STICKY}. *
* This "work queue processor" pattern is commonly used to offload tasks * from an application's main thread. The DownloaderService class exists to * simplify this pattern and take care of the mechanics. DownloaderService - * will receive the Intents, launch a worker thread, and stop the service as + * will receive the Intents, use a worker thread, and stop the service as * appropriate. *
* All requests are handled on a single worker thread -- they may take as @@ -94,14 +89,14 @@ import androidx.localbroadcastmanager.content.LocalBroadcastManager; * than with APKs since there is not reliable standard for a unique ID for * media files, unlike APKs with {@code packageName} and {@code versionCode}. * - * @see android.app.IntentService + * @see androidx.core.app.JobIntentService * @see org.fdroid.fdroid.installer.InstallManagerService */ -public class DownloaderService extends Service { +public class DownloaderService extends JobIntentService { private static final String TAG = "DownloaderService"; + private static final int JOB_ID = TAG.hashCode(); private static final String ACTION_QUEUE = "org.fdroid.fdroid.net.DownloaderService.action.QUEUE"; - private static final String ACTION_CANCEL = "org.fdroid.fdroid.net.DownloaderService.action.CANCEL"; public static final String ACTION_STARTED = "org.fdroid.fdroid.net.Downloader.action.STARTED"; public static final String ACTION_PROGRESS = "org.fdroid.fdroid.net.Downloader.action.PROGRESS"; @@ -125,106 +120,39 @@ public class DownloaderService extends Service { * @see android.content.Intent#EXTRA_ORIGINATING_URI */ public static final String EXTRA_CANONICAL_URL = "org.fdroid.fdroid.net.Downloader.extra.CANONICAL_URL"; - private static final String EXTRA_INDEX_FILE_V1 = "org.fdroid.fdroid.net.Downloader.extra.INDEX_FILE_V1"; - private volatile Looper serviceLooper; - private static volatile ServiceHandler serviceHandler; private static volatile Downloader downloader; private static volatile String activeCanonicalUrl; + private InstallManagerService installManagerService; private LocalBroadcastManager localBroadcastManager; - private final class ServiceHandler extends Handler { - static final String TAG = "ServiceHandler"; - - ServiceHandler(Looper looper) { - super(looper); - } - - @Override - public void handleMessage(Message msg) { - Utils.debugLog(TAG, "Handling download message with ID of " + msg.what); - handleIntent((Intent) msg.obj); - stopSelf(msg.arg1); - } - } - @Override public void onCreate() { super.onCreate(); Utils.debugLog(TAG, "Creating downloader service."); - - HandlerThread thread = new HandlerThread(TAG, Process.THREAD_PRIORITY_BACKGROUND); - thread.start(); - - serviceLooper = thread.getLooper(); - if (BuildConfig.DEBUG) { - serviceLooper.setMessageLogging(new LogPrinter(Log.DEBUG, ServiceHandler.TAG)); - } - serviceHandler = new ServiceHandler(serviceLooper); + installManagerService = InstallManagerService.getInstance(this); localBroadcastManager = LocalBroadcastManager.getInstance(this); } @Override - public int onStartCommand(Intent intent, int flags, int startId) { - Utils.debugLog(TAG, "Received Intent for downloading: " + intent + " (with a startId of " + startId + ")"); + protected void onHandleWork(@NonNull Intent intent) { + Utils.debugLog(TAG, "Received Intent for downloading: " + intent); - if (intent == null) { - return START_NOT_STICKY; - } - - String downloadUrl = intent.getDataString(); - if (downloadUrl == null) { - Utils.debugLog(TAG, "Received Intent with no URI: " + intent); - return START_NOT_STICKY; - } - String canonicalUrl = intent.getStringExtra(DownloaderService.EXTRA_CANONICAL_URL); + String canonicalUrl = intent.getDataString(); if (canonicalUrl == null) { - Utils.debugLog(TAG, "Received Intent with no EXTRA_CANONICAL_URL: " + intent); - return START_NOT_STICKY; + Utils.debugLog(TAG, "Received Intent with no URI: " + intent); + return; } - - if (ACTION_CANCEL.equals(intent.getAction())) { - Utils.debugLog(TAG, "Cancelling download of " + canonicalUrl.hashCode() + "/" + canonicalUrl - + " downloading from " + downloadUrl); - Integer whatToRemove = canonicalUrl.hashCode(); - if (serviceHandler.hasMessages(whatToRemove)) { - Utils.debugLog(TAG, "Removing download with ID of " + whatToRemove - + " from service handler, then sending interrupted event."); - serviceHandler.removeMessages(whatToRemove); - sendCancelledBroadcast(intent.getData(), canonicalUrl); - } else if (isActive(canonicalUrl)) { - downloader.cancelDownload(); - } else { - Utils.debugLog(TAG, "ACTION_CANCEL called on something not queued or running" - + " (expected to find message with ID of " + whatToRemove + " in queue)."); - } - } else if (ACTION_QUEUE.equals(intent.getAction())) { - Message msg = serviceHandler.obtainMessage(); - msg.arg1 = startId; - msg.obj = intent; - msg.what = canonicalUrl.hashCode(); - serviceHandler.sendMessage(msg); - Utils.debugLog(TAG, "Queued download of " + canonicalUrl.hashCode() + "/" + canonicalUrl - + " using " + downloadUrl); + if (ACTION_QUEUE.equals(intent.getAction())) { + handleIntent(intent); } else { Utils.debugLog(TAG, "Received Intent with unknown action: " + intent); } - - return START_REDELIVER_INTENT; // if killed before completion, retry Intent } @Override public void onDestroy() { - Utils.debugLog(TAG, "Destroying downloader service. Will move to background and stop our Looper."); - serviceLooper.quit(); //NOPMD - this is copied from IntentService, no super call needed - } - - /** - * This service does not use binding, so no need to implement this method - */ - @Override - public IBinder onBind(Intent intent) { - return null; + Utils.debugLog(TAG, "Destroying downloader service."); } /** @@ -244,14 +172,20 @@ public class DownloaderService extends Service { * android.content.Context#startService(Intent)}. */ private void handleIntent(Intent intent) { - final Uri uri = intent.getData(); - final long repoId = intent.getLongExtra(DownloaderService.EXTRA_REPO_ID, 0); final Uri canonicalUrl = intent.getData(); - final Uri downloadUrl = - Uri.parse(intent.getStringExtra(DownloaderService.EXTRA_CANONICAL_URL)); - final FileV1 fileV1 = FileV1.deserialize(intent.getStringExtra(DownloaderService.EXTRA_INDEX_FILE_V1)); + final App app = intent.getParcelableExtra(Installer.EXTRA_APP); + final Apk apk = intent.getParcelableExtra(Installer.EXTRA_APK); + final long repoId = intent.getLongExtra(DownloaderService.EXTRA_REPO_ID, apk.repoId); + final String extraUrl = intent.getStringExtra(DownloaderService.EXTRA_CANONICAL_URL); + final Uri downloadUrl = Uri.parse(extraUrl == null ? apk.getDownloadUrl() : extraUrl); + final FileV1 fileV1 = apk.apkFile; final SanitizedFile localFile = ApkCache.getApkDownloadPath(this, canonicalUrl); - sendBroadcast(uri, DownloaderService.ACTION_STARTED, localFile, repoId, canonicalUrl); + + Utils.debugLog(TAG, "Queued download of " + canonicalUrl.hashCode() + "/" + canonicalUrl + + " using " + downloadUrl); + + sendBroadcast(canonicalUrl, DownloaderService.ACTION_STARTED, localFile, repoId, canonicalUrl); + installManagerService.onDownloadStarted(canonicalUrl); try { activeCanonicalUrl = canonicalUrl.toString(); @@ -269,30 +203,38 @@ public class DownloaderService extends Service { } } downloader = DownloaderFactory.INSTANCE.create(repo, downloadUrl, fileV1, localFile); - downloader.setListener(new ProgressListener() { - @Override - public void onProgress(long bytesRead, long totalBytes) { - Intent intent = new Intent(DownloaderService.ACTION_PROGRESS); - intent.setData(canonicalUrl); - intent.putExtra(DownloaderService.EXTRA_BYTES_READ, bytesRead); - intent.putExtra(DownloaderService.EXTRA_TOTAL_BYTES, totalBytes); - localBroadcastManager.sendBroadcast(intent); - } + final long[] lastProgressSent = {0}; + downloader.setListener((bytesRead, totalBytes) -> { + // don't send a progress updates out to frequently, to not hit notification rate-limiting + // this can cause us to miss critical notification updates + long now = System.currentTimeMillis(); + if (now - lastProgressSent[0] < 1_000) return; + lastProgressSent[0] = now; + Intent intent1 = new Intent(DownloaderService.ACTION_PROGRESS); + intent1.setData(canonicalUrl); + intent1.putExtra(DownloaderService.EXTRA_BYTES_READ, bytesRead); + intent1.putExtra(DownloaderService.EXTRA_TOTAL_BYTES, totalBytes); + localBroadcastManager.sendBroadcast(intent1); + installManagerService.onDownloadProgress(canonicalUrl, app, apk, bytesRead, totalBytes); }); downloader.download(); - sendBroadcast(uri, DownloaderService.ACTION_COMPLETE, localFile, repoId, canonicalUrl); + sendBroadcast(canonicalUrl, DownloaderService.ACTION_COMPLETE, localFile, repoId, canonicalUrl); + installManagerService.onDownloadComplete(canonicalUrl, localFile, app, apk); } catch (InterruptedException e) { - sendBroadcast(uri, DownloaderService.ACTION_INTERRUPTED, localFile, repoId, canonicalUrl); + sendBroadcast(canonicalUrl, DownloaderService.ACTION_INTERRUPTED, localFile, repoId, canonicalUrl); + installManagerService.onDownloadFailed(canonicalUrl, null); } catch (ConnectException | HttpRetryException | NoRouteToHostException | SocketTimeoutException - | SSLHandshakeException | SSLKeyException | SSLPeerUnverifiedException | SSLProtocolException - | ProtocolException | UnknownHostException | NotFoundException e) { + | SSLHandshakeException | SSLKeyException | SSLPeerUnverifiedException | SSLProtocolException + | ProtocolException | UnknownHostException | NotFoundException e) { // if the above list of exceptions changes, also change it in IndexV1Updater.update() Log.e(TAG, "CONNECTION_FAILED: " + e.getLocalizedMessage()); - sendBroadcast(uri, DownloaderService.ACTION_CONNECTION_FAILED, localFile, repoId, canonicalUrl); + sendBroadcast(canonicalUrl, DownloaderService.ACTION_CONNECTION_FAILED, localFile, repoId, canonicalUrl); + installManagerService.onDownloadFailed(canonicalUrl, e.getLocalizedMessage()); } catch (IOException e) { - e.printStackTrace(); - sendBroadcast(uri, DownloaderService.ACTION_INTERRUPTED, localFile, + Log.e(TAG, "Error downloading: ", e); + sendBroadcast(canonicalUrl, DownloaderService.ACTION_INTERRUPTED, localFile, e.getLocalizedMessage(), repoId, canonicalUrl); + installManagerService.onDownloadFailed(canonicalUrl, e.getLocalizedMessage()); } finally { if (downloader != null) { downloader.close(); @@ -302,10 +244,6 @@ public class DownloaderService extends Service { activeCanonicalUrl = null; } - private void sendCancelledBroadcast(Uri uri, String canonicalUrl) { - sendBroadcast(uri, DownloaderService.ACTION_INTERRUPTED, null, 0, Uri.parse(canonicalUrl)); - } - private void sendBroadcast(Uri uri, String action, File file, long repoId, Uri canonicalUrl) { sendBroadcast(uri, action, file, null, repoId, canonicalUrl); } @@ -335,10 +273,10 @@ public class DownloaderService extends Service { * @param context this app's {@link Context} * @param repoId the database ID number representing one repo * @param canonicalUrl the URL used as the unique ID throughout F-Droid - * @see #cancel(Context, String) + * @see #cancel(String) */ public static void queue(Context context, long repoId, String canonicalUrl, - String downloadUrl, FileV1 fileV1) { + String downloadUrl) { if (TextUtils.isEmpty(canonicalUrl)) { return; } @@ -348,8 +286,20 @@ public class DownloaderService extends Service { intent.setData(Uri.parse(canonicalUrl)); intent.putExtra(DownloaderService.EXTRA_REPO_ID, repoId); intent.putExtra(DownloaderService.EXTRA_CANONICAL_URL, downloadUrl); - intent.putExtra(DownloaderService.EXTRA_INDEX_FILE_V1, fileV1.serialize()); - context.startService(intent); + JobIntentService.enqueueWork(context, DownloaderService.class, JOB_ID, intent); + } + + public static void queue(Context context, String canonicalUrl, @NonNull App app, @NonNull Apk apk) { + if (TextUtils.isEmpty(canonicalUrl) || apk.apkFile == null) { + return; + } + Utils.debugLog(TAG, "Queue download " + canonicalUrl.hashCode() + "/" + canonicalUrl); + Intent intent = new Intent(context, DownloaderService.class); + intent.setAction(ACTION_QUEUE); + intent.setData(Uri.parse(canonicalUrl)); + intent.putExtra(Installer.EXTRA_APP, app); + intent.putExtra(Installer.EXTRA_APK, apk); + JobIntentService.enqueueWork(context, DownloaderService.class, JOB_ID, intent); } /** @@ -357,35 +307,22 @@ public class DownloaderService extends Service { *
* All notifications are sent as an {@link Intent} via local broadcasts to be received by * - * @param context this app's {@link Context} * @param canonicalUrl The URL to remove from the download queue - * @see #queue(Context, long, String, String, FileV1 + * @see #queue(Context, String, App, Apk) */ - public static void cancel(Context context, String canonicalUrl) { + public static void cancel(String canonicalUrl) { if (TextUtils.isEmpty(canonicalUrl)) { return; } - Utils.debugLog(TAG, "Send cancel for " + canonicalUrl.hashCode() + "/" + canonicalUrl); - Intent intent = new Intent(context, DownloaderService.class); - intent.setAction(ACTION_CANCEL); - intent.setData(Uri.parse(canonicalUrl)); - intent.putExtra(DownloaderService.EXTRA_CANONICAL_URL, canonicalUrl); - context.startService(intent); - } - - /** - * 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.AppCompatActivity#onResume()}. - */ - public static boolean isQueuedOrActive(String canonicalUrl) { - if (TextUtils.isEmpty(canonicalUrl)) { //NOPMD - suggests unreadable format - return false; + Utils.debugLog(TAG, "Cancelling download of " + canonicalUrl.hashCode() + "/" + canonicalUrl + + " downloading from " + canonicalUrl); + int whatToRemove = canonicalUrl.hashCode(); + if (isActive(canonicalUrl)) { + downloader.cancelDownload(); + } else { + Utils.debugLog(TAG, "ACTION_CANCEL called on something not queued or running" + + " (expected to find message with ID of " + whatToRemove + " in queue)."); } - if (serviceHandler == null) { - return false; // this service is not even running - } - return serviceHandler.hasMessages(canonicalUrl.hashCode()) || isActive(canonicalUrl); } /**