diff --git a/app/src/main/java/org/fdroid/fdroid/CleanCacheService.java b/app/src/main/java/org/fdroid/fdroid/CleanCacheService.java index 2feb9d73d..1f817f03e 100644 --- a/app/src/main/java/org/fdroid/fdroid/CleanCacheService.java +++ b/app/src/main/java/org/fdroid/fdroid/CleanCacheService.java @@ -7,7 +7,6 @@ import android.content.Context; import android.content.Intent; import android.os.Process; import android.os.SystemClock; -import android.util.Log; import org.apache.commons.io.FileUtils; @@ -20,7 +19,6 @@ import java.io.File; * {@link FDroidApp#onCreate()} */ public class CleanCacheService extends IntentService { - private static final String TAG = "CleanCacheService"; /** * Schedule or cancel this service to update the app index, according to the @@ -33,7 +31,6 @@ public class CleanCacheService extends IntentService { if (keepTime < interval) { interval = keepTime * 1000; } - Log.i(TAG, "schedule " + keepTime + " " + interval); Intent intent = new Intent(context, CleanCacheService.class); PendingIntent pending = PendingIntent.getService(context, 0, intent, 0); @@ -53,6 +50,29 @@ public class CleanCacheService extends IntentService { Process.setThreadPriority(Process.THREAD_PRIORITY_LOWEST); Utils.clearOldFiles(Utils.getApkCacheDir(this), Preferences.get().getKeepCacheTime()); deleteStrayIndexFiles(); + deleteOldInstallerFiles(); + } + + /** + * {@link org.fdroid.fdroid.installer.Installer} instances copy the APK into + * a safe place before installing. It doesn't clean up them reliably yet. + */ + private void deleteOldInstallerFiles() { + File filesDir = getFilesDir(); + if (filesDir == null) { + return; + } + + final File[] files = filesDir.listFiles(); + if (files == null) { + return; + } + + for (File f : files) { + if (f.getName().startsWith("install-")) { + FileUtils.deleteQuietly(f); + } + } } /** diff --git a/app/src/main/java/org/fdroid/fdroid/UpdateService.java b/app/src/main/java/org/fdroid/fdroid/UpdateService.java index 3f163f438..0d189b4fc 100644 --- a/app/src/main/java/org/fdroid/fdroid/UpdateService.java +++ b/app/src/main/java/org/fdroid/fdroid/UpdateService.java @@ -512,7 +512,10 @@ public class UpdateService extends IntentService { public void onProgress(URL sourceUrl, int bytesRead, int totalBytes) { Log.i(TAG, "downloadProgressReceiver " + sourceUrl); String downloadedSizeFriendly = Utils.getFriendlySize(bytesRead); - int percent = (int) ((double) bytesRead / totalBytes * 100); + int percent = -1; + if (totalBytes > 0) { + percent = (int) ((double) bytesRead / totalBytes * 100); + } String message; if (totalBytes == -1) { message = getString(R.string.status_download_unknown_size, sourceUrl, downloadedSizeFriendly); diff --git a/app/src/main/java/org/fdroid/fdroid/data/Apk.java b/app/src/main/java/org/fdroid/fdroid/data/Apk.java index ee68ede09..377bf37bf 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/Apk.java +++ b/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 { public String repoAddress; public Utils.CommaSeparatedList incompatibleReasons; - public Apk() { } + public Apk() { + } + + public Apk(Parcelable parcelable) { + this(new ContentValuesCursor((ContentValues) parcelable)); + } public Apk(Cursor cursor) { diff --git a/app/src/main/java/org/fdroid/fdroid/data/App.java b/app/src/main/java/org/fdroid/fdroid/data/App.java index e53460ab3..94e1af011 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/App.java +++ b/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 { public App() { } + public App(Parcelable parcelable) { + this(new ContentValuesCursor((ContentValues) parcelable)); + } + public App(Cursor cursor) { checkCursorPosition(cursor); diff --git a/app/src/main/java/org/fdroid/fdroid/data/ContentValuesCursor.java b/app/src/main/java/org/fdroid/fdroid/data/ContentValuesCursor.java new file mode 100644 index 000000000..29ff7dfa5 --- /dev/null +++ b/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} + *

+ * 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 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; + } +} 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 322f8ab0e..7a2091a9c 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/InstallManagerService.java +++ b/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. *

- * 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. *

* 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); } 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 daf27221d..8f4011190 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/Installer.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/Installer.java @@ -170,7 +170,7 @@ public abstract class Installer { */ public void installPackage(File apkFile, String packageName, String urlString) throws InstallFailedException { - SanitizedFile apkToInstall; + SanitizedFile apkToInstall = null; try { Map attributes = AndroidXMLDecompress.getManifestHeaderAttributes(apkFile.getAbsolutePath()); @@ -232,6 +232,22 @@ public abstract class Installer { throw new InstallFailedException(e); } catch (ClassCastException e) { throw new InstallFailedException("F-Droid Privileged can only be updated using an activity!"); + } finally { + // 20 minutes the start of the install process, delete the file + final File apkToDelete = apkToInstall; + new Thread() { + @Override + public void run() { + android.os.Process.setThreadPriority(android.os.Process.THREAD_PRIORITY_LOWEST); + try { + Thread.sleep(1200000); + } catch (InterruptedException e) { + e.printStackTrace(); + } finally { + FileUtils.deleteQuietly(apkToDelete); + } + } + }.start(); } }