Index: chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java |
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java |
index 29ce08ba0c8c2bb34ef07da7873e916638cbc63e..c63f1e67b12d30efdd930d796190eb34e9817cee 100644 |
--- a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java |
+++ b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java |
@@ -216,7 +216,7 @@ public class DownloadNotificationService extends Service { |
// If we've lost all Activities, cancel the off the record downloads. |
if (ApplicationStatus.isEveryActivityDestroyed()) { |
cancelOffTheRecordDownloads(); |
- hideSummaryNotificationIfNecessary(); |
+ hideSummaryNotificationIfNecessary(null); |
} |
} |
@@ -246,7 +246,7 @@ public class DownloadNotificationService extends Service { |
updateNotificationsForShutdown(); |
handleDownloadOperation( |
new Intent(DownloadNotificationService.ACTION_DOWNLOAD_RESUME_ALL)); |
- hideSummaryNotificationIfNecessary(); |
+ hideSummaryNotificationIfNecessary(null); |
} else if (isDownloadOperationIntent(intent)) { |
handleDownloadOperation(intent); |
DownloadResumptionScheduler.getDownloadResumptionScheduler(mContext).cancelTask(); |
@@ -290,6 +290,7 @@ public class DownloadNotificationService extends Service { |
*/ |
@VisibleForTesting |
void shutdownService() { |
+ // TODO(dtrainor): Post a task to make SURE the summary notification is gone... |
David Trainor- moved to gerrit
2017/03/03 20:11:22
I'll look into fixing this before I land the patch
David Trainor- moved to gerrit
2017/03/04 06:46:01
I believe I fixed this without the posted task.
|
stopForeground(); |
for (Observer observer : mObservers) observer.onServiceShutdownRequested(); |
stopSelf(); |
@@ -389,31 +390,55 @@ public class DownloadNotificationService extends Service { |
} |
/** |
+ * @see #hideSummaryNotificationIfNecessary(Integer) |
+ * @param notificationIdToIgnore If not {@code null}, the id of a notification to ignore and |
+ * assume is closing or about to be closed. |
+ * @return Whether or not there are valid download notifications currently visible. |
+ */ |
+ @VisibleForTesting |
+ @TargetApi(Build.VERSION_CODES.M) |
+ boolean hasDownloadNotifications(Integer notificationIdToIgnore) { |
+ if (!BuildInfo.isAtLeastO()) return false; |
qinmin
2017/03/03 20:48:40
nit: android version doesn't matter with this func
David Trainor- moved to gerrit
2017/03/04 06:46:01
I figured it's a good safety catch in case someone
|
+ |
+ StatusBarNotification[] notifications = mNotificationManager.getActiveNotifications(); |
+ for (StatusBarNotification notification : notifications) { |
+ boolean isDownloadsGroup = TextUtils.equals(notification.getNotification().getGroup(), |
+ NotificationConstants.GROUP_DOWNLOADS); |
+ boolean isSummaryNotification = |
+ notification.getId() == NotificationConstants.NOTIFICATION_ID_DOWNLOAD_SUMMARY; |
+ boolean isIgnoredNotification = notificationIdToIgnore != null |
+ && notificationIdToIgnore == notification.getId(); |
+ if (isDownloadsGroup && !isSummaryNotification && !isIgnoredNotification) return true; |
+ } |
+ |
+ return false; |
+ } |
+ |
+ @VisibleForTesting |
+ void cancelSummaryNotification() { |
+ mNotificationManager.cancel(NotificationConstants.NOTIFICATION_ID_DOWNLOAD_SUMMARY); |
+ } |
+ |
+ /** |
* Check all current notifications and hide the summary notification if we have no downloads |
* notifications left. On Android if the user swipes away the last download notification the |
* summary will be dismissed. But if the last downloads notification is dismissed via |
* {@link NotificationManager#cancel(int)}, the summary will remain, so we need to check and |
* manually remove it ourselves. |
+ * @param notificationIdToIgnore Cancelling a notification and querying for the current list of |
+ * active notifications isn't synchronous. Pass a notification id |
+ * here if there is a notification that should be assumed gone. |
+ * Or pass {@code null} if no notification fits that criteria. |
*/ |
- @TargetApi(Build.VERSION_CODES.M) |
- void hideSummaryNotificationIfNecessary() { |
+ void hideSummaryNotificationIfNecessary(Integer notificationIdToIgnore) { |
if (!BuildInfo.isAtLeastO()) return; |
+ if (mDownloadsInProgress.size() > 0) return; |
stopForegroundIfNecessary(); |
- |
- StatusBarNotification[] notifications = mNotificationManager.getActiveNotifications(); |
- for (StatusBarNotification notification : notifications) { |
- if (TextUtils.equals(notification.getNotification().getGroup(), |
- NotificationConstants.GROUP_DOWNLOADS) |
- && notification.getId() |
- != NotificationConstants.NOTIFICATION_ID_DOWNLOAD_SUMMARY) { |
- return; |
- } |
- } |
- mNotificationManager.cancel(NotificationConstants.NOTIFICATION_ID_DOWNLOAD_SUMMARY); |
+ if (hasDownloadNotifications(notificationIdToIgnore)) return; |
+ cancelSummaryNotification(); |
// Added to shut down the service when everything is gone. |
- // TODO(dtrainor): Make sure this makes sense. |
shutdownService(); |
} |
@@ -572,7 +597,7 @@ public class DownloadNotificationService extends Service { |
mNotificationManager.cancel(NOTIFICATION_NAMESPACE, notificationId); |
mDownloadSharedPreferenceHelper.removeSharedPreferenceEntry(downloadGuid); |
stopTrackingInProgressDownload(downloadGuid); |
- hideSummaryNotificationIfNecessary(); |
+ hideSummaryNotificationIfNecessary(notificationId); |
} |
/** |
@@ -825,7 +850,7 @@ public class DownloadNotificationService extends Service { |
if (entry == null |
&& !(isOfflinePage && TextUtils.equals(intent.getAction(), ACTION_DOWNLOAD_OPEN))) { |
handleDownloadOperationForMissingNotification(intent); |
- hideSummaryNotificationIfNecessary(); |
+ hideSummaryNotificationIfNecessary(null); |
return; |
} |
@@ -834,7 +859,7 @@ public class DownloadNotificationService extends Service { |
// nothing in that case. |
if (!DownloadManagerService.hasDownloadManagerService()) { |
notifyDownloadPaused(entry.downloadGuid, !entry.isOffTheRecord, false); |
- hideSummaryNotificationIfNecessary(); |
+ hideSummaryNotificationIfNecessary(null); |
return; |
} |
} else if (ACTION_DOWNLOAD_RESUME.equals(intent.getAction())) { |
@@ -850,7 +875,7 @@ public class DownloadNotificationService extends Service { |
} else if (ACTION_DOWNLOAD_RESUME_ALL.equals(intent.getAction()) |
&& (mDownloadSharedPreferenceHelper.getEntries().isEmpty() |
|| DownloadManagerService.hasDownloadManagerService())) { |
- hideSummaryNotificationIfNecessary(); |
+ hideSummaryNotificationIfNecessary(null); |
return; |
} else if (ACTION_DOWNLOAD_OPEN.equals(intent.getAction())) { |
// TODO(fgorski): Do we even need to do anything special here, before we launch Chrome? |
@@ -902,7 +927,10 @@ public class DownloadNotificationService extends Service { |
if (!ACTION_DOWNLOAD_OPEN.equals(intent.getAction())) { |
downloadServiceDelegate.destroyServiceDelegate(); |
} |
- hideSummaryNotificationIfNecessary(); |
+ |
+ hideSummaryNotificationIfNecessary(ACTION_DOWNLOAD_CANCEL.equals(intent.getAction()) |
+ ? entry.notificationId |
+ : null); |
} |
}; |
try { |