diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index bff6cbc32..9ac4ea3a3 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -280,9 +280,6 @@ android:name=".data.InstalledAppProviderService" android:permission="android.permission.BIND_JOB_SERVICE" android:exported="false"/> - diff --git a/app/src/main/java/org/fdroid/fdroid/AppDetails2.java b/app/src/main/java/org/fdroid/fdroid/AppDetails2.java index 3ddc1b8d9..3019ace1e 100644 --- a/app/src/main/java/org/fdroid/fdroid/AppDetails2.java +++ b/app/src/main/java/org/fdroid/fdroid/AppDetails2.java @@ -488,15 +488,15 @@ public class AppDetails2 extends AppCompatActivity case Downloading: if (newStatus.progressMax == 0) { // The first progress notification we get telling us our status is "Downloading" - adapter.setProgress(-1, -1, R.string.download_pending); + adapter.setIndeterminateProgress(R.string.download_pending); } else { - adapter.setProgress(newStatus.progressCurrent, newStatus.progressMax, 0); + adapter.setProgress(newStatus.progressCurrent, newStatus.progressMax); } break; case ReadyToInstall: if (justReceived) { - adapter.clearProgress(); + adapter.setIndeterminateProgress(R.string.installing); localBroadcastManager.registerReceiver(installReceiver, Installer.getInstallIntentFilter(Uri.parse(newStatus.getUniqueKey()))); } @@ -517,6 +517,9 @@ public class AppDetails2 extends AppCompatActivity break; case Installing: + adapter.setIndeterminateProgress(R.string.installing); + break; + case Installed: case UpdateAvailable: case InstallError: @@ -553,7 +556,7 @@ public class AppDetails2 extends AppCompatActivity public void onReceive(Context context, Intent intent) { switch (intent.getAction()) { case Installer.ACTION_INSTALL_STARTED: - adapter.setProgress(-1, -1, R.string.installing); + adapter.setIndeterminateProgress(R.string.installing); break; case Installer.ACTION_INSTALL_COMPLETE: adapter.clearProgress(); @@ -625,7 +628,7 @@ public class AppDetails2 extends AppCompatActivity public void onReceive(Context context, Intent intent) { switch (intent.getAction()) { case Installer.ACTION_UNINSTALL_STARTED: - adapter.setProgress(-1, -1, R.string.uninstalling); + adapter.setIndeterminateProgress(R.string.uninstalling); break; case Installer.ACTION_UNINSTALL_COMPLETE: adapter.clearProgress(); @@ -762,18 +765,29 @@ public class AppDetails2 extends AppCompatActivity installApk(apkToInstall); } + /** + * Uninstall the app from the current screen. Since there are many ways + * to uninstall an app, including from Google Play, {@code adb uninstall}, + * or Settings -> Apps, this method cannot ever be sure that the app isn't + * already being uninstalled. So it needs to check that we can actually + * get info on the installed app, otherwise, just call it interrupted and + * quit. + * + * @see issue #1435 + */ @Override public void uninstallApk() { Apk apk = app.installedApk; if (apk == null) { - // TODO ideally, app would be refreshed immediately after install, then this - // workaround would be unnecessary - unless it is a media file apk = app.getMediaApkifInstalled(getApplicationContext()); if (apk == null) { // When the app isn't a media file - the above workaround refers to this. apk = app.getInstalledApk(this); if (apk == null) { - throw new IllegalStateException("Couldn't find installed apk for " + app.packageName); + Log.d(TAG, "Couldn't find installed apk for " + app.packageName); + Toast.makeText(this, R.string.uninstall_error_unknown, Toast.LENGTH_SHORT).show(); + uninstallReceiver.onReceive(this, new Intent(Installer.ACTION_UNINSTALL_INTERRUPTED)); + return; } } app.installedApk = apk; diff --git a/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusManager.java b/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusManager.java index 5a82d9612..939a47a39 100644 --- a/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusManager.java +++ b/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusManager.java @@ -18,6 +18,7 @@ import org.fdroid.fdroid.data.App; import org.fdroid.fdroid.data.AppProvider; import org.fdroid.fdroid.data.Repo; import org.fdroid.fdroid.installer.ErrorDialogActivity; +import org.fdroid.fdroid.installer.InstallManagerService; import java.util.ArrayList; import java.util.Collection; @@ -41,7 +42,7 @@ import java.util.Map; */ public final class AppUpdateStatusManager { - private static final String TAG = "AppUpdateStatusManager"; + public static final String TAG = "AppUpdateStatusManager"; /** * Broadcast when: @@ -90,7 +91,6 @@ public final class AppUpdateStatusManager { private static final String LOGTAG = "AppUpdateStatusManager"; public enum Status { - PendingDownload, DownloadInterrupted, UpdateAvailable, Downloading, @@ -125,6 +125,11 @@ public final class AppUpdateStatusManager { this.intent = intent; } + /** + * @return the unique ID used to represent this specific package's install process + * also known as {@code urlString}. + * @see org.fdroid.fdroid.installer.InstallManagerService + */ public String getUniqueKey() { return apk.getUrl(); } @@ -196,15 +201,9 @@ public final class AppUpdateStatusManager { private final HashMap appMapping = new HashMap<>(); private boolean isBatchUpdating; - /** - * @see #isPendingInstall(String) - */ - private final SharedPreferences apksPendingInstall; - private AppUpdateStatusManager(Context context) { this.context = context; localBroadcastManager = LocalBroadcastManager.getInstance(context.getApplicationContext()); - apksPendingInstall = context.getSharedPreferences("apks-pending-install", Context.MODE_PRIVATE); } public void removeAllByRepo(Repo repo) { @@ -261,6 +260,10 @@ public final class AppUpdateStatusManager { entry.intent = intent; setEntryContentIntentIfEmpty(entry); notifyChange(entry, isStatusUpdate); + + if (status == Status.Installed) { + InstallManagerService.removePendingInstall(context, entry.getUniqueKey()); + } } private void addApkInternal(@NonNull Apk apk, @NonNull Status status, PendingIntent intent) { @@ -269,6 +272,10 @@ public final class AppUpdateStatusManager { setEntryContentIntentIfEmpty(entry); appMapping.put(entry.getUniqueKey(), entry); notifyAdd(entry); + + if (status == Status.Installed) { + InstallManagerService.removePendingInstall(context, entry.getUniqueKey()); + } } private void notifyChange(String reason) { @@ -370,8 +377,15 @@ public final class AppUpdateStatusManager { } } + /** + * Remove an APK from being tracked, since it is now considered {@link Status#Installed} + * + * @param key the unique ID for the install process, also called {@code urlString} + * @see org.fdroid.fdroid.installer.InstallManagerService + */ public void removeApk(String key) { synchronized (appMapping) { + InstallManagerService.removePendingInstall(context, key); AppUpdateStatus entry = appMapping.remove(key); if (entry != null) { Utils.debugLog(LOGTAG, "Remove APK " + entry.apk.apkName); @@ -426,6 +440,8 @@ public final class AppUpdateStatusManager { entry.errorText = errorText; entry.intent = getAppErrorIntent(entry); notifyChange(entry, false); + + InstallManagerService.removePendingInstall(context, entry.getUniqueKey()); } } @@ -541,55 +557,4 @@ public final class AppUpdateStatusManager { errorDialogIntent, PendingIntent.FLAG_UPDATE_CURRENT); } - - /** - * Note that this could technically be made private and automatically invoked when - * {@link #addApk(Apk, Status, PendingIntent)} is called, but that would greatly reduce - * the maintainability of this class. Right now it is used by two clients: the notification - * manager, and the Updates tab. They have different requirements, with the Updates information - * being more permanent than the notification info. As such, the different clients should be - * aware of their requirements when invoking general-sounding methods like "addApk()", rather - * than this class trying to second-guess why they added an apk. - * - * @see #isPendingInstall(String) - */ - public void markAsPendingInstall(String urlString) { - AppUpdateStatus entry = get(urlString); - if (entry != null) { - Utils.debugLog(TAG, "Marking " + entry.apk.packageName + " as pending install."); - apksPendingInstall.edit().putBoolean(entry.apk.hash, true).apply(); - } - } - - /** - * @see #markAsNoLongerPendingInstall(AppUpdateStatus) - * @see #isPendingInstall(String) - */ - public void markAsNoLongerPendingInstall(String urlString) { - AppUpdateStatus entry = get(urlString); - if (entry != null) { - markAsNoLongerPendingInstall(entry); - } - } - - /** - * @see #markAsNoLongerPendingInstall(AppUpdateStatus) - * @see #isPendingInstall(String) - */ - public void markAsNoLongerPendingInstall(@NonNull AppUpdateStatus entry) { - Utils.debugLog(TAG, "Marking " + entry.apk.packageName + " as NO LONGER pending install."); - apksPendingInstall.edit().remove(entry.apk.hash).apply(); - } - - /** - * Keep track of the list of apks for which an install was initiated (i.e. a download + install). - * This is used when F-Droid starts, so that it can look through the cached apks and decide whether - * the presence of a .apk file means we should tell the user to press "Install" to complete the - * process, or whether it is purely there because it was installed some time ago and is no longer - * needed. - */ - public boolean isPendingInstall(String apkHash) { - return apksPendingInstall.contains(apkHash); - } - -} +} \ No newline at end of file diff --git a/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusService.java b/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusService.java deleted file mode 100644 index 2fb81baff..000000000 --- a/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusService.java +++ /dev/null @@ -1,183 +0,0 @@ -package org.fdroid.fdroid; - -import android.app.IntentService; -import android.content.Context; -import android.content.Intent; -import android.content.pm.PackageInfo; -import android.content.pm.PackageManager; -import android.net.Uri; -import android.support.annotation.Nullable; -import android.text.TextUtils; -import android.util.Log; -import org.fdroid.fdroid.data.Apk; -import org.fdroid.fdroid.data.ApkProvider; -import org.fdroid.fdroid.data.InstalledAppProviderService; -import org.fdroid.fdroid.installer.ApkCache; -import org.fdroid.fdroid.installer.InstallManagerService; - -import java.io.File; -import java.util.ArrayList; -import java.util.List; - -/** - * Scans the list of downloaded .apk files in the cache. This info is used to - * determine if an APK is ready to install. When a valid .apk file is found, - * this checks whether that APK is already installed, and whether the user's - * install request is still active. If all those are true, then this tells the - * {@link AppUpdateStatusManager} that the APK is - * {@link AppUpdateStatusManager.Status#ReadyToInstall}. This is an - * {@link IntentService} so as to run on a background thread, as it hits the - * disk a bit to figure out the hash of each downloaded file. - */ -public class AppUpdateStatusService extends IntentService { - - private static final String TAG = "AppUpdateStatusService"; - - /** - * Queue up a background scan of all downloaded apk files to see if we should notify the user - * that they are ready to install. - */ - public static void scanDownloadedApks(Context context) { - context.startService(new Intent(context, AppUpdateStatusService.class)); - } - - public AppUpdateStatusService() { - super("AppUpdateStatusService"); - } - - @Override - protected void onHandleIntent(@Nullable Intent intent) { - Utils.debugLog(TAG, "Scanning apk cache to see if we need to prompt the user to install any apks."); - File cacheDir = ApkCache.getApkCacheDir(this); - if (cacheDir == null) { - return; - } - String[] cacheDirList = cacheDir.list(); - if (cacheDirList == null) { - return; - } - List apksReadyToInstall = new ArrayList<>(); - for (String repoDirName : cacheDirList) { - File repoDir = new File(cacheDir, repoDirName); - String[] apks = repoDir.list(); - if (apks == null) { - continue; - } - for (String apkFileName : apks) { - Apk apk = processDownloadedApk(new File(repoDir, apkFileName)); - if (apk != null) { - PackageInfo packageInfo = Utils.getPackageInfo(this, apk.packageName); - if (packageInfo == null || packageInfo.versionCode != apk.versionCode) { - Utils.debugLog(TAG, "Marking downloaded apk " + apk.apkName + " as ReadyToInstall"); - apksReadyToInstall.add(apk); - } - } - } - } - - if (apksReadyToInstall.size() > 0) { - AppUpdateStatusManager.getInstance(this).addApks(apksReadyToInstall, - AppUpdateStatusManager.Status.ReadyToInstall); - InstallManagerService.managePreviouslyDownloadedApks(this); - } - } - - /** - * Verifies that {@param apkPath} is a valid apk which the user intends to install. - * If it is corrupted to the point where {@link PackageManager} can't read it, doesn't match the hash of any apk - * we know about in our database, is not pending install, or is already installed, then it will return null. - */ - @Nullable - private Apk processDownloadedApk(File apkPath) { - Utils.debugLog(TAG, "Checking " + apkPath); - - // Overly defensive checking for existence. One would think that the file exists at this point, - // because we got it from the result of File#list() earlier. However, this has proven to not be - // sufficient, and by the time we get here we are often hitting a non-existent file. - // This may be due to the fact that the loop checking each file in the cache takes a long time to execute. - // If the number of apps in the cache is large, it can take 10s of seconds to complete. In such - // cases, it is possible that Android has cleared up some files in the cache to make space in - // the meantime. - // - // This is all just a hypothesis about what may have caused - // https://gitlab.com/fdroid/fdroidclient/issues/1172 - if (!apkPath.exists()) { - Log.i(TAG, "Was going to check " + apkPath + ", but it has since been removed from the cache."); - return null; - } - - PackageInfo downloadedInfo = getPackageManager().getPackageArchiveInfo(apkPath.getAbsolutePath(), - PackageManager.GET_GIDS); - if (downloadedInfo == null) { - Log.i(TAG, "Skipping " + apkPath + " because PackageManager was unable to read it."); - return null; - } - - Utils.debugLog(TAG, "Found package for " + downloadedInfo.packageName + ':' + downloadedInfo.versionCode - + ", checking its hash to see if it downloaded correctly."); - Apk downloadedApk = findApkMatchingHash(apkPath); - if (downloadedApk == null) { - Log.i(TAG, "Either the apk wasn't downloaded fully, or the repo it came from has been disabled. " - + "Either way, not notifying the user about it."); - return null; - } - - if (!AppUpdateStatusManager.getInstance(this).isPendingInstall(downloadedApk.hash)) { - Log.i(TAG, downloadedApk.packageName + ':' + downloadedApk.versionCode - + " is NOT pending install, probably just left over from a previous install."); - return null; - } - - PackageInfo info = Utils.getPackageInfo(this, downloadedApk.packageName); - if (info != null) { - File pathToInstalled = InstalledAppProviderService.getPathToInstalledApk(info); - if (pathToInstalled != null && pathToInstalled.canRead() && - pathToInstalled.length() == downloadedApk.size && // Check size before hash for performance. - TextUtils.equals(Utils.getBinaryHash(pathToInstalled, "sha256"), downloadedApk.hash)) { - Log.i(TAG, downloadedApk.packageName - + " is pending install, but we already have the correct version installed."); - AppUpdateStatusManager.getInstance(this).markAsNoLongerPendingInstall(downloadedApk.getUrl()); - return null; - } - } - - Utils.debugLog(TAG, downloadedApk.packageName + ':' + downloadedApk.versionCode - + " is pending install, so we need to notify the user about installing it."); - return downloadedApk; - } - - /** - * There could be multiple apks with the same hash, provided by different repositories. - * This method looks for all matching records in the database. It then asks each of these - * {@link Apk} instances where they expect to be downloaded. If they expect to be downloaded - * to {@param apkPath} then that instance is returned. - *

- * If no files have a matching hash, or only those which don't belong to the correct repo, then - * this will return null. This method needs to do its own check whether the file exists, - * since files can be deleted from the cache at any time without warning. - */ - @Nullable - private Apk findApkMatchingHash(File apkPath) { - if (!apkPath.canRead()) { - return null; - } - - // NOTE: This presumes SHA256 is the only supported hash. It seems like that is an assumption - // in more than one place in the F-Droid client. If this becomes a problem in the future, we - // can query the Apk table for `SELECT DISTINCT hashType FROM fdroid_apk` and then we can just - // try each of the hash types that have been specified in the metadata. Seems a bit overkill - // at the time of writing though. - String hash = Utils.getBinaryHash(apkPath, "sha256"); - - List apksMatchingHash = ApkProvider.Helper.findApksByHash(this, hash); - Utils.debugLog(TAG, "Found " + apksMatchingHash.size() + " apk(s) matching the hash " + hash); - - for (Apk apk : apksMatchingHash) { - if (apkPath.equals(ApkCache.getApkDownloadPath(this, Uri.parse(apk.getUrl())))) { - return apk; - } - } - - return null; - } -} \ No newline at end of file diff --git a/app/src/main/java/org/fdroid/fdroid/FDroidApp.java b/app/src/main/java/org/fdroid/fdroid/FDroidApp.java index 19ab12592..19c097646 100644 --- a/app/src/main/java/org/fdroid/fdroid/FDroidApp.java +++ b/app/src/main/java/org/fdroid/fdroid/FDroidApp.java @@ -372,7 +372,6 @@ public class FDroidApp extends Application { } InstalledAppProviderService.compareToPackageManager(this); - AppUpdateStatusService.scanDownloadedApks(this); // If the user changes the preference to do with filtering rooted apps, // it is easier to just notify a change in the app provider, diff --git a/app/src/main/java/org/fdroid/fdroid/NotificationHelper.java b/app/src/main/java/org/fdroid/fdroid/NotificationHelper.java index 5c290d4e9..aab59045a 100644 --- a/app/src/main/java/org/fdroid/fdroid/NotificationHelper.java +++ b/app/src/main/java/org/fdroid/fdroid/NotificationHelper.java @@ -45,6 +45,12 @@ class NotificationHelper { private static final int MAX_UPDATES_TO_SHOW = 5; private static final int MAX_INSTALLED_TO_SHOW = 10; + /** + * Unique ID used to represent this specific package's install process, + * including {@link Notification}s, also known as {@code urlString}. + * + * @see org.fdroid.fdroid.installer.InstallManagerService + */ static final String EXTRA_NOTIFICATION_KEY = "key"; private static final String GROUP_UPDATES = "updates"; private static final String GROUP_INSTALLED = "installed"; diff --git a/app/src/main/java/org/fdroid/fdroid/data/InstalledAppProviderService.java b/app/src/main/java/org/fdroid/fdroid/data/InstalledAppProviderService.java index 0084c5e13..5f01aa7bd 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/InstalledAppProviderService.java +++ b/app/src/main/java/org/fdroid/fdroid/data/InstalledAppProviderService.java @@ -12,7 +12,6 @@ import android.support.annotation.Nullable; import android.support.v4.app.JobIntentService; import android.util.Log; import org.acra.ACRA; -import org.fdroid.fdroid.AppUpdateStatusManager; import org.fdroid.fdroid.Utils; import org.fdroid.fdroid.data.Schema.InstalledAppTable; import rx.functions.Action1; @@ -235,22 +234,6 @@ public class InstalledAppProviderService extends JobIntentService { try { String hashType = "sha256"; String hash = Utils.getBinaryHash(apk, hashType); - - // Ensure that we no longer notify the user that this apk successfully - // downloaded and is now ready to be installed. Used to be handled only - // by InstallManagerService after receiving ACTION_INSTALL_COMPLETE, but - // that doesn't work for F-Droid itself, which never receives that action. - for (Apk apkInRepo : ApkProvider.Helper.findApksByHash(this, hash)) { - - Utils.debugLog(TAG, "Noticed that " + apkInRepo.apkName + - " version " + apkInRepo.versionName + " was installed," + - " so marking as no longer pending install"); - - AppUpdateStatusManager.getInstance(this) - .markAsNoLongerPendingInstall(apkInRepo.getUrl()); - - } - insertAppIntoDb(this, packageInfo, hashType, hash); } catch (IllegalArgumentException e) { Utils.debugLog(TAG, e.getMessage()); diff --git a/app/src/main/java/org/fdroid/fdroid/installer/DefaultInstaller.java b/app/src/main/java/org/fdroid/fdroid/installer/DefaultInstaller.java index b806d5724..70c23e054 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/DefaultInstaller.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/DefaultInstaller.java @@ -63,8 +63,6 @@ public class DefaultInstaller extends Installer { @Override protected void uninstallPackage() { - sendBroadcastUninstall(Installer.ACTION_UNINSTALL_STARTED); - Intent uninstallIntent = new Intent(context, DefaultInstallerActivity.class); uninstallIntent.setAction(DefaultInstallerActivity.ACTION_UNINSTALL_PACKAGE); uninstallIntent.putExtra(Installer.EXTRA_APK, apk); diff --git a/app/src/main/java/org/fdroid/fdroid/installer/ExtensionInstaller.java b/app/src/main/java/org/fdroid/fdroid/installer/ExtensionInstaller.java index 647ed89af..2ec681cf0 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/ExtensionInstaller.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/ExtensionInstaller.java @@ -78,8 +78,6 @@ public class ExtensionInstaller extends Installer { @Override protected void uninstallPackage() { - sendBroadcastUninstall(Installer.ACTION_UNINSTALL_STARTED); - Intent uninstallIntent = new Intent(context, InstallExtensionDialogActivity.class); uninstallIntent.setAction(InstallExtensionDialogActivity.ACTION_UNINSTALL); diff --git a/app/src/main/java/org/fdroid/fdroid/installer/FileInstaller.java b/app/src/main/java/org/fdroid/fdroid/installer/FileInstaller.java index aa7d41d48..01bf38d3d 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/FileInstaller.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/FileInstaller.java @@ -68,8 +68,6 @@ public class FileInstaller extends Installer { @Override protected void uninstallPackage() { - sendBroadcastUninstall(Installer.ACTION_UNINSTALL_STARTED); - Intent uninstallIntent = new Intent(context, FileInstallerActivity.class); uninstallIntent.setAction(FileInstallerActivity.ACTION_UNINSTALL_FILE); uninstallIntent.putExtra(Installer.EXTRA_APK, apk); diff --git a/app/src/main/java/org/fdroid/fdroid/installer/InstallManagerService.java b/app/src/main/java/org/fdroid/fdroid/installer/InstallManagerService.java index 73343bfd6..2c5440dee 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/InstallManagerService.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/InstallManagerService.java @@ -7,9 +7,11 @@ import android.content.BroadcastReceiver; import android.content.Context; import android.content.Intent; import android.content.IntentFilter; +import android.content.SharedPreferences; import android.content.pm.PackageInfo; import android.net.Uri; import android.os.IBinder; +import android.support.annotation.NonNull; import android.support.v4.content.LocalBroadcastManager; import android.text.TextUtils; import android.util.Log; @@ -40,6 +42,10 @@ import java.io.IOException; * For example, if this {@code InstallManagerService} gets killed, Android will cache * and then redeliver the {@link Intent} for us, which includes all of the data needed * for {@code InstallManagerService} to do its job for the whole lifecycle of an install. + * This {@code Service} never stops itself after completing the action, e.g. + * {@code {@link #stopSelf(int)}}, so {@code Intent}s are sometimes redelivered even + * though they are no longer valid. {@link #onStartCommand(Intent, int, int)} checks + * first that the incoming {@code Intent} is not an invalid, redelivered {@code Intent}. *

* 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 @@ -61,6 +67,11 @@ import java.io.IOException; * This also handles downloading OBB "APK Extension" files for any APK that has one * assigned to it. OBB files are queued up for download before the APK so that they * are hopefully in place before the APK starts. That is not guaranteed though. + *

+ * There may be multiple, available APK files with the same hash. Although it + * is not a security issue to install one or the other, they may have different + * metadata to display in the client. Thus, it may result in weirdness if one + * has a different name/description/summary, etc). * * @see APK Expansion Files */ @@ -71,19 +82,11 @@ public class InstallManagerService extends Service { private static final String ACTION_INSTALL = "org.fdroid.fdroid.installer.action.INSTALL"; private static final String ACTION_CANCEL = "org.fdroid.fdroid.installer.action.CANCEL"; - /** - * The install manager service needs to monitor downloaded apks so that it can wait for a user to - * install them and respond accordingly. Usually the thing which starts listening for such events - * does so directly after a download is complete. This works great, except when the user then - * subsequently closes F-Droid and opens it at a later date. Under these circumstances, a background - * service will scan all downloaded apks and notify the user about them. When it does so, the - * install manager service needs to add listeners for if the apks get installed. - */ - private static final String ACTION_MANAGE_DOWNLOADED_APKS = "org.fdroid.fdroid.installer.action.ACTION_MANAGE_DOWNLOADED_APKS"; - private static final String EXTRA_APP = "org.fdroid.fdroid.installer.extra.APP"; private static final String EXTRA_APK = "org.fdroid.fdroid.installer.extra.APK"; + private static SharedPreferences pendingInstalls; + private LocalBroadcastManager localBroadcastManager; private AppUpdateStatusManager appUpdateStatusManager; private BroadcastReceiver broadcastReceiver; @@ -119,8 +122,16 @@ public class InstallManagerService extends Service { intentFilter.addDataScheme("package"); registerReceiver(broadcastReceiver, intentFilter); running = true; + pendingInstalls = getPendingInstalls(this); } + /** + * 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; @@ -145,19 +156,14 @@ public class InstallManagerService extends Service { public int onStartCommand(Intent intent, int flags, int startId) { Utils.debugLog(TAG, "onStartCommand " + intent); - String action = intent.getAction(); - - if (ACTION_MANAGE_DOWNLOADED_APKS.equals(action)) { - registerInstallerReceiversForDownlaodedApks(); - return START_NOT_STICKY; - } - String urlString = intent.getDataString(); if (TextUtils.isEmpty(urlString)) { Utils.debugLog(TAG, "empty urlString, nothing to do"); return START_NOT_STICKY; } + String action = intent.getAction(); + if (ACTION_CANCEL.equals(action)) { DownloaderService.cancel(this, urlString); Apk apk = appUpdateStatusManager.getApk(urlString); @@ -165,10 +171,14 @@ public class InstallManagerService extends Service { DownloaderService.cancel(this, apk.getPatchObbUrl()); DownloaderService.cancel(this, apk.getMainObbUrl()); } - appUpdateStatusManager.markAsNoLongerPendingInstall(urlString); appUpdateStatusManager.removeApk(urlString); return START_NOT_STICKY; - } else if (!ACTION_INSTALL.equals(action)) { + } else if (ACTION_INSTALL.equals(action)) { + if (!isPendingInstall(urlString)) { + 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; } @@ -204,9 +214,8 @@ public class InstallManagerService extends Service { DownloaderService.setTimeout(FDroidApp.getTimeout()); appUpdateStatusManager.addApk(apk, AppUpdateStatusManager.Status.Downloading, null); - appUpdateStatusManager.markAsPendingInstall(urlString); - registerApkDownloaderReceivers(urlString); + registerPackageDownloaderReceivers(urlString); getObb(urlString, apk.getMainObbUrl(), apk.getMainObbFile(), apk.obbMainFileSha256); getObb(urlString, apk.getPatchObbUrl(), apk.getPatchObbFile(), apk.obbPatchFileSha256); @@ -304,7 +313,11 @@ public class InstallManagerService extends Service { DownloaderService.getIntentFilter(obbUrlString)); } - private void registerApkDownloaderReceivers(String urlString) { + /** + * Register a {@link BroadcastReceiver} for tracking download progress for a + * give {@code urlString}. There can be multiple of these registered at a time. + */ + private void registerPackageDownloaderReceivers(String urlString) { BroadcastReceiver downloadReceiver = new BroadcastReceiver() { @Override @@ -340,7 +353,7 @@ public class InstallManagerService extends Service { appUpdateStatusManager.updateApk(urlString, AppUpdateStatusManager.Status.ReadyToInstall, null); localBroadcastManager.unregisterReceiver(this); - registerInstallerReceivers(downloadUri); + registerInstallReceiver(downloadUri); Apk apk = appUpdateStatusManager.getApk(urlString); if (apk != null) { @@ -348,7 +361,6 @@ public class InstallManagerService extends Service { } break; case Downloader.ACTION_INTERRUPTED: - appUpdateStatusManager.markAsNoLongerPendingInstall(urlString); appUpdateStatusManager.setDownloadError(urlString, intent.getStringExtra(Downloader.EXTRA_ERROR_MESSAGE)); localBroadcastManager.unregisterReceiver(this); break; @@ -357,7 +369,6 @@ public class InstallManagerService extends Service { DownloaderService.queue(context, FDroidApp.getMirror(mirrorUrlString, repoId), repoId, urlString); DownloaderService.setTimeout(FDroidApp.getTimeout()); } catch (IOException e) { - appUpdateStatusManager.markAsNoLongerPendingInstall(urlString); appUpdateStatusManager.setDownloadError(urlString, intent.getStringExtra(Downloader.EXTRA_ERROR_MESSAGE)); localBroadcastManager.unregisterReceiver(this); } @@ -373,20 +384,10 @@ public class InstallManagerService extends Service { } /** - * For each app in the {@link AppUpdateStatusManager.Status#ReadyToInstall} state, setup listeners - * so that if the user installs it then we can respond accordingly. This makes sure that whether - * the user just finished downloading it, or whether they downloaded it a day ago but have not yet - * installed it, we get the same experience upon completing an install. + * 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 registerInstallerReceiversForDownlaodedApks() { - for (AppUpdateStatusManager.AppUpdateStatus appStatus : AppUpdateStatusManager.getInstance(this).getAll()) { - if (appStatus.status == AppUpdateStatusManager.Status.ReadyToInstall) { - registerInstallerReceivers(Uri.parse(appStatus.getUniqueKey())); - } - } - } - - private void registerInstallerReceivers(Uri downloadUri) { + private void registerInstallReceiver(Uri downloadUri) { BroadcastReceiver installReceiver = new BroadcastReceiver() { @Override @@ -402,9 +403,8 @@ public class InstallManagerService extends Service { appUpdateStatusManager.updateApk(downloadUrl, AppUpdateStatusManager.Status.Installing, null); break; case Installer.ACTION_INSTALL_COMPLETE: - appUpdateStatusManager.markAsNoLongerPendingInstall(downloadUrl); appUpdateStatusManager.updateApk(downloadUrl, AppUpdateStatusManager.Status.Installed, null); - Apk apkComplete = appUpdateStatusManager.getApk(downloadUrl); + Apk apkComplete = appUpdateStatusManager.getApk(downloadUrl); if (apkComplete != null && apkComplete.isApk()) { try { @@ -419,7 +419,6 @@ public class InstallManagerService extends Service { apk = intent.getParcelableExtra(Installer.EXTRA_APK); String errorMessage = intent.getStringExtra(Installer.EXTRA_ERROR_MESSAGE); - appUpdateStatusManager.markAsNoLongerPendingInstall(downloadUrl); if (!TextUtils.isEmpty(errorMessage)) { appUpdateStatusManager.setApkError(apk, errorMessage); } else { @@ -443,13 +442,20 @@ public class InstallManagerService extends Service { } /** - * Install an APK, checking the cache and downloading if necessary before starting the process. - * All notifications are sent as an {@link Intent} via local broadcasts to be received by + * Install an APK, checking the cache and downloading if necessary before + * starting the process. All notifications are sent as an {@link Intent} + * via local broadcasts to be received by {@link BroadcastReceiver}s per + * {@code urlString}. This also marks a given APK as in the process of + * being installed, with the {@code urlString} of the download used as the + * unique ID, + *

+ * and the file hash used to verify that things are the same. * * @param context this app's {@link Context} */ - public static void queue(Context context, App app, Apk apk) { + public static void queue(Context context, App app, @NonNull Apk apk) { String urlString = apk.getUrl(); + putPendingInstall(context, urlString, apk.packageName); Uri downloadUri = Uri.parse(urlString); Installer.sendBroadcastInstall(context, downloadUri, Installer.ACTION_INSTALL_STARTED, apk, null, null); @@ -463,15 +469,46 @@ public class InstallManagerService extends Service { } public static void cancel(Context context, String urlString) { + removePendingInstall(context, urlString); Intent intent = new Intent(context, InstallManagerService.class); intent.setAction(ACTION_CANCEL); intent.setData(Uri.parse(urlString)); context.startService(intent); } - public static void managePreviouslyDownloadedApks(Context context) { - Intent intent = new Intent(context, InstallManagerService.class); - intent.setAction(ACTION_MANAGE_DOWNLOADED_APKS); - 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 urlString) { + return pendingInstalls.contains(urlString); + } + + /** + * Mark a given APK as in the process of being installed, with + * the {@code urlString} 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 urlString, String packageName) { + if (pendingInstalls == null) { + pendingInstalls = getPendingInstalls(context); + } + pendingInstalls.edit().putString(urlString, packageName).apply(); + } + + public static void removePendingInstall(Context context, String urlString) { + if (pendingInstalls == null) { + pendingInstalls = getPendingInstalls(context); + } + pendingInstalls.edit().remove(urlString).apply(); + } + + private static SharedPreferences getPendingInstalls(Context context) { + return context.getSharedPreferences("pending-installs", Context.MODE_PRIVATE); } } diff --git a/app/src/main/java/org/fdroid/fdroid/installer/Installer.java b/app/src/main/java/org/fdroid/fdroid/installer/Installer.java index 38268e983..817f1fa53 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/Installer.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/Installer.java @@ -200,6 +200,15 @@ public abstract class Installer { } private void sendBroadcastUninstall(String action, PendingIntent pendingIntent, String errorMessage) { + sendBroadcastUninstall(context, apk, action, pendingIntent, errorMessage); + } + + static void sendBroadcastUninstall(Context context, Apk apk, String action) { + sendBroadcastUninstall(context, apk, action, null, null); + } + + private static void sendBroadcastUninstall(Context context, Apk apk, String action, + PendingIntent pendingIntent, String errorMessage) { Uri uri = Uri.fromParts("package", apk.packageName, null); Intent intent = new Intent(action); diff --git a/app/src/main/java/org/fdroid/fdroid/installer/InstallerService.java b/app/src/main/java/org/fdroid/fdroid/installer/InstallerService.java index db214122b..ee47f5050 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/InstallerService.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/InstallerService.java @@ -142,6 +142,9 @@ public class InstallerService extends JobIntentService { */ public static void uninstall(Context context, @NonNull Apk apk) { Objects.requireNonNull(apk); + + Installer.sendBroadcastUninstall(context, apk, Installer.ACTION_UNINSTALL_STARTED); + Intent intent = new Intent(context, InstallerService.class); intent.setAction(ACTION_UNINSTALL); intent.putExtra(Installer.EXTRA_APK, apk); diff --git a/app/src/main/java/org/fdroid/fdroid/installer/PrivilegedInstaller.java b/app/src/main/java/org/fdroid/fdroid/installer/PrivilegedInstaller.java index fee625af3..653f32068 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/PrivilegedInstaller.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/PrivilegedInstaller.java @@ -355,8 +355,6 @@ public class PrivilegedInstaller extends Installer { @Override protected void uninstallPackage() { - sendBroadcastUninstall(Installer.ACTION_UNINSTALL_STARTED); - ServiceConnection mServiceConnection = new ServiceConnection() { public void onServiceConnected(ComponentName name, IBinder service) { IPrivilegedService privService = IPrivilegedService.Stub.asInterface(service); diff --git a/app/src/main/java/org/fdroid/fdroid/views/AppDetailsRecyclerViewAdapter.java b/app/src/main/java/org/fdroid/fdroid/views/AppDetailsRecyclerViewAdapter.java index a07982285..84bdbf378 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/AppDetailsRecyclerViewAdapter.java +++ b/app/src/main/java/org/fdroid/fdroid/views/AppDetailsRecyclerViewAdapter.java @@ -34,9 +34,7 @@ import android.widget.LinearLayout; import android.widget.ProgressBar; import android.widget.TextView; import android.widget.Toast; - import com.nostra13.universalimageloader.core.ImageLoader; - import org.fdroid.fdroid.Preferences; import org.fdroid.fdroid.R; import org.fdroid.fdroid.Utils; @@ -204,12 +202,20 @@ public class AppDetailsRecyclerViewAdapter } public void clearProgress() { - setProgress(0, 0, 0); + if (headerView != null) { + headerView.clearProgress(); + } } - public void setProgress(long bytesDownloaded, long totalBytes, int resIdString) { + public void setIndeterminateProgress(int resIdString) { if (headerView != null) { - headerView.setProgress(bytesDownloaded, totalBytes, resIdString); + headerView.setIndeterminateProgress(resIdString); + } + } + + public void setProgress(long bytesDownloaded, long totalBytes) { + if (headerView != null) { + headerView.setProgress(bytesDownloaded, totalBytes); } } @@ -360,38 +366,39 @@ public class AppDetailsRecyclerViewAdapter }); } - public void setProgress(long bytesDownloaded, long totalBytes, int resIdString) { - if (bytesDownloaded == 0 && totalBytes == 0) { - // Remove progress bar - progressLayout.setVisibility(View.GONE); - buttonLayout.setVisibility(View.VISIBLE); - } else { - progressBar.setMax(Utils.bytesToKb(totalBytes)); - progressBar.setProgress(Utils.bytesToKb(bytesDownloaded)); - progressBar.setIndeterminate(totalBytes == -1); - progressLabel.setContentDescription(""); - if (resIdString != 0) { - progressLabel.setText(resIdString); - progressLabel.setContentDescription(context.getString(R.string.downloading)); - progressPercent.setText(""); - } else if (totalBytes > 0 && bytesDownloaded >= 0) { - int percent = Utils.getPercent(bytesDownloaded, totalBytes); - progressLabel.setText(Utils.getFriendlySize(bytesDownloaded) - + " / " + Utils.getFriendlySize(totalBytes)); - progressLabel.setContentDescription(context.getString(R.string.app__tts__downloading_progress, - percent)); - progressPercent.setText(String.format(Locale.ENGLISH, "%d%%", percent)); - } else if (bytesDownloaded >= 0) { - progressLabel.setText(Utils.getFriendlySize(bytesDownloaded)); - progressLabel.setContentDescription(context.getString(R.string.downloading)); - progressPercent.setText(""); - } + public void clearProgress() { + progressLayout.setVisibility(View.GONE); + buttonLayout.setVisibility(View.VISIBLE); + } - // Make sure it's visible - if (progressLayout.getVisibility() != View.VISIBLE) { - progressLayout.setVisibility(View.VISIBLE); - buttonLayout.setVisibility(View.GONE); - } + public void setIndeterminateProgress(int resIdString) { + progressLayout.setVisibility(View.VISIBLE); + buttonLayout.setVisibility(View.GONE); + progressBar.setIndeterminate(true); + progressLabel.setText(resIdString); + progressLabel.setContentDescription(context.getString(R.string.downloading)); + progressPercent.setText(""); + } + + public void setProgress(long bytesDownloaded, long totalBytes) { + progressLayout.setVisibility(View.VISIBLE); + buttonLayout.setVisibility(View.GONE); + + progressBar.setMax(Utils.bytesToKb(totalBytes)); + progressBar.setProgress(Utils.bytesToKb(bytesDownloaded)); + progressBar.setIndeterminate(totalBytes <= 0); + progressLabel.setContentDescription(""); + if (totalBytes > 0 && bytesDownloaded >= 0) { + int percent = Utils.getPercent(bytesDownloaded, totalBytes); + progressLabel.setText(Utils.getFriendlySize(bytesDownloaded) + + " / " + Utils.getFriendlySize(totalBytes)); + progressLabel.setContentDescription(context.getString(R.string.app__tts__downloading_progress, + percent)); + progressPercent.setText(String.format(Locale.ENGLISH, "%d%%", percent)); + } else if (bytesDownloaded >= 0) { + progressLabel.setText(Utils.getFriendlySize(bytesDownloaded)); + progressLabel.setContentDescription(context.getString(R.string.downloading)); + progressPercent.setText(""); } } diff --git a/app/src/main/java/org/fdroid/fdroid/views/main/MainActivity.java b/app/src/main/java/org/fdroid/fdroid/views/main/MainActivity.java index 7350e5c80..fcf90e1c2 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/main/MainActivity.java +++ b/app/src/main/java/org/fdroid/fdroid/views/main/MainActivity.java @@ -395,7 +395,6 @@ public class MainActivity extends AppCompatActivity implements BottomNavigationB * There are a bunch of reasons why we would get notified about app statuses. * The ones we are interested in are those which would result in the "items requiring user interaction" * to increase or decrease: - * * Bulk updates of ready-to-install-apps (relating to {@link org.fdroid.fdroid.AppUpdateStatusService}. * * Change in status to: * * {@link AppUpdateStatusManager.Status#ReadyToInstall} (Causes the count to go UP by one) * * {@link AppUpdateStatusManager.Status#Installed} (Causes the count to go DOWN by one) diff --git a/app/src/main/java/org/fdroid/fdroid/views/updates/items/AppStatusListItemController.java b/app/src/main/java/org/fdroid/fdroid/views/updates/items/AppStatusListItemController.java index 981409eff..4bf93f9e0 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/updates/items/AppStatusListItemController.java +++ b/app/src/main/java/org/fdroid/fdroid/views/updates/items/AppStatusListItemController.java @@ -61,12 +61,6 @@ public class AppStatusListItemController extends AppListItemController { AppUpdateStatusManager manager = AppUpdateStatusManager.getInstance(activity); manager.removeApk(status.getUniqueKey()); switch (status.status) { - case ReadyToInstall: - manager.markAsNoLongerPendingInstall(status); - // Do this silently, because it should be pretty obvious based on the context - // of a "Ready to install" app being dismissed. - break; - case Downloading: cancelDownload(); message = activity.getString(R.string.app_list__dismiss_downloading_app);