include all needed data in install Intents

Including the App and Apk instances in the Intent that starts
InstallManagerService ensures that the needed data is present in the
Service no matter what happens outside of the Service.  For example, if the
index is updated or cleared while an install is in progress, the install
process still needs to know the name and packageName of the app to update
the Notification.

A cleaner but more labor-intensive way to implement this would be to make
App and Apk properly implement the full Parcelable interface.  That would
require tests to check that the Parcelable methods have all the same fields
as toContentValues() and the database.

closes #660 https://gitlab.com/fdroid/fdroidclient/issues/660

app/src/main/java/org/fdroid/fdroid/data/Apk.java

@@ -4,6 +4,7 @@ import android.annotation.TargetApi;
 import android.content.ContentValues;
 import android.database.Cursor;
 import android.os.Build;
+import android.os.Parcelable;
 
 import org.fdroid.fdroid.Utils;
 
@@ -49,7 +50,12 @@ public class Apk extends ValueObject implements Comparable<Apk> {
     public String repoAddress;
     public Utils.CommaSeparatedList incompatibleReasons;
 
-    public Apk() { }
+    public Apk() {
+    }
+
+    public Apk(Parcelable parcelable) {
+        this(new ContentValuesCursor((ContentValues) parcelable));
+    }
 
     public Apk(Cursor cursor) {
 

app/src/main/java/org/fdroid/fdroid/data/App.java

@@ -8,6 +8,7 @@ import android.content.pm.FeatureInfo;
 import android.content.pm.PackageInfo;
 import android.content.pm.PackageManager;
 import android.database.Cursor;
+import android.os.Parcelable;
 import android.text.TextUtils;
 import android.util.Log;
 
@@ -124,6 +125,10 @@ public class App extends ValueObject implements Comparable<App> {
     public App() {
     }
 
+    public App(Parcelable parcelable) {
+        this(new ContentValuesCursor((ContentValues) parcelable));
+    }
+
     public App(Cursor cursor) {
 
         checkCursorPosition(cursor);

app/src/main/java/org/fdroid/fdroid/data/ContentValuesCursor.java

@@ -0,0 +1,90 @@
+package org.fdroid.fdroid.data;
+
+import android.content.ContentValues;
+import android.database.AbstractCursor;
+import android.database.Cursor;
+import android.os.Bundle;
+
+import java.util.Map;
+
+/**
+ * In order to keep {@link App#App(Cursor)} and {@link Apk#Apk(Cursor)} as
+ * efficient as possible, this wrapper class is used to instantiate {@code App}
+ * and {@code Apk} from {@link App#toContentValues()} and
+ * {@link Apk#toContentValues()} included as extras {@link Bundle}s in the
+ * {@link android.content.Intent} that starts
+ * {@link org.fdroid.fdroid.installer.InstallManagerService}
+ * <p>
+ * This implemented to throw an {@link IllegalArgumentException} if the types
+ * do not match what they are expected to be so that things fail fast. So that
+ * means only types used in {@link App#toContentValues()} and
+ * {@link Apk#toContentValues()} are implemented.
+ */
+public class ContentValuesCursor extends AbstractCursor {
+
+    private final String[] keys;
+    private final Object[] values;
+
+    public ContentValuesCursor(ContentValues contentValues) {
+        super();
+        keys = new String[contentValues.size()];
+        values = new Object[contentValues.size()];
+        int i = 0;
+        for (Map.Entry<String, Object> entry : contentValues.valueSet()) {
+            keys[i] = entry.getKey();
+            values[i] = entry.getValue();
+            i++;
+        }
+        moveToFirst();
+    }
+
+    @Override
+    public int getCount() {
+        return 1;
+    }
+
+    @Override
+    public String[] getColumnNames() {
+        return keys;
+    }
+
+    @Override
+    public String getString(int i) {
+        return (String) values[i];
+    }
+
+    @Override
+    public int getInt(int i) {
+        if (values[i] instanceof Long) {
+            return ((Long) values[i]).intValue();
+        } else if (values[i] instanceof Integer) {
+            return (int) values[i];
+        }
+        throw new IllegalArgumentException("unimplemented");
+    }
+
+    @Override
+    public long getLong(int i) {
+        throw new IllegalArgumentException("unimplemented");
+    }
+
+    @Override
+    public short getShort(int i) {
+        throw new IllegalArgumentException("unimplemented");
+    }
+
+    @Override
+    public float getFloat(int i) {
+        throw new IllegalArgumentException("unimplemented");
+    }
+
+    @Override
+    public double getDouble(int i) {
+        throw new IllegalArgumentException("unimplemented");
+    }
+
+    @Override
+    public boolean isNull(int i) {
+        return values[i] == null;
+    }
+}

app/src/main/java/org/fdroid/fdroid/installer/InstallManagerService.java

@@ -15,7 +15,6 @@ import android.support.v4.app.NotificationCompat;
 import android.support.v4.app.TaskStackBuilder;
 import android.support.v4.content.LocalBroadcastManager;
 import android.text.TextUtils;
-import android.widget.Toast;
 
 import org.fdroid.fdroid.AppDetails;
 import org.fdroid.fdroid.R;
@@ -36,10 +35,12 @@ import java.util.Set;
  * requests an APK to be installed.  It handles checking whether the APK is cached,
  * downloading it, putting up and maintaining a {@link Notification}, and more.
  * <p>
- * Data is sent via {@link Intent}s so that Android handles the message queuing
- * and {@link Service} lifecycle for us, although it adds one layer of redirection
- * between the static method to send the {@code Intent} and the method to
- * actually process it.
+ * The {@link App} and {@link Apk} instances are sent via
+ * {@link Intent#putExtra(String, android.os.Bundle)}
+ * so that Android handles the message queuing and {@link Service} lifecycle for us.
+ * 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.
  * <p>
  * The full 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
@@ -59,7 +60,10 @@ import java.util.Set;
 public class InstallManagerService extends Service {
     public static final String TAG = "InstallManagerService";
 
-    private static final String ACTION_INSTALL = "org.fdroid.fdroid.InstallManagerService.action.INSTALL";
+    private static final String ACTION_INSTALL = "org.fdroid.fdroid.installer.action.INSTALL";
+
+    private static final String EXTRA_APP = "org.fdroid.fdroid.installer.extra.APP";
+    private static final String EXTRA_APK = "org.fdroid.fdroid.installer.extra.APK";
 
     /**
      * The collection of {@link Apk}s that are actively going through this whole process,
@@ -145,14 +149,21 @@ public class InstallManagerService extends Service {
             return START_NOT_STICKY;
         }
 
-        Apk apk = ACTIVE_APKS.get(urlString);
-        if (apk == null) {
-            Utils.debugLog(TAG, urlString + " is not in ACTIVE_APKS, why are we trying to download it?");
-            Toast.makeText(this, urlString + " failed with an imcomplete download request!",
-                    Toast.LENGTH_LONG).show();
+        if (!intent.hasExtra(EXTRA_APP) || !intent.hasExtra(EXTRA_APK)) {
+            Utils.debugLog(TAG, urlString + " did not include both an App and Apk instance, ignoring");
             return START_NOT_STICKY;
         }
 
+        if (!DownloaderService.isQueuedOrActive(urlString)) {
+            Utils.debugLog(TAG, urlString + " finished downloading while InstallManagerService was killed.");
+            cancelNotification(urlString);
+            return START_NOT_STICKY;
+        }
+
+        App app = new App(intent.getParcelableExtra(EXTRA_APP));
+        Apk apk = new Apk(intent.getParcelableExtra(EXTRA_APK));
+        addToActive(urlString, app, apk);
+
         Notification notification = createNotification(intent.getDataString(), apk).build();
         notificationManager.notify(urlString.hashCode(), notification);
 
@@ -357,10 +368,11 @@ public class InstallManagerService extends Service {
     public static void queue(Context context, App app, Apk apk) {
         String urlString = apk.getUrl();
         Utils.debugLog(TAG, "queue " + app.packageName + " " + apk.versionCode + " from " + urlString);
-        addToActive(urlString, app, apk);
         Intent intent = new Intent(context, InstallManagerService.class);
         intent.setAction(ACTION_INSTALL);
         intent.setData(Uri.parse(urlString));
+        intent.putExtra(EXTRA_APP, app.toContentValues());
+        intent.putExtra(EXTRA_APK, apk.toContentValues());
         context.startService(intent);
     }