From ee7055e118260152e4983fb8e824db43494bfc11 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Fri, 26 May 2017 09:21:42 +1000 Subject: [PATCH 1/8] Listen to AppUpdateStatusManager events instead of DownloadManager events. Also, make sure to correctly update the app details view when te user leaves then returns to the view. Prior to this, the user would need to wait for a download event to be received. However even that was broken, because the download listener was not being added correctly once the user returned to the app details screen. --- .../java/org/fdroid/fdroid/AppDetails2.java | 179 +++++++++++------- .../fdroid/fdroid/AppUpdateStatusManager.java | 25 ++- .../org/fdroid/fdroid/NotificationHelper.java | 7 +- .../installer/InstallManagerService.java | 6 +- .../fdroid/fdroid/net/DownloaderService.java | 7 +- .../fdroid/views/updates/UpdatesAdapter.java | 2 +- app/src/main/res/values/strings.xml | 1 + 7 files changed, 153 insertions(+), 74 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/AppDetails2.java b/app/src/main/java/org/fdroid/fdroid/AppDetails2.java index 25e9469cd..ce988c741 100644 --- a/app/src/main/java/org/fdroid/fdroid/AppDetails2.java +++ b/app/src/main/java/org/fdroid/fdroid/AppDetails2.java @@ -28,6 +28,7 @@ import android.content.BroadcastReceiver; import android.content.Context; import android.content.DialogInterface; import android.content.Intent; +import android.content.IntentFilter; import android.content.pm.PackageInfo; import android.content.pm.PackageManager; import android.database.ContentObserver; @@ -35,6 +36,7 @@ import android.net.Uri; import android.os.Bundle; import android.os.Handler; import android.support.annotation.NonNull; +import android.support.annotation.Nullable; import android.support.design.widget.AppBarLayout; import android.support.design.widget.CoordinatorLayout; import android.support.v4.content.LocalBroadcastManager; @@ -65,13 +67,13 @@ import org.fdroid.fdroid.installer.InstallManagerService; import org.fdroid.fdroid.installer.Installer; import org.fdroid.fdroid.installer.InstallerFactory; import org.fdroid.fdroid.installer.InstallerService; -import org.fdroid.fdroid.net.Downloader; -import org.fdroid.fdroid.net.DownloaderService; import org.fdroid.fdroid.views.AppDetailsRecyclerViewAdapter; import org.fdroid.fdroid.views.OverscrollLinearLayoutManager; import org.fdroid.fdroid.views.ShareChooserDialog; import org.fdroid.fdroid.views.apps.FeatureImage; +import java.util.Iterator; + public class AppDetails2 extends AppCompatActivity implements ShareChooserDialog.ShareChooserDialogListener, AppDetailsRecyclerViewAdapter.AppDetailsRecyclerViewAdapterCallbacks { public static final String EXTRA_APPID = "appid"; @@ -88,7 +90,7 @@ public class AppDetails2 extends AppCompatActivity implements ShareChooserDialog private RecyclerView recyclerView; private AppDetailsRecyclerViewAdapter adapter; private LocalBroadcastManager localBroadcastManager; - private String activeDownloadUrlString; + private AppUpdateStatusManager.AppUpdateStatus currentStatus; /** * Check if {@code packageName} is currently visible to the user. @@ -125,7 +127,10 @@ public class AppDetails2 extends AppCompatActivity implements ShareChooserDialog OverscrollLinearLayoutManager lm = new OverscrollLinearLayoutManager(this, LinearLayoutManager.VERTICAL, false); lm.setStackFromEnd(false); - /** The recyclerView/AppBarLayout combo has a bug that prevents a "fling" from the bottom + // Has to be invoked after AppDetailsRecyclerViewAdapter is created. + refreshStatus(); + + /* The recyclerView/AppBarLayout combo has a bug that prevents a "fling" from the bottom * to continue all the way to the top by expanding the AppBarLayout. It will instead stop * with the app bar in a collapsed state. See here: https://code.google.com/p/android/issues/detail?id=177729 * Not sure this is the exact issue, but it is true that while in a fling the RecyclerView will @@ -234,6 +239,28 @@ public class AppDetails2 extends AppCompatActivity implements ShareChooserDialog appObserver); updateNotificationsForApp(); + refreshStatus(); + registerAppStatusReceiver(); + } + + /** + * Figures out the current install/update/download/etc status for the app we are viewing. + * Then, asks the view to update itself to reflect this status. + */ + private void refreshStatus() { + Iterator statuses = AppUpdateStatusManager.getInstance(this).getByPackageName(app.packageName).iterator(); + if (statuses.hasNext()) { + AppUpdateStatusManager.AppUpdateStatus status = statuses.next(); + updateAppStatus(status, false); + } + + currentStatus = null; + } + + @Override + protected void onPause() { + super.onPause(); + unregisterAppStatusReceiver(); } protected void onStop() { @@ -395,6 +422,11 @@ public class AppDetails2 extends AppCompatActivity implements ShareChooserDialog } private void initiateInstall(Apk apk) { + if (isAppDownloading()) { + Log.i(TAG, "Ignoring request to install " + apk.packageName + " version " + apk.versionName + ", as we are already downloading (either that or a different version)."); + return; + } + Installer installer = InstallerFactory.create(this, apk); Intent intent = installer.getPermissionScreen(); if (intent != null) { @@ -408,8 +440,6 @@ public class AppDetails2 extends AppCompatActivity implements ShareChooserDialog } private void startInstall(Apk apk) { - activeDownloadUrlString = apk.getUrl(); - registerDownloaderReceiver(); InstallManagerService.queue(this, app, apk); } @@ -427,52 +457,87 @@ public class AppDetails2 extends AppCompatActivity implements ShareChooserDialog localBroadcastManager.unregisterReceiver(uninstallReceiver); } - private void registerDownloaderReceiver() { - if (activeDownloadUrlString != null) { // if a download is active - String url = activeDownloadUrlString; - localBroadcastManager.registerReceiver(downloadReceiver, - DownloaderService.getIntentFilter(url)); - } + private void registerAppStatusReceiver() { + IntentFilter filter = new IntentFilter(); + filter.addAction(AppUpdateStatusManager.BROADCAST_APPSTATUS_REMOVED); + filter.addAction(AppUpdateStatusManager.BROADCAST_APPSTATUS_ADDED); + filter.addAction(AppUpdateStatusManager.BROADCAST_APPSTATUS_CHANGED); + localBroadcastManager.registerReceiver(appStatusReceiver, filter); } - private void unregisterDownloaderReceiver() { - localBroadcastManager.unregisterReceiver(downloadReceiver); + private void unregisterAppStatusReceiver() { + localBroadcastManager.unregisterReceiver(appStatusReceiver); } private void unregisterInstallReceiver() { localBroadcastManager.unregisterReceiver(installReceiver); } - private final BroadcastReceiver downloadReceiver = new BroadcastReceiver() { + private void updateAppStatus(@Nullable AppUpdateStatusManager.AppUpdateStatus newStatus, boolean justReceived) { + this.currentStatus = newStatus; + if (this.currentStatus == null) { + return; + } + + switch (newStatus.status) { + case PendingDownload: + adapter.setProgress(-1, -1, R.string.download_pending); + break; + + case Downloading: + if (newStatus.progressMax == 0) { + // The first progress notification we get telling us our status is "Downloading" + adapter.setProgress(-1, -1, 0); + } else { + adapter.setProgress(newStatus.progressCurrent, newStatus.progressMax, 0); + } + break; + + case ReadyToInstall: + if (justReceived) { + adapter.clearProgress(); + localBroadcastManager.registerReceiver(installReceiver, Installer.getInstallIntentFilter(Uri.parse(newStatus.getUniqueKey()))); + } + break; + + case DownloadInterrupted: + if (justReceived) { + if (TextUtils.isEmpty(newStatus.errorText)) { + Toast.makeText(this, R.string.details_notinstalled, Toast.LENGTH_LONG).show(); + } else { + String msg = newStatus.errorText + " " + newStatus.getUniqueKey(); + Toast.makeText(this, R.string.download_error, Toast.LENGTH_SHORT).show(); + Toast.makeText(this, msg, Toast.LENGTH_LONG).show(); + } + + adapter.clearProgress(); + } + break; + + case Installing: + case Installed: + case UpdateAvailable: + case InstallError: + // Ignore. + break; + } + + } + + private final BroadcastReceiver appStatusReceiver = new BroadcastReceiver() { @Override public void onReceive(Context context, Intent intent) { - switch (intent.getAction()) { - case Downloader.ACTION_STARTED: - adapter.setProgress(-1, -1, R.string.download_pending); - break; - case Downloader.ACTION_PROGRESS: - adapter.setProgress(intent.getIntExtra(Downloader.EXTRA_BYTES_READ, -1), - intent.getIntExtra(Downloader.EXTRA_TOTAL_BYTES, -1), 0); - break; - case Downloader.ACTION_COMPLETE: - // Starts the install process once the download is complete. - cleanUpFinishedDownload(); - localBroadcastManager.registerReceiver(installReceiver, - Installer.getInstallIntentFilter(intent.getData())); - break; - case Downloader.ACTION_INTERRUPTED: - if (intent.hasExtra(Downloader.EXTRA_ERROR_MESSAGE)) { - String msg = intent.getStringExtra(Downloader.EXTRA_ERROR_MESSAGE) - + " " + intent.getDataString(); - Toast.makeText(context, R.string.download_error, Toast.LENGTH_SHORT).show(); - Toast.makeText(context, msg, Toast.LENGTH_LONG).show(); - } else { // user canceled - Toast.makeText(context, R.string.details_notinstalled, Toast.LENGTH_LONG).show(); - } - cleanUpFinishedDownload(); - break; - default: - throw new RuntimeException("intent action not handled!"); + String apkUrl = intent.getStringExtra(AppUpdateStatusManager.EXTRA_APK_URL); + AppUpdateStatusManager.AppUpdateStatus status = AppUpdateStatusManager.getInstance(context).get(apkUrl); + + boolean isRemoving = TextUtils.equals(intent.getAction(), AppUpdateStatusManager.BROADCAST_APPSTATUS_REMOVED); + // !TextUtils.equals(status.apk.packageName, app.packageName) + if (status == null && currentStatus != null && isRemoving && !TextUtils.equals(apkUrl, currentStatus.getUniqueKey())) { + Utils.debugLog(TAG, "Ignoring app status change because it belongs to " + apkUrl + " not " + currentStatus.getUniqueKey()); + } else if (status != null && !TextUtils.equals(status.apk.packageName, app.packageName)) { + Utils.debugLog(TAG, "Ignoring app status change because it belongs to " + status.apk.packageName + " not " + app.packageName); + } else { + updateAppStatus(status, true); } } }; @@ -602,8 +667,6 @@ public class AppDetails2 extends AppCompatActivity implements ShareChooserDialog Utils.debugLog(TAG, "Getting application details for " + packageName); App newApp = null; - calcActiveDownloadUrlString(packageName); - if (!TextUtils.isEmpty(packageName)) { newApp = AppProvider.Helper.findHighestPriorityMetadata(getContentResolver(), packageName); } @@ -612,25 +675,6 @@ public class AppDetails2 extends AppCompatActivity implements ShareChooserDialog return this.app != null; } - private void calcActiveDownloadUrlString(String packageName) { - String urlString = getPreferences(MODE_PRIVATE).getString(packageName, null); - if (DownloaderService.isQueuedOrActive(urlString)) { - activeDownloadUrlString = urlString; - } else { - // this URL is no longer active, remove it - getPreferences(MODE_PRIVATE).edit().remove(packageName).apply(); - } - } - - /** - * Remove progress listener, suppress progress bar, set downloadHandler to null. - */ - private void cleanUpFinishedDownload() { - activeDownloadUrlString = null; - adapter.clearProgress(); - unregisterDownloaderReceiver(); - } - private void onAppChanged() { recyclerView.post(new Runnable() { @Override @@ -641,6 +685,7 @@ public class AppDetails2 extends AppCompatActivity implements ShareChooserDialog } AppDetailsRecyclerViewAdapter adapter = (AppDetailsRecyclerViewAdapter) recyclerView.getAdapter(); adapter.updateItems(app); + refreshStatus(); supportInvalidateOptionsMenu(); } }); @@ -648,7 +693,8 @@ public class AppDetails2 extends AppCompatActivity implements ShareChooserDialog @Override public boolean isAppDownloading() { - return !TextUtils.isEmpty(activeDownloadUrlString); + return currentStatus != null && + (currentStatus.status == AppUpdateStatusManager.Status.PendingDownload || currentStatus.status == AppUpdateStatusManager.Status.Downloading); } @Override @@ -675,8 +721,9 @@ public class AppDetails2 extends AppCompatActivity implements ShareChooserDialog @Override public void installCancel() { - if (!TextUtils.isEmpty(activeDownloadUrlString)) { - InstallManagerService.cancel(this, activeDownloadUrlString); + if (isAppDownloading()) { + InstallManagerService.cancel(this, currentStatus.getUniqueKey()); + adapter.clearProgress(); } } diff --git a/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusManager.java b/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusManager.java index 8e74558a6..71856f76f 100644 --- a/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusManager.java +++ b/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusManager.java @@ -83,7 +83,8 @@ public final class AppUpdateStatusManager { private static final String LOGTAG = "AppUpdateStatusManager"; public enum Status { - Unknown, + PendingDownload, + DownloadInterrupted, UpdateAvailable, Downloading, ReadyToInstall, @@ -120,6 +121,13 @@ public final class AppUpdateStatusManager { public String getUniqueKey() { return apk.getUrl(); } + + /** + * Dumps some information about the status for debugging purposes. + */ + public String toString() { + return app.packageName + " [Status: " + status + ", Progress: " + progressCurrent + " / " + progressMax + "]"; + } } private final Context context; @@ -316,6 +324,21 @@ public final class AppUpdateStatusManager { } } + /** + * @param errorText If null, then it is likely because the user cancelled the download. + */ + public void setDownloadError(String url, @Nullable String errorText) { + synchronized (appMapping) { + AppUpdateStatus entry = appMapping.get(url); + if (entry != null) { + entry.status = Status.DownloadInterrupted; + entry.errorText = errorText; + entry.intent = null; + notifyChange(entry, true); + } + } + } + public void setApkError(Apk apk, String errorText) { synchronized (appMapping) { AppUpdateStatus entry = appMapping.get(apk.getUrl()); diff --git a/app/src/main/java/org/fdroid/fdroid/NotificationHelper.java b/app/src/main/java/org/fdroid/fdroid/NotificationHelper.java index 1c3e05576..7c2baf1ee 100644 --- a/app/src/main/java/org/fdroid/fdroid/NotificationHelper.java +++ b/app/src/main/java/org/fdroid/fdroid/NotificationHelper.java @@ -148,9 +148,12 @@ class NotificationHelper { private boolean shouldIgnoreEntry(AppUpdateStatusManager.AppUpdateStatus entry) { // Ignore unknown status - if (entry.status == AppUpdateStatusManager.Status.Unknown) { + if (entry.status == AppUpdateStatusManager.Status.DownloadInterrupted) { return true; - } else if ((entry.status == AppUpdateStatusManager.Status.Downloading || entry.status == AppUpdateStatusManager.Status.ReadyToInstall || entry.status == AppUpdateStatusManager.Status.InstallError) && + } else if ((entry.status == AppUpdateStatusManager.Status.Downloading || + entry.status == AppUpdateStatusManager.Status.PendingDownload || + entry.status == AppUpdateStatusManager.Status.ReadyToInstall || + entry.status == AppUpdateStatusManager.Status.InstallError) && AppDetails2.isAppVisible(entry.app.packageName)) { // Ignore downloading, readyToInstall and installError if we are showing the details screen for this app return true; 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 ae78434af..a1d115cd0 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/InstallManagerService.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/InstallManagerService.java @@ -166,7 +166,7 @@ public class InstallManagerService extends Service { return START_NOT_STICKY; } - appUpdateStatusManager.addApk(apk, AppUpdateStatusManager.Status.Unknown, null); + appUpdateStatusManager.addApk(apk, AppUpdateStatusManager.Status.PendingDownload, null); appUpdateStatusManager.markAsPendingInstall(urlString); registerApkDownloaderReceivers(urlString); @@ -270,7 +270,7 @@ public class InstallManagerService extends Service { switch (intent.getAction()) { case Downloader.ACTION_STARTED: - // App should currently be in the "Unknown" state, so this changes it to "Downloading". + // App should currently be in the "PendingDownload" state, so this changes it to "Downloading". Intent intentObject = new Intent(context, InstallManagerService.class); intentObject.setAction(ACTION_CANCEL); intentObject.setData(downloadUri); @@ -299,7 +299,7 @@ public class InstallManagerService extends Service { break; case Downloader.ACTION_INTERRUPTED: appUpdateStatusManager.markAsNoLongerPendingInstall(urlString); - appUpdateStatusManager.updateApk(urlString, AppUpdateStatusManager.Status.Unknown, null); + appUpdateStatusManager.setDownloadError(urlString, intent.getStringExtra(Downloader.EXTRA_ERROR_MESSAGE)); localBroadcastManager.unregisterReceiver(this); break; default: diff --git a/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java b/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java index 0dfab580b..86ec8862a 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java +++ b/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java @@ -33,6 +33,7 @@ import android.support.v4.content.LocalBroadcastManager; import android.text.TextUtils; import org.fdroid.fdroid.ProgressListener; +import org.fdroid.fdroid.R; import org.fdroid.fdroid.Utils; import org.fdroid.fdroid.data.SanitizedFile; import org.fdroid.fdroid.installer.ApkCache; @@ -200,7 +201,11 @@ public class DownloaderService extends Service { } }); downloader.download(); - sendBroadcast(uri, Downloader.ACTION_COMPLETE, localFile); + if (downloader.isNotFound()) { + sendBroadcast(uri, Downloader.ACTION_INTERRUPTED, localFile, getString(R.string.download_404)); + } else { + sendBroadcast(uri, Downloader.ACTION_COMPLETE, localFile); + } } catch (InterruptedException e) { sendBroadcast(uri, Downloader.ACTION_INTERRUPTED, localFile); } catch (IOException e) { diff --git a/app/src/main/java/org/fdroid/fdroid/views/updates/UpdatesAdapter.java b/app/src/main/java/org/fdroid/fdroid/views/updates/UpdatesAdapter.java index b756cd3e6..441964611 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/updates/UpdatesAdapter.java +++ b/app/src/main/java/org/fdroid/fdroid/views/updates/UpdatesAdapter.java @@ -98,7 +98,7 @@ public class UpdatesAdapter extends RecyclerView.Adapter Downloading\n%2$s from\n%1$s + The requested file was not found. Updating repositories Processing %2$s / %3$s (%4$d%%) from %1$s Connecting to\n%1$s From 8e2a099e516ca68b9a07cfa2c8db52d361887dce Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Fri, 26 May 2017 09:45:36 +1000 Subject: [PATCH 2/8] Make app status updates include parcelized version of status. This means that we no longer need to receive an APK_URL and then directly ask the status manager for the relevant status object. This causes problems when consecutive updates happen in the same event loop, e.g. download started + download complete. In this case, the receiver will receive two events for the same app. When it asks for the associated status object for the first (download started) event, it will receive a status that says "download complete ready to install". This is because the status object has already been updated by the second event. Furthermore, the broadcast manager must receive a copy of the status object, not the original object. This is because the broadcast manager doesn't parcel the relevant extras until the end of the event loop. This means that if the status is changed twice in one frame, then both parcels will end up looking the same. By sending through a copy instead, this ensures that any listener receives the statuses in the correct order, rather than two parceled versions of the same status notification. --- .../java/org/fdroid/fdroid/AppDetails2.java | 8 +-- .../fdroid/fdroid/AppUpdateStatusManager.java | 61 ++++++++++++++++++- 2 files changed, 63 insertions(+), 6 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/AppDetails2.java b/app/src/main/java/org/fdroid/fdroid/AppDetails2.java index ce988c741..e2fa2978c 100644 --- a/app/src/main/java/org/fdroid/fdroid/AppDetails2.java +++ b/app/src/main/java/org/fdroid/fdroid/AppDetails2.java @@ -527,13 +527,11 @@ public class AppDetails2 extends AppCompatActivity implements ShareChooserDialog private final BroadcastReceiver appStatusReceiver = new BroadcastReceiver() { @Override public void onReceive(Context context, Intent intent) { - String apkUrl = intent.getStringExtra(AppUpdateStatusManager.EXTRA_APK_URL); - AppUpdateStatusManager.AppUpdateStatus status = AppUpdateStatusManager.getInstance(context).get(apkUrl); + AppUpdateStatusManager.AppUpdateStatus status = intent.getParcelableExtra(AppUpdateStatusManager.EXTRA_STATUS); boolean isRemoving = TextUtils.equals(intent.getAction(), AppUpdateStatusManager.BROADCAST_APPSTATUS_REMOVED); - // !TextUtils.equals(status.apk.packageName, app.packageName) - if (status == null && currentStatus != null && isRemoving && !TextUtils.equals(apkUrl, currentStatus.getUniqueKey())) { - Utils.debugLog(TAG, "Ignoring app status change because it belongs to " + apkUrl + " not " + currentStatus.getUniqueKey()); + if (currentStatus != null && isRemoving && !TextUtils.equals(status.getUniqueKey(), currentStatus.getUniqueKey())) { + Utils.debugLog(TAG, "Ignoring app status change because it belongs to " + status.getUniqueKey() + " not " + currentStatus.getUniqueKey()); } else if (status != null && !TextUtils.equals(status.apk.packageName, app.packageName)) { Utils.debugLog(TAG, "Ignoring app status change because it belongs to " + status.apk.packageName + " not " + app.packageName); } else { diff --git a/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusManager.java b/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusManager.java index 71856f76f..31acaa2bf 100644 --- a/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusManager.java +++ b/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusManager.java @@ -7,6 +7,8 @@ import android.content.Context; import android.content.Intent; import android.content.SharedPreferences; import android.content.pm.PackageManager; +import android.os.Parcel; +import android.os.Parcelable; import android.support.annotation.NonNull; import android.support.annotation.Nullable; import android.support.v4.app.TaskStackBuilder; @@ -66,6 +68,7 @@ public final class AppUpdateStatusManager { public static final String BROADCAST_APPSTATUS_REMOVED = "org.fdroid.fdroid.installer.appstatus.appchange.remove"; public static final String EXTRA_APK_URL = "urlstring"; + public static final String EXTRA_STATUS = "status"; public static final String EXTRA_REASON_FOR_CHANGE = "reason"; @@ -102,7 +105,7 @@ public final class AppUpdateStatusManager { private static AppUpdateStatusManager instance; - public class AppUpdateStatus { + public static class AppUpdateStatus implements Parcelable { public final App app; public final Apk apk; public Status status; @@ -128,6 +131,59 @@ public final class AppUpdateStatusManager { public String toString() { return app.packageName + " [Status: " + status + ", Progress: " + progressCurrent + " / " + progressMax + "]"; } + + protected AppUpdateStatus(Parcel in) { + app = in.readParcelable(getClass().getClassLoader()); + apk = in.readParcelable(getClass().getClassLoader()); + intent = in.readParcelable(getClass().getClassLoader()); + status = (Status) in.readSerializable(); + progressCurrent = in.readInt(); + progressMax = in.readInt(); + errorText = in.readString(); + } + + @Override + public void writeToParcel(@NonNull Parcel dest, int flags) { + dest.writeParcelable(app, 0); + dest.writeParcelable(apk, 0); + dest.writeParcelable(intent, 0); + dest.writeSerializable(status); + dest.writeInt(progressCurrent); + dest.writeInt(progressMax); + dest.writeString(errorText); + } + + @Override + public int describeContents() { + return 0; + } + + public static final Parcelable.Creator CREATOR = new Parcelable.Creator() { + @Override + public AppUpdateStatus createFromParcel(Parcel in) { + return new AppUpdateStatus(in); + } + + @Override + public AppUpdateStatus[] newArray(int size) { + return new AppUpdateStatus[size]; + } + }; + + /** + * When passing to the broadcast manager, it is important to pass a copy rather than the original object. + * This is because if two status changes are noticed in the same event loop, than they will both refer + * to the same status object. The objects are not parceled until the end of the event loop, and so the first + * parceled event will refer to the updated object (with a different status) rather than the intended + * status (i.e. the one in existence when talking to the broadcast manager). + */ + public AppUpdateStatus copy() { + AppUpdateStatus copy = new AppUpdateStatus(app, apk, status, intent); + copy.errorText = errorText; + copy.progressCurrent = progressCurrent; + copy.progressMax = progressMax; + return copy; + } } private final Context context; @@ -209,6 +265,7 @@ public final class AppUpdateStatusManager { if (!isBatchUpdating) { Intent broadcastIntent = new Intent(BROADCAST_APPSTATUS_ADDED); broadcastIntent.putExtra(EXTRA_APK_URL, entry.getUniqueKey()); + broadcastIntent.putExtra(EXTRA_STATUS, entry.copy()); localBroadcastManager.sendBroadcast(broadcastIntent); } } @@ -217,6 +274,7 @@ public final class AppUpdateStatusManager { if (!isBatchUpdating) { Intent broadcastIntent = new Intent(BROADCAST_APPSTATUS_CHANGED); broadcastIntent.putExtra(EXTRA_APK_URL, entry.getUniqueKey()); + broadcastIntent.putExtra(EXTRA_STATUS, entry.copy()); broadcastIntent.putExtra(EXTRA_IS_STATUS_UPDATE, isStatusUpdate); localBroadcastManager.sendBroadcast(broadcastIntent); } @@ -226,6 +284,7 @@ public final class AppUpdateStatusManager { if (!isBatchUpdating) { Intent broadcastIntent = new Intent(BROADCAST_APPSTATUS_REMOVED); broadcastIntent.putExtra(EXTRA_APK_URL, entry.getUniqueKey()); + broadcastIntent.putExtra(EXTRA_STATUS, entry.copy()); localBroadcastManager.sendBroadcast(broadcastIntent); } } From 7d1fac2729fa6a142e96de95096b80dc45c3477b Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Fri, 26 May 2017 15:57:50 +1000 Subject: [PATCH 3/8] Extract isDownloading check into method of status class. This is also going to be used elsewhere. --- app/src/main/java/org/fdroid/fdroid/AppDetails2.java | 3 +-- .../main/java/org/fdroid/fdroid/AppUpdateStatusManager.java | 4 ++++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/AppDetails2.java b/app/src/main/java/org/fdroid/fdroid/AppDetails2.java index e2fa2978c..29e88242f 100644 --- a/app/src/main/java/org/fdroid/fdroid/AppDetails2.java +++ b/app/src/main/java/org/fdroid/fdroid/AppDetails2.java @@ -691,8 +691,7 @@ public class AppDetails2 extends AppCompatActivity implements ShareChooserDialog @Override public boolean isAppDownloading() { - return currentStatus != null && - (currentStatus.status == AppUpdateStatusManager.Status.PendingDownload || currentStatus.status == AppUpdateStatusManager.Status.Downloading); + return currentStatus != null && currentStatus.isDownloading(); } @Override diff --git a/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusManager.java b/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusManager.java index 31acaa2bf..453516771 100644 --- a/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusManager.java +++ b/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusManager.java @@ -184,6 +184,10 @@ public final class AppUpdateStatusManager { copy.progressMax = progressMax; return copy; } + + public boolean isDownloading() { + return status == Status.Downloading || status == Status.PendingDownload; + } } private final Context context; From b69587ca65260286e95eea9e5df4a3c0f9e4b6df Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Fri, 26 May 2017 15:59:38 +1000 Subject: [PATCH 4/8] Make contract about nullable currentApp explicit. This identified a couple of places where it needed to be guarded against. --- .../fdroid/fdroid/views/apps/AppListItemController.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/views/apps/AppListItemController.java b/app/src/main/java/org/fdroid/fdroid/views/apps/AppListItemController.java index c9e387e23..5a3ce3af0 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/apps/AppListItemController.java +++ b/app/src/main/java/org/fdroid/fdroid/views/apps/AppListItemController.java @@ -47,7 +47,6 @@ import org.fdroid.fdroid.net.DownloaderService; import java.io.File; // TODO: Support cancelling of downloads by tapping the install button a second time. -// TODO: Support installing of an app once downloaded by tapping the install button a second time. public class AppListItemController extends RecyclerView.ViewHolder { private static final String TAG = "AppListItemController"; @@ -79,13 +78,15 @@ public class AppListItemController extends RecyclerView.ViewHolder { private final ImageButton cancelButton; /** - * Will operate as the "Download is complete, click to (install|update)" button. + * Will operate as the "Download is complete, click to (install|update)" button, as well as the + * "Installed successfully, click to run" button. */ @Nullable private final Button actionButton; private final DisplayImageOptions displayImageOptions; + @Nullable private App currentApp; private String currentAppDownloadUrl; @@ -416,7 +417,9 @@ public class AppListItemController extends RecyclerView.ViewHolder { cancelButton.setVisibility(View.GONE); } - configureActionButton(currentApp); + if (currentApp != null) { + configureActionButton(currentApp); + } } @SuppressWarnings("FieldCanBeLocal") From 2a9fefd54e933d35ddb433a1282d1e841a33f8fc Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Fri, 26 May 2017 16:45:53 +1000 Subject: [PATCH 5/8] Simplify logic in UpdatesAdapter. No longer do we try to nicely maintain the state of the adapter in "Updates" in order to notify the recycler view about changes to its underlying data. Instead, we just rebuild the entire structure each time a new thing needs to be shown/removed. This means no more smooth scrolling to the relevant item after it is changed, but it results in a far less buggy interface. --- .../fdroid/views/updates/UpdatesAdapter.java | 119 +++++------------- 1 file changed, 32 insertions(+), 87 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/views/updates/UpdatesAdapter.java b/app/src/main/java/org/fdroid/fdroid/views/updates/UpdatesAdapter.java index 441964611..59c7507e5 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/updates/UpdatesAdapter.java +++ b/app/src/main/java/org/fdroid/fdroid/views/updates/UpdatesAdapter.java @@ -6,15 +6,12 @@ import android.content.Intent; import android.content.IntentFilter; import android.database.Cursor; import android.os.Bundle; -import android.support.annotation.Nullable; import android.support.v4.app.LoaderManager; import android.support.v4.content.CursorLoader; import android.support.v4.content.Loader; import android.support.v4.content.LocalBroadcastManager; import android.support.v7.app.AppCompatActivity; -import android.support.v7.widget.LinearLayoutManager; import android.support.v7.widget.RecyclerView; -import android.text.TextUtils; import android.view.ViewGroup; import com.hannesdorfmann.adapterdelegates3.AdapterDelegatesManager; @@ -33,7 +30,9 @@ import org.fdroid.fdroid.views.updates.items.UpdateableAppsHeader; import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; +import java.util.HashSet; import java.util.List; +import java.util.Set; /** * Manages the following types of information: @@ -68,9 +67,6 @@ public class UpdatesAdapter extends RecyclerView.Adapter appsToShowStatus = new ArrayList<>(); private final List appsToPromptForDonation = new ArrayList<>(); private final List appsToNotifyAbout = new ArrayList<>(); @@ -135,17 +131,6 @@ public class UpdatesAdapter extends RecyclerView.Adapter toShowStatusPackageNames = new HashSet<>(appsToShowStatus.size()); + for (AppStatus app : appsToShowStatus) { + toShowStatusPackageNames.add(app.status.app.packageName); + items.add(app); + } - if (updateableApps != null && updateableApps.size() > 0) { - items.add(new UpdateableAppsHeader(activity, this, updateableApps)); - if (showAllUpdateableApps) { - items.addAll(updateableApps); + if (updateableApps != null) { + // Only count/show apps which are not shown above in the "Apps to show status" list. + List updateableAppsToShow = new ArrayList<>(updateableApps.size()); + for (UpdateableApp app : updateableApps) { + if (!toShowStatusPackageNames.contains(app.app.packageName)) { + updateableAppsToShow.add(app); + } + } + + if (updateableAppsToShow.size() > 0) { + items.add(new UpdateableAppsHeader(activity, this, updateableAppsToShow)); + + if (showAllUpdateableApps) { + items.addAll(updateableAppsToShow); + } } } @@ -189,12 +189,6 @@ public class UpdatesAdapter extends RecyclerView.Adapter onCreateLoader(int id, Bundle args) { return new CursorLoader( @@ -224,12 +218,7 @@ public class UpdatesAdapter extends RecyclerView.Adapter loader, Cursor cursor) { - int numberRemoved = updateableApps.size(); - boolean hadHeader = updateableApps.size() > 0; - boolean willHaveHeader = cursor.getCount() > 0; - updateableApps.clear(); - notifyItemRangeRemoved(appsToShowStatus.size(), numberRemoved + (hadHeader ? 1 : 0)); cursor.moveToFirst(); while (!cursor.isAfterLast()) { @@ -238,7 +227,7 @@ public class UpdatesAdapter extends RecyclerView.Adapter 0) { - int size = appsToShowStatus.size(); - appsToShowStatus.clear(); - notifyItemRangeRemoved(0, size); - } - populateAppStatuses(); - notifyItemRangeInserted(0, appsToShowStatus.size()); - - if (recyclerView != null) { - recyclerView.smoothScrollToPosition(0); - } + notifyDataSetChanged(); } - private void onAppStatusAdded(String apkUrl) { - // We could try and find the specific place where we need to add our new item, but it is - // far simpler to clear the list and rebuild it (sorting it in the process). + private void onAppStatusAdded() { appsToShowStatus.clear(); populateAppStatuses(); - - // After adding the new item to our list (somewhere) we can then look it back up again in - // order to notify the recycler view and scroll to that item. - int positionOfNewApp = -1; - for (int i = 0; i < appsToShowStatus.size(); i++) { - if (TextUtils.equals(appsToShowStatus.get(i).status.getUniqueKey(), apkUrl)) { - positionOfNewApp = i; - break; - } - } - - if (positionOfNewApp != -1) { - notifyItemInserted(positionOfNewApp); - - if (recyclerView != null) { - recyclerView.smoothScrollToPosition(positionOfNewApp); - } - } + notifyDataSetChanged(); } - private void onAppStatusRemoved(String apkUrl) { - // Find out where the item is in our internal data structure, so that we can remove it and - // also notify the recycler view appropriately. - int positionOfOldApp = -1; - for (int i = 0; i < appsToShowStatus.size(); i++) { - if (TextUtils.equals(appsToShowStatus.get(i).status.getUniqueKey(), apkUrl)) { - positionOfOldApp = i; - break; - } - } - - if (positionOfOldApp != -1) { - appsToShowStatus.remove(positionOfOldApp); - - populateItems(); - notifyItemRemoved(positionOfOldApp); - } + private void onAppStatusRemoved() { + appsToShowStatus.clear(); + populateAppStatuses(); + notifyDataSetChanged(); } private final BroadcastReceiver receiverAppStatusChanges = new BroadcastReceiver() { @Override public void onReceive(Context context, Intent intent) { - String apkUrl = intent.getStringExtra(AppUpdateStatusManager.EXTRA_APK_URL); - switch (intent.getAction()) { case AppUpdateStatusManager.BROADCAST_APPSTATUS_LIST_CHANGED: onManyAppStatusesChanged(intent.getStringExtra(AppUpdateStatusManager.EXTRA_REASON_FOR_CHANGE)); break; case AppUpdateStatusManager.BROADCAST_APPSTATUS_ADDED: - onAppStatusAdded(apkUrl); + onAppStatusAdded(); break; case AppUpdateStatusManager.BROADCAST_APPSTATUS_REMOVED: - onAppStatusRemoved(apkUrl); + onAppStatusRemoved(); break; } } From b3ed64ddaf3597b828dad7935fdaad46955fc397 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Fri, 26 May 2017 16:46:04 +1000 Subject: [PATCH 6/8] Simplify and clean up updates view logic. Same as how AppDetails2 was recently cleaned up to depend more on AppUpdateStatusManager. In addition, it also removes items from the "X apps can be updated" lower part of the "Updates" view when they are present in the upper half (i.e. the half showing feedback about the current download/install progress). --- .../views/apps/AppListItemController.java | 169 ++++++++---------- 1 file changed, 79 insertions(+), 90 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/views/apps/AppListItemController.java b/app/src/main/java/org/fdroid/fdroid/views/apps/AppListItemController.java index 5a3ce3af0..03f9ab456 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/apps/AppListItemController.java +++ b/app/src/main/java/org/fdroid/fdroid/views/apps/AppListItemController.java @@ -41,10 +41,9 @@ import org.fdroid.fdroid.installer.ApkCache; import org.fdroid.fdroid.installer.InstallManagerService; import org.fdroid.fdroid.installer.Installer; import org.fdroid.fdroid.installer.InstallerFactory; -import org.fdroid.fdroid.net.Downloader; -import org.fdroid.fdroid.net.DownloaderService; import java.io.File; +import java.util.Iterator; // TODO: Support cancelling of downloads by tapping the install button a second time. public class AppListItemController extends RecyclerView.ViewHolder { @@ -88,7 +87,9 @@ public class AppListItemController extends RecyclerView.ViewHolder { @Nullable private App currentApp; - private String currentAppDownloadUrl; + + @Nullable + private AppUpdateStatusManager.AppUpdateStatus currentStatus; @TargetApi(21) public AppListItemController(final Activity activity, View itemView) { @@ -139,22 +140,39 @@ public class AppListItemController extends RecyclerView.ViewHolder { itemView.setOnClickListener(onAppClicked); } + /** + * Figures out the current install/update/download/etc status for the app we are viewing. + * Then, asks the view to update itself to reflect this status. + */ + private void refreshStatus(@NonNull App app) { + Iterator statuses = AppUpdateStatusManager.getInstance(activity).getByPackageName(app.packageName).iterator(); + if (statuses.hasNext()) { + AppUpdateStatusManager.AppUpdateStatus status = statuses.next(); + updateAppStatus(app, status); + } else { + currentStatus = null; + } + } + public void bindModel(@NonNull App app) { currentApp = app; ImageLoader.getInstance().displayImage(app.iconUrl, icon, displayImageOptions); - Apk apkToInstall = ApkProvider.Helper.findApkFromAnyRepo(activity, app.packageName, app.suggestedVersionCode); - currentAppDownloadUrl = apkToInstall.getUrl(); + refreshStatus(app); final LocalBroadcastManager broadcastManager = LocalBroadcastManager.getInstance(activity.getApplicationContext()); - broadcastManager.unregisterReceiver(onDownloadProgress); broadcastManager.unregisterReceiver(onInstallAction); - broadcastManager.unregisterReceiver(onStatusRemoved); + broadcastManager.unregisterReceiver(onStatusChanged); - broadcastManager.registerReceiver(onDownloadProgress, DownloaderService.getIntentFilter(currentAppDownloadUrl)); - broadcastManager.registerReceiver(onInstallAction, Installer.getInstallIntentFilter(Uri.parse(currentAppDownloadUrl))); - broadcastManager.registerReceiver(onStatusRemoved, new IntentFilter(AppUpdateStatusManager.BROADCAST_APPSTATUS_REMOVED)); + // broadcastManager.registerReceiver(onInstallAction, Installer.getInstallIntentFilter(Uri.parse(currentAppDownloadUrl))); + + + IntentFilter intentFilter = new IntentFilter(); + intentFilter.addAction(AppUpdateStatusManager.BROADCAST_APPSTATUS_ADDED); + intentFilter.addAction(AppUpdateStatusManager.BROADCAST_APPSTATUS_REMOVED); + intentFilter.addAction(AppUpdateStatusManager.BROADCAST_APPSTATUS_CHANGED); + broadcastManager.registerReceiver(onStatusChanged, intentFilter); configureAppName(app); configureStatusText(app); @@ -243,34 +261,6 @@ public class AppListItemController extends RecyclerView.ViewHolder { return false; } - /** - * Queries the {@link AppUpdateStatusManager} to find out if there are any apks corresponding to - * `app` which are in the process of being downloaded. - */ - private boolean isDownloading(@NonNull App app) { - for (AppUpdateStatusManager.AppUpdateStatus appStatus : AppUpdateStatusManager.getInstance(activity).getByPackageName(app.packageName)) { - if (appStatus.status == AppUpdateStatusManager.Status.Downloading) { - return true; - } - } - return false; - } - - /** - * Queries the {@link AppUpdateStatusManager} and asks if the app was just successfully installed. - * For convenience, returns the {@link org.fdroid.fdroid.AppUpdateStatusManager.AppUpdateStatus} - * object if it was sucessfully installed, or null otherwise. - */ - @Nullable - private AppUpdateStatusManager.AppUpdateStatus wasSuccessfullyInstalled(@NonNull App app) { - for (AppUpdateStatusManager.AppUpdateStatus appStatus : AppUpdateStatusManager.getInstance(activity).getByPackageName(app.packageName)) { - if (appStatus.status == AppUpdateStatusManager.Status.Installed) { - return appStatus; - } - } - return null; - } - /** * The app name {@link TextView} is used for a few reasons: *
  • Display name + summary of the app (most common). @@ -278,7 +268,7 @@ public class AppListItemController extends RecyclerView.ViewHolder { *
  • If downloaded and ready to install, mention that it is ready to update/install. */ private void configureAppName(@NonNull App app) { - if (isReadyToInstall(app)) { + if (currentStatus != null && currentStatus.status == AppUpdateStatusManager.Status.ReadyToInstall) { if (app.isInstalled()) { String appName = activity.getString(R.string.app_list__name__downloaded_and_ready_to_update, app.name); if (app.lastUpdated != null) { @@ -301,9 +291,9 @@ public class AppListItemController extends RecyclerView.ViewHolder { } else { name.setText(activity.getString(R.string.app_list__name__downloaded_and_ready_to_install, app.name)); } - } else if (isDownloading(app)) { + } else if (currentStatus != null && currentStatus.isDownloading()) { name.setText(activity.getString(R.string.app_list__name__downloading_in_progress, app.name)); - } else if (wasSuccessfullyInstalled(app) != null) { + } else if (currentStatus != null && currentStatus.status == AppUpdateStatusManager.Status.Installed) { name.setText(activity.getString(R.string.app_list__name__successfully_installed, app.name)); } else { name.setText(Utils.formatAppNameAndSummary(app.name, app.summary)); @@ -322,13 +312,13 @@ public class AppListItemController extends RecyclerView.ViewHolder { actionButton.setVisibility(View.VISIBLE); - if (wasSuccessfullyInstalled(app) != null) { - if (activity.getPackageManager().getLaunchIntentForPackage(currentApp.packageName) != null) { + if (currentStatus != null && currentStatus.status == AppUpdateStatusManager.Status.Installed) { + if (activity.getPackageManager().getLaunchIntentForPackage(app.packageName) != null) { actionButton.setText(R.string.menu_launch); } else { actionButton.setVisibility(View.GONE); } - } else if (isReadyToInstall(app)) { + } else if (currentStatus != null && currentStatus.status == AppUpdateStatusManager.Status.ReadyToInstall) { if (app.isInstalled()) { actionButton.setText(R.string.app__install_downloaded_update); } else { @@ -446,24 +436,45 @@ public class AppListItemController extends RecyclerView.ViewHolder { * Updates both the progress bar and the circular install button (which shows progress around the outside of the circle). * Also updates the app label to indicate that the app is being downloaded. */ - private final BroadcastReceiver onDownloadProgress = new BroadcastReceiver() { + private void updateAppStatus(@NonNull App app, @NonNull AppUpdateStatusManager.AppUpdateStatus status) { + currentStatus = status; + + configureAppName(app); + configureActionButton(app); + + switch (status.status) { + case PendingDownload: + onDownloadStarted(); + break; + + case Downloading: + onDownloadProgressUpdated(status.progressCurrent, status.progressMax); + break; + + case ReadyToInstall: + onDownloadComplete(); + break; + + + case Installed: + case Installing: + case InstallError: + case UpdateAvailable: + case DownloadInterrupted: + break; + } + } + + private final BroadcastReceiver onStatusChanged = new BroadcastReceiver() { @Override public void onReceive(Context context, Intent intent) { - if (currentApp == null || !TextUtils.equals(currentAppDownloadUrl, intent.getDataString()) || (installButton == null && progressBar == null)) { + AppUpdateStatusManager.AppUpdateStatus newStatus = intent.getParcelableExtra(AppUpdateStatusManager.EXTRA_STATUS); + + if (currentApp == null || !TextUtils.equals(newStatus.app.packageName, currentApp.packageName) || (installButton == null && progressBar == null)) { return; } - configureAppName(currentApp); - - if (Downloader.ACTION_STARTED.equals(intent.getAction())) { - onDownloadStarted(); - } else if (Downloader.ACTION_PROGRESS.equals(intent.getAction())) { - int bytesRead = intent.getIntExtra(Downloader.EXTRA_BYTES_READ, 0); - int totalBytes = intent.getIntExtra(Downloader.EXTRA_TOTAL_BYTES, 0); - onDownloadProgressUpdated(bytesRead, totalBytes); - } else if (Downloader.ACTION_COMPLETE.equals(intent.getAction())) { - onDownloadComplete(); - } + updateAppStatus(currentApp, newStatus); } }; @@ -495,26 +506,6 @@ public class AppListItemController extends RecyclerView.ViewHolder { } }; - /** - * If the app goes from "Successfully installed" to anything else, then reset the action button - * and the app label text to whatever they should be. - */ - private final BroadcastReceiver onStatusRemoved = new BroadcastReceiver() { - @Override - public void onReceive(Context context, Intent intent) { - if (currentApp == null || currentAppDownloadUrl == null) { - return; - } - - if (!TextUtils.equals(intent.getStringExtra(AppUpdateStatusManager.EXTRA_APK_URL), currentAppDownloadUrl)) { - return; - } - - configureAppName(currentApp); - configureActionButton(currentApp); - } - }; - @SuppressWarnings("FieldCanBeLocal") private final View.OnClickListener onActionClicked = new View.OnClickListener() { @Override @@ -524,8 +515,7 @@ public class AppListItemController extends RecyclerView.ViewHolder { } // When the button says "Run", then launch the app. - AppUpdateStatusManager.AppUpdateStatus successfullyInstalledStatus = wasSuccessfullyInstalled(currentApp); - if (successfullyInstalledStatus != null) { + if (currentStatus != null && currentStatus.status == AppUpdateStatusManager.Status.Installed) { Intent intent = activity.getPackageManager().getLaunchIntentForPackage(currentApp.packageName); if (intent != null) { activity.startActivity(intent); @@ -533,16 +523,14 @@ public class AppListItemController extends RecyclerView.ViewHolder { // Once it is explicitly launched by the user, then we can pretty much forget about // any sort of notification that the app was successfully installed. It should be // apparent to the user because they just launched it. - AppUpdateStatusManager.getInstance(activity).removeApk(successfullyInstalledStatus.getUniqueKey()); + AppUpdateStatusManager.getInstance(activity).removeApk(currentStatus.getUniqueKey()); } return; } - final Apk suggestedApk = ApkProvider.Helper.findApkFromAnyRepo(activity, currentApp.packageName, currentApp.suggestedVersionCode); - - if (isReadyToInstall(currentApp)) { - File apkFilePath = ApkCache.getApkDownloadPath(activity, Uri.parse(suggestedApk.getUrl())); - Utils.debugLog(TAG, "skip download, we have already downloaded " + suggestedApk.getUrl() + " to " + apkFilePath); + if (currentStatus != null && currentStatus.status == AppUpdateStatusManager.Status.ReadyToInstall) { + File apkFilePath = ApkCache.getApkDownloadPath(activity, Uri.parse(currentStatus.apk.getUrl())); + Utils.debugLog(TAG, "skip download, we have already downloaded " + currentStatus.apk.getUrl() + " to " + apkFilePath); // TODO: This seems like a bit of a hack. Is there a better way to do this by changing // the Installer API so that we can ask it to install without having to get it to fire @@ -562,10 +550,11 @@ public class AppListItemController extends RecyclerView.ViewHolder { } }; - broadcastManager.registerReceiver(receiver, Installer.getInstallIntentFilter(Uri.parse(suggestedApk.getUrl()))); - Installer installer = InstallerFactory.create(activity, suggestedApk); - installer.installPackage(Uri.parse(apkFilePath.toURI().toString()), Uri.parse(suggestedApk.getUrl())); + broadcastManager.registerReceiver(receiver, Installer.getInstallIntentFilter(Uri.parse(currentStatus.apk.getUrl()))); + Installer installer = InstallerFactory.create(activity, currentStatus.apk); + installer.installPackage(Uri.parse(apkFilePath.toURI().toString()), Uri.parse(currentStatus.apk.getUrl())); } else { + final Apk suggestedApk = ApkProvider.Helper.findApkFromAnyRepo(activity, currentApp.packageName, currentApp.suggestedVersionCode); InstallManagerService.queue(activity, currentApp, suggestedApk); } } @@ -575,11 +564,11 @@ public class AppListItemController extends RecyclerView.ViewHolder { private final View.OnClickListener onCancelDownload = new View.OnClickListener() { @Override public void onClick(View v) { - if (currentAppDownloadUrl == null) { + if (currentStatus == null || !currentStatus.isDownloading()) { return; } - InstallManagerService.cancel(activity, currentAppDownloadUrl); + InstallManagerService.cancel(activity, currentStatus.getUniqueKey()); } }; } From a656e8e1336a7e3fb8e03e953238964a5f02797c Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Fri, 26 May 2017 16:04:40 +1000 Subject: [PATCH 7/8] Remove dead code. Fixes #1013. --- .../fdroid/views/updates/UpdatesAdapter.java | 13 +---- .../views/updates/items/AppNotification.java | 56 ------------------- .../views/updates/items/DonationPrompt.java | 54 ------------------ 3 files changed, 2 insertions(+), 121 deletions(-) delete mode 100644 app/src/main/java/org/fdroid/fdroid/views/updates/items/AppNotification.java delete mode 100644 app/src/main/java/org/fdroid/fdroid/views/updates/items/DonationPrompt.java diff --git a/app/src/main/java/org/fdroid/fdroid/views/updates/UpdatesAdapter.java b/app/src/main/java/org/fdroid/fdroid/views/updates/UpdatesAdapter.java index 59c7507e5..96dabb24a 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/updates/UpdatesAdapter.java +++ b/app/src/main/java/org/fdroid/fdroid/views/updates/UpdatesAdapter.java @@ -20,10 +20,8 @@ import org.fdroid.fdroid.AppUpdateStatusManager; import org.fdroid.fdroid.data.App; import org.fdroid.fdroid.data.AppProvider; import org.fdroid.fdroid.data.Schema; -import org.fdroid.fdroid.views.updates.items.AppNotification; import org.fdroid.fdroid.views.updates.items.AppStatus; import org.fdroid.fdroid.views.updates.items.AppUpdateData; -import org.fdroid.fdroid.views.updates.items.DonationPrompt; import org.fdroid.fdroid.views.updates.items.UpdateableApp; import org.fdroid.fdroid.views.updates.items.UpdateableAppsHeader; @@ -45,8 +43,8 @@ import java.util.Set; * + Once "Show apps" is expanded then each app is shown along with its own download button. * * It does this by maintaining several different lists of interesting apps. Each list contains wrappers - * around the piece of data it wants to render ({@link AppStatus}, {@link DonationPrompt}, - * {@link AppNotification}, {@link UpdateableApp}). Instead of juggling the various viewTypes + * around the piece of data it wants to render ({@link AppStatus}, {@link UpdateableApp}). + * Instead of juggling the various viewTypes * to find out which position in the adapter corresponds to which view type, this is handled by * the {@link UpdatesAdapter#delegatesManager}. * @@ -68,8 +66,6 @@ public class UpdatesAdapter extends RecyclerView.Adapter appsToShowStatus = new ArrayList<>(); - private final List appsToPromptForDonation = new ArrayList<>(); - private final List appsToNotifyAbout = new ArrayList<>(); private final List updateableApps = new ArrayList<>(); private boolean showAllUpdateableApps = false; @@ -78,8 +74,6 @@ public class UpdatesAdapter extends RecyclerView.Adapter> { - - @Override - protected boolean isForViewType(@NonNull List items, int position) { - return items.get(position) instanceof AppNotification; - } - - @NonNull - @Override - protected RecyclerView.ViewHolder onCreateViewHolder(ViewGroup parent) { - return new ViewHolder(new TextView(parent.getContext())); - } - - @Override - protected void onBindViewHolder(@NonNull List items, int position, @NonNull RecyclerView.ViewHolder holder, @NonNull List payloads) { - AppNotification app = (AppNotification) items.get(position); - ((ViewHolder) holder).bindApp(app); - } - } - - public static class ViewHolder extends RecyclerView.ViewHolder { - public ViewHolder(View itemView) { - super(itemView); - } - - public void bindApp(AppNotification app) { - ((TextView) itemView).setText(""); - } - } - -} diff --git a/app/src/main/java/org/fdroid/fdroid/views/updates/items/DonationPrompt.java b/app/src/main/java/org/fdroid/fdroid/views/updates/items/DonationPrompt.java deleted file mode 100644 index 697860703..000000000 --- a/app/src/main/java/org/fdroid/fdroid/views/updates/items/DonationPrompt.java +++ /dev/null @@ -1,54 +0,0 @@ -package org.fdroid.fdroid.views.updates.items; - -import android.app.Activity; -import android.support.annotation.NonNull; -import android.support.v7.widget.RecyclerView; -import android.view.View; -import android.view.ViewGroup; -import android.widget.TextView; - -import com.hannesdorfmann.adapterdelegates3.AdapterDelegate; - -import java.util.List; - -/** - * The app (if any) which we should prompt the user about potentially donating to (due to having - * updated several times). - */ -public class DonationPrompt extends AppUpdateData { - - public DonationPrompt(Activity activity) { - super(activity); - } - - public static class Delegate extends AdapterDelegate> { - - @Override - protected boolean isForViewType(@NonNull List items, int position) { - return items.get(position) instanceof DonationPrompt; - } - - @NonNull - @Override - protected RecyclerView.ViewHolder onCreateViewHolder(ViewGroup parent) { - return new ViewHolder(new TextView(parent.getContext())); - } - - @Override - protected void onBindViewHolder(@NonNull List items, int position, @NonNull RecyclerView.ViewHolder holder, @NonNull List payloads) { - DonationPrompt app = (DonationPrompt) items.get(position); - ((ViewHolder) holder).bindApp(app); - } - } - - public static class ViewHolder extends RecyclerView.ViewHolder { - public ViewHolder(View itemView) { - super(itemView); - } - - public void bindApp(DonationPrompt app) { - ((TextView) itemView).setText(""); - } - } - -} From f617402f323b15ecf680dbe461ecc180fa2dd232 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Wed, 31 May 2017 09:43:05 +1000 Subject: [PATCH 8/8] Remove PendingDownload in favour of Downloading The response to receiving PendingDownload was always a more specific case of the Downloading event. By removing it, the code which was listening for Downloading events is capable of doing everything that the PendingDownloading listeners were doing. --- .../java/org/fdroid/fdroid/AppDetails2.java | 8 ++----- .../fdroid/fdroid/AppUpdateStatusManager.java | 4 ---- .../org/fdroid/fdroid/NotificationHelper.java | 1 - .../installer/InstallManagerService.java | 2 +- .../views/apps/AppListItemController.java | 24 ++----------------- .../fdroid/views/updates/UpdatesAdapter.java | 3 +-- 6 files changed, 6 insertions(+), 36 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/AppDetails2.java b/app/src/main/java/org/fdroid/fdroid/AppDetails2.java index 29e88242f..71d09d26f 100644 --- a/app/src/main/java/org/fdroid/fdroid/AppDetails2.java +++ b/app/src/main/java/org/fdroid/fdroid/AppDetails2.java @@ -480,14 +480,10 @@ public class AppDetails2 extends AppCompatActivity implements ShareChooserDialog } switch (newStatus.status) { - case PendingDownload: - adapter.setProgress(-1, -1, R.string.download_pending); - break; - case Downloading: if (newStatus.progressMax == 0) { // The first progress notification we get telling us our status is "Downloading" - adapter.setProgress(-1, -1, 0); + adapter.setProgress(-1, -1, R.string.download_pending); } else { adapter.setProgress(newStatus.progressCurrent, newStatus.progressMax, 0); } @@ -691,7 +687,7 @@ public class AppDetails2 extends AppCompatActivity implements ShareChooserDialog @Override public boolean isAppDownloading() { - return currentStatus != null && currentStatus.isDownloading(); + return currentStatus != null && currentStatus.status == AppUpdateStatusManager.Status.Downloading; } @Override diff --git a/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusManager.java b/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusManager.java index 453516771..31acaa2bf 100644 --- a/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusManager.java +++ b/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusManager.java @@ -184,10 +184,6 @@ public final class AppUpdateStatusManager { copy.progressMax = progressMax; return copy; } - - public boolean isDownloading() { - return status == Status.Downloading || status == Status.PendingDownload; - } } private final Context context; diff --git a/app/src/main/java/org/fdroid/fdroid/NotificationHelper.java b/app/src/main/java/org/fdroid/fdroid/NotificationHelper.java index 7c2baf1ee..a9d9fd4b7 100644 --- a/app/src/main/java/org/fdroid/fdroid/NotificationHelper.java +++ b/app/src/main/java/org/fdroid/fdroid/NotificationHelper.java @@ -151,7 +151,6 @@ class NotificationHelper { if (entry.status == AppUpdateStatusManager.Status.DownloadInterrupted) { return true; } else if ((entry.status == AppUpdateStatusManager.Status.Downloading || - entry.status == AppUpdateStatusManager.Status.PendingDownload || entry.status == AppUpdateStatusManager.Status.ReadyToInstall || entry.status == AppUpdateStatusManager.Status.InstallError) && AppDetails2.isAppVisible(entry.app.packageName)) { 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 a1d115cd0..c429d139c 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/InstallManagerService.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/InstallManagerService.java @@ -166,7 +166,7 @@ public class InstallManagerService extends Service { return START_NOT_STICKY; } - appUpdateStatusManager.addApk(apk, AppUpdateStatusManager.Status.PendingDownload, null); + appUpdateStatusManager.addApk(apk, AppUpdateStatusManager.Status.Downloading, null); appUpdateStatusManager.markAsPendingInstall(urlString); registerApkDownloaderReceivers(urlString); diff --git a/app/src/main/java/org/fdroid/fdroid/views/apps/AppListItemController.java b/app/src/main/java/org/fdroid/fdroid/views/apps/AppListItemController.java index 03f9ab456..0b4802d47 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/apps/AppListItemController.java +++ b/app/src/main/java/org/fdroid/fdroid/views/apps/AppListItemController.java @@ -291,7 +291,7 @@ public class AppListItemController extends RecyclerView.ViewHolder { } else { name.setText(activity.getString(R.string.app_list__name__downloaded_and_ready_to_install, app.name)); } - } else if (currentStatus != null && currentStatus.isDownloading()) { + } else if (currentStatus != null && currentStatus.status == AppUpdateStatusManager.Status.Downloading) { name.setText(activity.getString(R.string.app_list__name__downloading_in_progress, app.name)); } else if (currentStatus != null && currentStatus.status == AppUpdateStatusManager.Status.Installed) { name.setText(activity.getString(R.string.app_list__name__successfully_installed, app.name)); @@ -355,22 +355,6 @@ public class AppListItemController extends RecyclerView.ViewHolder { } } - private void onDownloadStarted() { - if (installButton != null) { - installButton.setImageDrawable(ContextCompat.getDrawable(activity, R.drawable.ic_download_progress)); - installButton.setImageLevel(0); - } - - if (progressBar != null) { - progressBar.setVisibility(View.VISIBLE); - progressBar.setIndeterminate(true); - } - - if (cancelButton != null) { - cancelButton.setVisibility(View.VISIBLE); - } - } - private void onDownloadProgressUpdated(int bytesRead, int totalBytes) { if (installButton != null) { installButton.setImageDrawable(ContextCompat.getDrawable(activity, R.drawable.ic_download_progress)); @@ -443,10 +427,6 @@ public class AppListItemController extends RecyclerView.ViewHolder { configureActionButton(app); switch (status.status) { - case PendingDownload: - onDownloadStarted(); - break; - case Downloading: onDownloadProgressUpdated(status.progressCurrent, status.progressMax); break; @@ -564,7 +544,7 @@ public class AppListItemController extends RecyclerView.ViewHolder { private final View.OnClickListener onCancelDownload = new View.OnClickListener() { @Override public void onClick(View v) { - if (currentStatus == null || !currentStatus.isDownloading()) { + if (currentStatus == null || currentStatus.status != AppUpdateStatusManager.Status.Downloading) { return; } diff --git a/app/src/main/java/org/fdroid/fdroid/views/updates/UpdatesAdapter.java b/app/src/main/java/org/fdroid/fdroid/views/updates/UpdatesAdapter.java index 96dabb24a..a73d8a3af 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/updates/UpdatesAdapter.java +++ b/app/src/main/java/org/fdroid/fdroid/views/updates/UpdatesAdapter.java @@ -88,8 +88,7 @@ public class UpdatesAdapter extends RecyclerView.Adapter