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 1118aac1d3a75f17569cf4a22523517802c3dd1e..d2d00103bd68d927feb26003bd1ff1a6ede9dc18 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 |
@@ -23,9 +23,11 @@ import android.graphics.Rect; |
import android.graphics.drawable.shapes.OvalShape; |
import android.os.Binder; |
import android.os.Build; |
+import android.os.Bundle; |
import android.os.IBinder; |
import android.service.notification.StatusBarNotification; |
import android.text.TextUtils; |
+import android.util.Pair; |
import org.chromium.base.ApiCompatibilityUtils; |
import org.chromium.base.ApplicationStatus; |
@@ -59,7 +61,7 @@ import java.util.concurrent.TimeUnit; |
* |
* On O and above, this service will receive {@link Service#startForeground(int, Notification)} |
* calls when containing active downloads. The foreground notification will be the summary |
- * notification generated by {@link DownloadNotificationService#getSummaryNotification(Context)}. |
+ * notification generated by {@link DownloadNotificationService#buildSummaryNotification(Context)}. |
* The service will receive a {@link Service#stopForeground(boolean)} call when all active downloads |
* are paused. The summary notification will be hidden when there are no other notifications in the |
* {@link NotificationConstants#GROUP_DOWNLOADS} group. This gets checked after every notification |
@@ -85,6 +87,8 @@ public class DownloadNotificationService extends Service { |
"org.chromium.chrome.browser.download.DOWNLOAD_RESUME_ALL"; |
public static final String ACTION_DOWNLOAD_OPEN = |
"org.chromium.chrome.browser.download.DOWNLOAD_OPEN"; |
+ public static final String ACTION_UPDATE_SUMMARY_ICON = |
+ "org.chromium.chrome.browser.download.UPDATE_SUMMARY_ICON"; |
static final String NOTIFICATION_NAMESPACE = "DownloadNotificationService"; |
private static final String TAG = "DownloadNotification"; |
@@ -93,6 +97,8 @@ public class DownloadNotificationService extends Service { |
/** Notification Id starting value, to avoid conflicts from IDs used in prior versions. */ |
+ private static final String EXTRA_NOTIFICATION_BUNDLE_ICON_ID = |
+ "Chrome.NotificationBundleIconIdExtra"; |
private static final int STARTING_NOTIFICATION_ID = 1000000; |
private static final int MAX_RESUMPTION_ATTEMPT_LEFT = 5; |
@VisibleForTesting static final long SECONDS_PER_MINUTE = TimeUnit.MINUTES.toSeconds(1); |
@@ -145,7 +151,7 @@ public class DownloadNotificationService extends Service { |
intent.setComponent(new ComponentName(context, DownloadNotificationService.class)); |
if (BuildInfo.isAtLeastO()) { |
- Notification notification = getSummaryNotification(context); |
+ Notification notification = buildSummaryNotification(context); |
AppHooks.get().startServiceWithNotification( |
intent, NotificationConstants.NOTIFICATION_ID_DOWNLOAD_SUMMARY, notification); |
@@ -155,16 +161,101 @@ public class DownloadNotificationService extends Service { |
} |
/** |
- * Builds a summary notification that represents downloads. This is the notification passed to |
- * {@link #startForeground(int, Notification)}, which keeps this service in the foreground. |
- * @param context The context used to build the notification and pull specific resources. |
- * @return The {@link Notification} to show for the summary. Meant to be used by |
- * {@link NotificationManager#notify(int, Notification)}. |
+ * Calculates the suggested icon for the summary notification based on the other notifications |
+ * currently showing. |
+ * @param context A context to use to query Android-specific information (NotificationManager). |
+ * @param removedNotificationId The id of a notification that is currently closing and should be |
+ * ignored. -1 if no notifications are being closed. |
+ * @param addedNotification A {@link Pair} of <id, Notification> of a notification that is |
+ * currently being added and should be used in addition to or in |
+ * place of the existing icons. |
+ * @return A {@link Pair} that represents both whether or not the new icon |
+ * is different from the old one and the icon id itself. |
+ */ |
+ @TargetApi(Build.VERSION_CODES.M) |
+ private static Pair<Boolean, Integer> getSummaryIcon(Context context, int removedNotificationId, |
+ Pair<Integer, Notification> addedNotification) { |
+ if (!BuildInfo.isAtLeastO()) return new Pair<Boolean, Integer>(false, -1); |
+ boolean progress = false; |
+ boolean paused = false; |
+ boolean pending = false; |
+ boolean completed = false; |
+ boolean failed = false; |
+ |
+ final int progressIcon = android.R.drawable.stat_sys_download; |
+ final int pausedIcon = R.drawable.ic_download_pause; |
+ final int pendingIcon = R.drawable.ic_download_pending; |
+ final int completedIcon = R.drawable.offline_pin; |
+ final int failedIcon = android.R.drawable.stat_sys_download_done; |
+ |
+ NotificationManager manager = |
+ (NotificationManager) context.getSystemService(Context.NOTIFICATION_SERVICE); |
+ StatusBarNotification[] notifications = manager.getActiveNotifications(); |
+ |
+ int oldIcon = -1; |
+ for (StatusBarNotification notification : notifications) { |
+ boolean isDownloadsGroup = TextUtils.equals(notification.getNotification().getGroup(), |
+ NotificationConstants.GROUP_DOWNLOADS); |
+ if (!isDownloadsGroup) continue; |
+ if (notification.getId() == removedNotificationId) continue; |
+ |
+ boolean isSummaryNotification = |
+ notification.getId() == NotificationConstants.NOTIFICATION_ID_DOWNLOAD_SUMMARY; |
+ |
+ if (addedNotification != null && addedNotification.first == notification.getId()) |
qinmin
2017/03/05 06:48:26
if addedNotification is not null, this for loop sk
David Trainor- moved to gerrit
2017/03/06 16:29:54
The added notification might not be shown yet, so
qinmin
2017/03/06 21:43:43
i see, in that case, can we just initialize those
|
+ continue; |
+ |
+ int icon = |
+ notification.getNotification().extras.getInt(EXTRA_NOTIFICATION_BUNDLE_ICON_ID); |
+ if (isSummaryNotification) { |
+ oldIcon = icon; |
+ continue; |
+ } |
+ |
+ progress |= icon == progressIcon; |
+ paused |= icon == pausedIcon; |
+ pending |= icon == pendingIcon; |
+ completed |= icon == completedIcon; |
+ failed |= icon == failedIcon; |
+ } |
+ |
+ if (addedNotification != null) { |
+ int icon = addedNotification.second.extras.getInt(EXTRA_NOTIFICATION_BUNDLE_ICON_ID); |
+ |
+ progress |= icon == progressIcon; |
+ paused |= icon == pausedIcon; |
+ pending |= icon == pendingIcon; |
+ completed |= icon == completedIcon; |
+ failed |= icon == failedIcon; |
+ } |
+ |
+ int newIcon = android.R.drawable.stat_sys_download_done; |
+ if (progress) { |
+ newIcon = android.R.drawable.stat_sys_download; |
+ } else if (paused) { |
+ newIcon = R.drawable.ic_download_pause; |
+ } else if (pending) { |
+ newIcon = R.drawable.ic_download_pending; |
+ } else if (completed) { |
+ newIcon = R.drawable.offline_pin; |
+ } else if (failed) { |
+ newIcon = android.R.drawable.stat_sys_download_done; |
+ } |
+ return new Pair<Boolean, Integer>(newIcon != oldIcon, newIcon); |
+ } |
+ |
+ /** |
+ * Builds a summary notification that represents all downloads. |
+ * {@see #buildSummaryNotification(Context)}. |
+ * @param context A context used to query Android strings and resources. |
+ * @param iconId The id of an icon to use for the notification. |
+ * @return a {@link Notification} that represents the summary icon for all downloads. |
*/ |
- private static Notification getSummaryNotification(Context context) { |
+ private static Notification buildSummaryNotificationWithIcon(Context context, int iconId) { |
String title = |
context.getResources().getString(R.string.download_notification_summary_title, |
DownloadSharedPreferenceHelper.getInstance().getEntries().size()); |
+ |
ChromeNotificationBuilder builder = |
AppHooks.get() |
.createChromeNotificationBuilder(true /* preferCompat */, |
@@ -173,10 +264,13 @@ public class DownloadNotificationService extends Service { |
NotificationConstants.CATEGORY_GROUP_ID_GENERAL, |
context.getString(R.string.notification_category_group_general)) |
.setContentTitle(title) |
- .setSmallIcon(android.R.drawable.stat_sys_download_done) |
+ .setSmallIcon(iconId) |
.setLocalOnly(true) |
.setGroup(NotificationConstants.GROUP_DOWNLOADS) |
.setGroupSummary(true); |
+ Bundle extras = new Bundle(); |
+ extras.putInt(EXTRA_NOTIFICATION_BUNDLE_ICON_ID, iconId); |
+ builder.addExtras(extras); |
// This notification should not actually be shown. But if it is, set the click intent to |
// open downloads home. |
@@ -190,6 +284,18 @@ public class DownloadNotificationService extends Service { |
} |
/** |
+ * Builds a summary notification that represents downloads. This is the notification passed to |
+ * {@link #startForeground(int, Notification)}, which keeps this service in the foreground. |
+ * @param context The context used to build the notification and pull specific resources. |
+ * @return The {@link Notification} to show for the summary. Meant to be used by |
+ * {@link NotificationManager#notify(int, Notification)}. |
+ */ |
+ private static Notification buildSummaryNotification(Context context) { |
+ Pair<Boolean, Integer> icon = getSummaryIcon(context, -1, null); |
+ return buildSummaryNotificationWithIcon(context, icon.second); |
+ } |
+ |
+ /** |
* @return Whether or not there are any current resumable downloads being tracked. These |
* tracked downloads may not currently be showing notifications. |
*/ |
@@ -268,6 +374,9 @@ public class DownloadNotificationService extends Service { |
mNumAutoResumptionAttemptLeft = MAX_RESUMPTION_ATTEMPT_LEFT; |
clearResumptionAttemptLeft(); |
} |
+ } else if (ACTION_UPDATE_SUMMARY_ICON.equals(intent.getAction())) { |
+ updateSummaryIcon(-1, null); |
+ hideSummaryNotificationIfNecessary(null); |
} |
// This should restart the service after Chrome gets killed. However, this |
// doesn't work on Android 4.4.2. |
@@ -305,12 +414,12 @@ public class DownloadNotificationService extends Service { |
/** |
* On >= O Android releases, puts this service into a foreground state, binding it to the |
- * {@link Notification} generated by {@link #getSummaryNotification(Context)}. |
+ * {@link Notification} generated by {@link #buildSummaryNotification(Context)}. |
*/ |
@VisibleForTesting |
void startForegroundInternal() { |
if (!BuildInfo.isAtLeastO()) return; |
- Notification notification = getSummaryNotification(getApplicationContext()); |
+ Notification notification = buildSummaryNotification(getApplicationContext()); |
startForeground(NotificationConstants.NOTIFICATION_ID_DOWNLOAD_SUMMARY, notification); |
} |
@@ -441,6 +550,32 @@ public class DownloadNotificationService extends Service { |
} |
/** |
+ * Updates the notification summary with a new icon, if necessary. |
+ * @param removedNotificationId The id of a notification that is currently closing and should be |
+ * ignored. -1 if no notifications are being closed. |
+ * @param addedNotification A {@link Pair} of <id, Notification> of a notification that is |
+ * currently being added and should be used in addition to or in |
+ * place of the existing icons. |
+ */ |
+ @VisibleForTesting |
+ void updateSummaryIcon( |
+ int removedNotificationId, Pair<Integer, Notification> addedNotification) { |
+ if (!BuildInfo.isAtLeastO()) return; |
+ |
+ Pair<Boolean, Integer> icon = |
+ getSummaryIcon(mContext, removedNotificationId, addedNotification); |
+ |
+ // Avoid rebuilding the summary notification if the icon hasn't changed or we have (or are |
+ // about to have) no active downloads. |
+ if (!icon.first || !hasDownloadNotifications(removedNotificationId)) { |
+ return; |
+ } |
+ |
+ mNotificationManager.notify(NotificationConstants.NOTIFICATION_ID_DOWNLOAD_SUMMARY, |
+ buildSummaryNotificationWithIcon(mContext, icon.second)); |
+ } |
+ |
+ /** |
* Cancels the existing summary notification. Moved to a helper method for test mocking. |
*/ |
@VisibleForTesting |
@@ -460,11 +595,11 @@ public class DownloadNotificationService extends Service { |
* Or pass {@code null} if no notification fits that criteria. |
*/ |
@TargetApi(Build.VERSION_CODES.M) |
- void hideSummaryNotificationIfNecessary(Integer notificationIdToIgnore) { |
- if (!BuildInfo.isAtLeastO()) return; |
- if (mDownloadsInProgress.size() > 0) return; |
+ boolean hideSummaryNotificationIfNecessary(Integer notificationIdToIgnore) { |
+ if (!BuildInfo.isAtLeastO()) return false; |
+ if (mDownloadsInProgress.size() > 0) return false; |
- if (hasDownloadNotifications(notificationIdToIgnore)) return; |
+ if (hasDownloadNotifications(notificationIdToIgnore)) return false; |
StatusBarNotification notification = getSummaryNotification(); |
if (notification != null) { |
@@ -495,6 +630,7 @@ public class DownloadNotificationService extends Service { |
// Stop the service which should start the destruction process. At this point we should be |
// (1) a background service and (2) unbound from any clients. |
stopSelf(); |
+ return true; |
} |
@Override |
@@ -654,7 +790,9 @@ public class DownloadNotificationService extends Service { |
// down, don't stop foreground if we have no download notifications left to show. Hiding |
// the summary will take care of that for us. |
stopTrackingInProgressDownload(downloadGuid, hasDownloadNotifications(notificationId)); |
- hideSummaryNotificationIfNecessary(notificationId); |
+ if (!hideSummaryNotificationIfNecessary(notificationId)) { |
+ updateSummaryIcon(notificationId, null); |
+ } |
} |
/** |
@@ -769,6 +907,7 @@ public class DownloadNotificationService extends Service { |
mDownloadSuccessLargeIcon = getLargeNotificationIcon(bitmap); |
} |
builder.setLargeIcon(mDownloadSuccessLargeIcon); |
+ builder.setDeleteIntent(buildIconUpdateIntent(notificationId)); |
updateNotification(notificationId, builder.build(), downloadGuid, isOfflinePage, null); |
stopTrackingInProgressDownload(downloadGuid, true); |
return notificationId; |
@@ -795,6 +934,7 @@ public class DownloadNotificationService extends Service { |
ChromeNotificationBuilder builder = |
buildNotification(android.R.drawable.stat_sys_download_done, fileName, |
mContext.getResources().getString(R.string.download_notification_failed)); |
+ builder.setDeleteIntent(buildIconUpdateIntent(notificationId)); |
updateNotification(notificationId, builder.build(), downloadGuid, isOfflinePage, null); |
stopTrackingInProgressDownload(downloadGuid, true); |
} |
@@ -810,6 +950,21 @@ public class DownloadNotificationService extends Service { |
} |
/** |
+ * Helper method to build a {@link PendingIntent} that will wrap an {@link Intent} that will |
+ * update the summary notification icon. |
+ * @param notificationId ID of the notification. |
+ */ |
+ // ---- BEFORE LANDING NOTE ---- |
+ // TODO(dtrainor): This is not firing for the complete notification. Test or remove this if we |
+ // don't need it. |
+ // ---- BEFORE LANDING NOTE ---- |
+ private PendingIntent buildIconUpdateIntent(int notificationId) { |
+ Intent intent = new Intent(mContext, DownloadBroadcastReceiver.class); |
+ intent.setAction(ACTION_UPDATE_SUMMARY_ICON); |
+ return buildPendingIntent(intent, notificationId); |
+ } |
+ |
+ /** |
* Helper method to build an download action Intent from the provided information. |
* @param context {@link Context} to pull resources from. |
* @param action Download action to perform. |
@@ -838,6 +993,9 @@ public class DownloadNotificationService extends Service { |
*/ |
private ChromeNotificationBuilder buildNotification( |
int iconId, String title, String contentText) { |
+ Bundle extras = new Bundle(); |
+ extras.putInt(EXTRA_NOTIFICATION_BUNDLE_ICON_ID, iconId); |
+ |
ChromeNotificationBuilder builder = |
AppHooks.get() |
.createChromeNotificationBuilder(true /* preferCompat */, |
@@ -851,7 +1009,8 @@ public class DownloadNotificationService extends Service { |
.setLocalOnly(true) |
.setAutoCancel(true) |
.setContentText(contentText) |
- .setGroup(NotificationConstants.GROUP_DOWNLOADS); |
+ .setGroup(NotificationConstants.GROUP_DOWNLOADS) |
+ .addExtras(extras); |
return builder; |
} |
@@ -1061,6 +1220,7 @@ public class DownloadNotificationService extends Service { |
} else { |
mDownloadSharedPreferenceHelper.removeSharedPreferenceEntry(downloadGuid); |
} |
+ updateSummaryIcon(-1, new Pair<Integer, Notification>(id, notification)); |
} |
private void trackNotificationUma(boolean isOfflinePage, String downloadGuid) { |