From ccdd8a168ce9492ff8893dc14db73c2ecba094a7 Mon Sep 17 00:00:00 2001
From: Peter Serwylo <peter@serwylo.com>
Date: Fri, 21 Apr 2017 15:51:55 +1000
Subject: [PATCH] Don't prompt user to install, if apk is in cache from a
 previous install

Use SharedPreferences to keep track of whether we are in the middle of
an install for a particular apk or not. If sothen the presence of an
.apk file in the cache means we need to tell the user (in the updates
tab) that a file is ready to install.
---
 .../fdroid/fdroid/AppUpdateStatusManager.java | 57 +++++++++++++++++++
 .../fdroid/fdroid/AppUpdateStatusService.java | 19 ++++---
 .../installer/InstallManagerService.java      |  6 ++
 3 files changed, 75 insertions(+), 7 deletions(-)

diff --git a/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusManager.java b/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusManager.java
index 39f45d5da..9ce22abb9 100644
--- a/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusManager.java
+++ b/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusManager.java
@@ -5,6 +5,7 @@ import android.app.PendingIntent;
 import android.content.ContentResolver;
 import android.content.Context;
 import android.content.Intent;
+import android.content.SharedPreferences;
 import android.content.pm.PackageManager;
 import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
@@ -35,6 +36,8 @@ import java.util.Map;
  */
 public final class AppUpdateStatusManager {
 
+    private static final String TAG = "AppUpdateStatusManager";
+
     /**
      * Broadcast when:
      *  * The user clears the list of installed apps from notification manager.
@@ -124,9 +127,13 @@ public final class AppUpdateStatusManager {
     private final HashMap<String, AppUpdateStatus> 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);
     }
 
     @Nullable
@@ -419,4 +426,54 @@ 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 uniqueKey) {
+        AppUpdateStatus entry = get(uniqueKey);
+        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 uniqueKey) {
+        AppUpdateStatus entry = get(uniqueKey);
+        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);
+    }
+
 }
diff --git a/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusService.java b/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusService.java
index 416543ebe..0b41dbfa7 100644
--- a/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusService.java
+++ b/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusService.java
@@ -84,20 +84,25 @@ public class AppUpdateStatusService extends IntentService {
             return null;
         }
 
-        // It makes zero difference whether we return the apk from one repo or another. The hash
-        // calculation shows that they are exactly the same binary.
+        // It makes zero difference which apk we get from this list. By definition they all have
+        // the exact same hash, and are thus the same binary.
         Apk downloadedApk = apksMatchingHash.get(0);
 
         PackageInfo installedInfo = null;
         try {
             installedInfo = getPackageManager().getPackageInfo(downloadedApk.packageName, PackageManager.GET_META_DATA);
-        } catch (PackageManager.NameNotFoundException ignored) {}
+        } catch (PackageManager.NameNotFoundException ignored) { }
 
         if (installedInfo == null) {
-            // This will return some false positives, because it is possible that
-            // the user downloaded + installed, tried the apk, and then uninstalled it.
-            Utils.debugLog(TAG, downloadedApk.packageName + " is not installed, so presuming we need to notify the user about installing it.");
-            return downloadedApk;
+            if (AppUpdateStatusManager.getInstance(this).isPendingInstall(hash)) {
+                Utils.debugLog(TAG, downloadedApk.packageName + " is not installed, so presuming we need to notify the user about installing it.");
+                return downloadedApk;
+            } else {
+                // It was probably downloaded for a previous install, and then subsequently removed
+                // (but stayed in the cache, as is the designed behaviour). Under these circumstances
+                // we don't want to tell the user to try and install it.
+                return null;
+            }
         }
 
         if (installedInfo.versionCode >= downloadedInfo.versionCode) {
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 1237d5181..ae78434af 100644
--- a/app/src/main/java/org/fdroid/fdroid/installer/InstallManagerService.java
+++ b/app/src/main/java/org/fdroid/fdroid/installer/InstallManagerService.java
@@ -138,6 +138,7 @@ 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)) {
@@ -164,7 +165,9 @@ public class InstallManagerService extends Service {
             Utils.debugLog(TAG, "Intent had null EXTRA_APP and/or EXTRA_APK: " + intent);
             return START_NOT_STICKY;
         }
+
         appUpdateStatusManager.addApk(apk, AppUpdateStatusManager.Status.Unknown, null);
+        appUpdateStatusManager.markAsPendingInstall(urlString);
 
         registerApkDownloaderReceivers(urlString);
         getObb(urlString, apk.getMainObbUrl(), apk.getMainObbFile(), apk.obbMainFileSha256);
@@ -295,6 +298,7 @@ public class InstallManagerService extends Service {
                         }
                         break;
                     case Downloader.ACTION_INTERRUPTED:
+                        appUpdateStatusManager.markAsNoLongerPendingInstall(urlString);
                         appUpdateStatusManager.updateApk(urlString, AppUpdateStatusManager.Status.Unknown, null);
                         localBroadcastManager.unregisterReceiver(this);
                         break;
@@ -334,6 +338,7 @@ 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);
 
@@ -350,6 +355,7 @@ 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 {