Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(26)

Issue 2751813004: Fix the download summary notification icons (Closed)

Created:
3 years, 9 months ago by David Trainor- moved to gerrit
Modified:
3 years, 9 months ago
Reviewers:
qinmin
CC:
chromium-reviews, asanka, agrieve+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Fix the download summary notification icons Make the icon ordering follow the expected behavior: progress, pending, failed, paused, completed. This requires additional changes to support listening to when completed and failed notifications are dismissed so we can update the icon state, so I added a pending intent to those delete actions as well, but prevented it from restarting the service in case it's the last notification. For proper handling of all intent actions, I still added code to handle the intent for the case where the service does get started with that action. BUG=699687 Review-Url: https://codereview.chromium.org/2751813004 Cr-Commit-Position: refs/heads/master@{#457873} Committed: https://chromium.googlesource.com/chromium/src/+/8055a28acd2bf29534913610369be7ae1b0f894f

Patch Set 1 #

Total comments: 1

Patch Set 2 : Added code to respond to the pause dismissal as well #

Patch Set 3 : Fixed test compile error #

Messages

Total messages: 19 (14 generated)
David Trainor- moved to gerrit
ptal thanks! https://codereview.chromium.org/2751813004/diff/1/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java File chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java (right): https://codereview.chromium.org/2751813004/diff/1/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java#newcode188 chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java:188: private static void updateSummaryIcon(Context context, NotificationManager manager, ...
3 years, 9 months ago (2017-03-16 00:05:11 UTC) #2
David Trainor- moved to gerrit
ping thanks min! sorry for poking but want to get this in for 58.
3 years, 9 months ago (2017-03-17 02:58:22 UTC) #3
qinmin
lgtm
3 years, 9 months ago (2017-03-17 04:41:59 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2751813004/40001
3 years, 9 months ago (2017-03-17 20:10:52 UTC) #16
commit-bot: I haz the power
3 years, 9 months ago (2017-03-17 20:19:02 UTC) #19
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/8055a28acd2bf29534913610369b...

Powered by Google App Engine
This is Rietveld 408576698