Karl/BobStore
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove download notification when last queued download cancelled.

Browse Source 
There was a race condition such that when the cancel `Intent` is
received, it would queue up a message on the download thread to stop
downloading. This was followed by cancelling the notification on the
thread the `Intent` was received on. As a result of the message taking
some time to get to the other thread, it would likely cause another
progress notification to be shown after we had already removed that
notification - making it show again after being cancelled.

The code in the download thread progress handler, which updates the
notification with progress, is not perfect. It does a check before
deciding to broadcast progress and update the notification. However,
the check happens at the beginning of the group of expressions. As
such, there is likely a small change that `isActive` returns true,
then the notification is cancelled on the other thread, and finally,
the download thread updates the progress again. However this is better
than it was before where the notification didn't go away.

Previously, apks were not being removed from the queue once download
was completed, only when removed. It didn't cause any specific issues
here, but I removed downloads from the queue after completion to prevent
potential problems in the future. Now the queue is a better representation
of what is to be downloaded.
This commit is contained in:
Peter Serwylo 2016-04-22 22:53:59 +10:00
parent bbf7cd81a6
commit b66810944f
1 changed files with 25 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java
View File

@ -133,7 +133,6 @@ public class DownloaderService extends Service {
} }
if (ACTION_CANCEL.equals(intent.getAction())) { if (ACTION_CANCEL.equals(intent.getAction())) {
Log.i(TAG, "Removed " + intent); Log.i(TAG, "Removed " + intent);
Integer what = QUEUE_WHATS.remove(uriString);
if (isQueued(uriString)) { if (isQueued(uriString)) {
serviceHandler.removeMessages(what); serviceHandler.removeMessages(what);
} else if (isActive(uriString)) { } else if (isActive(uriString)) {
@ -141,6 +140,11 @@ public class DownloaderService extends Service {
} else { } else {
Log.e(TAG, "CANCEL called on something not queued or running: " + startId + " " + intent); Log.e(TAG, "CANCEL called on something not queued or running: " + startId + " " + intent);
} }
QUEUE_WHATS.remove(uriString);
if (isQueueEmpty()) {
stopForeground(true);
}
} else if (ACTION_QUEUE.equals(intent.getAction())) { } else if (ACTION_QUEUE.equals(intent.getAction())) {
if (Preferences.get().isUpdateNotificationEnabled()) { if (Preferences.get().isUpdateNotificationEnabled()) {
Notification notification = createNotification(intent.getDataString(), getPackageNameFromIntent(intent)).build(); Notification notification = createNotification(intent.getDataString(), getPackageNameFromIntent(intent)).build();
@ -262,17 +266,19 @@ public class DownloaderService extends Service {
downloader.setListener(new Downloader.DownloaderProgressListener() { downloader.setListener(new Downloader.DownloaderProgressListener() {
@Override @Override
public void sendProgress(URL sourceUrl, int bytesRead, int totalBytes) { public void sendProgress(URL sourceUrl, int bytesRead, int totalBytes) {
Intent intent = new Intent(Downloader.ACTION_PROGRESS); if (isActive(uri.toString())) {
intent.setData(uri); Intent intent = new Intent(Downloader.ACTION_PROGRESS);
intent.putExtra(Downloader.EXTRA_BYTES_READ, bytesRead); intent.setData(uri);
intent.putExtra(Downloader.EXTRA_TOTAL_BYTES, totalBytes); intent.putExtra(Downloader.EXTRA_BYTES_READ, bytesRead);
localBroadcastManager.sendBroadcast(intent); intent.putExtra(Downloader.EXTRA_TOTAL_BYTES, totalBytes);
localBroadcastManager.sendBroadcast(intent);
NotificationManager nm = (NotificationManager) getSystemService(NOTIFICATION_SERVICE); NotificationManager nm = (NotificationManager) getSystemService(NOTIFICATION_SERVICE);
Notification notification = createNotification(uri.toString(), packageName) Notification notification = createNotification(uri.toString(), packageName)
.setProgress(totalBytes, bytesRead, false) .setProgress(totalBytes, bytesRead, false)
.build(); .build();
nm.notify(NOTIFY_DOWNLOADING, notification); nm.notify(NOTIFY_DOWNLOADING, notification);
}
} }
}); });
downloader.download(); downloader.download();
@ -288,6 +294,9 @@ public class DownloaderService extends Service {
if (downloader != null) { if (downloader != null) {
downloader.close(); downloader.close();
} }
// May have already been removed in response to a cancel intent, but that wont cause
// problems if we ask to remove it again.
QUEUE_WHATS.remove(uri.toString());
} }
downloader = null; downloader = null;
} }
@ -355,6 +364,10 @@ public class DownloaderService extends Service {
return isQueued(urlString) || isActive(urlString); return isQueued(urlString) || isActive(urlString);
} }
public static boolean isQueueEmpty() {
return QUEUE_WHATS.isEmpty();
}
/** /**
* Check if a URL is waiting in the queue for downloading. * Check if a URL is waiting in the queue for downloading.
*/ */
@ -370,7 +383,7 @@ public class DownloaderService extends Service {
* Check if a URL is actively being downloaded. * Check if a URL is actively being downloaded.
*/ */
public static boolean isActive(String urlString) { public static boolean isActive(String urlString) {
return downloader != null && TextUtils.equals(urlString, downloader.sourceUrl.toString()); return downloader != null && QUEUE_WHATS.containsKey(urlString) && TextUtils.equals(urlString, downloader.sourceUrl.toString());
} }
/** /**