Merge branch 'installer-manager-fixes' into 'master'

Installer manager fixes

Builds on the recently merged `InstallManagerService` to fix a few minor UX bugs:
 * Cancellation of pending downloads now removes notifications.
 * No longer shows "Downloading Downloading {AppName}" in notification, just "Downloading {AppName}"
 * If cached file is same size but corrupted, remove it and then continue with the process of downloading + installing

See merge request !281
This commit is contained in:
Hans-Christoph Steiner 2016-05-12 07:22:35 +00:00
commit a0c20a35c3
3 changed files with 25 additions and 9 deletions

View File

@ -25,6 +25,7 @@ import org.fdroid.fdroid.net.Downloader;
import org.fdroid.fdroid.net.DownloaderService; import org.fdroid.fdroid.net.DownloaderService;
import java.io.File; import java.io.File;
import java.security.NoSuchAlgorithmException;
import java.util.HashMap; import java.util.HashMap;
import java.util.Map; import java.util.Map;
import java.util.Set; import java.util.Set;
@ -144,7 +145,7 @@ public class InstallManagerService extends Service {
if (!apkFilePath.exists() || apkFileSize < apk.size) { if (!apkFilePath.exists() || apkFileSize < apk.size) {
Utils.debugLog(TAG, "download " + urlString + " " + apkFilePath); Utils.debugLog(TAG, "download " + urlString + " " + apkFilePath);
DownloaderService.queue(this, urlString); DownloaderService.queue(this, urlString);
} else if (apkFileSize == apk.size) { } else if (apkIsCached(apkFilePath, apk)) {
Utils.debugLog(TAG, "skip download, we have it, straight to install " + urlString + " " + apkFilePath); Utils.debugLog(TAG, "skip download, we have it, straight to install " + urlString + " " + apkFilePath);
sendBroadcast(intent.getData(), Downloader.ACTION_STARTED, apkFilePath); sendBroadcast(intent.getData(), Downloader.ACTION_STARTED, apkFilePath);
sendBroadcast(intent.getData(), Downloader.ACTION_COMPLETE, apkFilePath); sendBroadcast(intent.getData(), Downloader.ACTION_COMPLETE, apkFilePath);
@ -156,6 +157,21 @@ public class InstallManagerService extends Service {
return START_REDELIVER_INTENT; // if killed before completion, retry Intent return START_REDELIVER_INTENT; // if killed before completion, retry Intent
} }
/**
* Verifies the size of the file on disk matches, and then hashes the file to compare with what
* we received from the signed repo (i.e. {@link Apk#hash} and {@link Apk#hashType}).
* Bails out if the file sizes don't match to prevent having to do the work of hashing the file.
*/
private static boolean apkIsCached(File apkFile, Apk apkToCheck) {
try {
return apkFile.length() == apkToCheck.size &&
Installer.verifyApkFile(apkFile, apkToCheck.hash, apkToCheck.hashType);
} catch (NoSuchAlgorithmException e) {
e.printStackTrace();
return false;
}
}
private void sendBroadcast(Uri uri, String action, File file) { private void sendBroadcast(Uri uri, String action, File file) {
Intent intent = new Intent(action); Intent intent = new Intent(action);
intent.setData(uri); intent.setData(uri);
@ -210,9 +226,7 @@ public class InstallManagerService extends Service {
Apk apk = ACTIVE_APKS.remove(urlString); Apk apk = ACTIVE_APKS.remove(urlString);
ACTIVE_APPS.remove(apk.packageName); ACTIVE_APPS.remove(apk.packageName);
unregisterDownloaderReceivers(urlString); unregisterDownloaderReceivers(urlString);
if (AppDetails.isAppVisible(apk.packageName)) { cancelNotification(urlString);
cancelNotification(urlString);
}
} }
}; };
localBroadcastManager.registerReceiver(startedReceiver, localBroadcastManager.registerReceiver(startedReceiver,
@ -245,13 +259,13 @@ public class InstallManagerService extends Service {
App app = ACTIVE_APPS.get(apk.packageName); App app = ACTIVE_APPS.get(apk.packageName);
if (app == null || TextUtils.isEmpty(app.name)) { if (app == null || TextUtils.isEmpty(app.name)) {
if (TEMP_HACK_APP_NAMES.containsKey(urlString)) { if (TEMP_HACK_APP_NAMES.containsKey(urlString)) {
return getString(R.string.downloading_apk, TEMP_HACK_APP_NAMES.get(urlString)); return TEMP_HACK_APP_NAMES.get(urlString);
} else { } else {
// this is ugly, but its better than nothing as a failsafe // this is ugly, but its better than nothing as a failsafe
return getString(R.string.downloading_apk, urlString); return urlString;
} }
} else { } else {
return getString(R.string.downloading_apk, app.name); return app.name;
} }
} }

View File

@ -156,7 +156,7 @@ public abstract class Installer {
/** /**
* Checks the APK file against the provided hash, returning whether it is a match. * Checks the APK file against the provided hash, returning whether it is a match.
*/ */
private static boolean verifyApkFile(File apkFile, String hash, String hashType) public static boolean verifyApkFile(File apkFile, String hash, String hashType)
throws NoSuchAlgorithmException { throws NoSuchAlgorithmException {
if (!apkFile.exists()) { if (!apkFile.exists()) {
return false; return false;

View File

@ -89,6 +89,7 @@ public class DownloaderService extends Service {
@Override @Override
public void handleMessage(Message msg) { public void handleMessage(Message msg) {
Utils.debugLog(TAG, "Handling download message with ID of " + msg.what);
handleIntent((Intent) msg.obj); handleIntent((Intent) msg.obj);
stopSelf(msg.arg1); stopSelf(msg.arg1);
} }
@ -120,12 +121,13 @@ public class DownloaderService extends Service {
Utils.debugLog(TAG, "Cancelling download of " + uriString); Utils.debugLog(TAG, "Cancelling download of " + uriString);
Integer whatToRemove = uriString.hashCode(); Integer whatToRemove = uriString.hashCode();
if (serviceHandler.hasMessages(whatToRemove)) { if (serviceHandler.hasMessages(whatToRemove)) {
Utils.debugLog(TAG, "Removing download with ID of " + whatToRemove + " from service handler, then sending interrupted event.");
serviceHandler.removeMessages(whatToRemove); serviceHandler.removeMessages(whatToRemove);
sendBroadcast(intent.getData(), Downloader.ACTION_INTERRUPTED); sendBroadcast(intent.getData(), Downloader.ACTION_INTERRUPTED);
} else if (isActive(uriString)) { } else if (isActive(uriString)) {
downloader.cancelDownload(); downloader.cancelDownload();
} else { } else {
Log.e(TAG, "ACTION_CANCEL called on something not queued or running"); Log.e(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())) { } else if (ACTION_QUEUE.equals(intent.getAction())) {
Message msg = serviceHandler.obtainMessage(); Message msg = serviceHandler.obtainMessage();