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

Unified Diff: chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridge.java

Issue 2440483002: Combine action parameters sent to Android displayNotification (Closed)
Patch Set: Comments Created 4 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridge.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridge.java b/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridge.java
index d68eea08e96c3e42ae3742d1da2bff90163827c1..182ab94789f45830328023481f83f58f5b1334c6 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridge.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridge.java
@@ -458,7 +458,6 @@ public class NotificationPlatformBridge {
/**
* Displays a notification with the given details.
*
- * TODO(crbug.com/650302): Combine the 'action*' parameters into a single array of objects.
* @param notificationId The id of the notification.
* @param origin Full text of the origin, including the protocol, owning this notification.
* @param profileId Id of the profile that showed the notification.
@@ -482,22 +481,14 @@ public class NotificationPlatformBridge {
* @param renotify Whether the sound, vibration, and lights should be replayed if the
* notification is replacing another notification.
* @param silent Whether the default sound, vibration and lights should be suppressed.
- * @param actionTitles Titles of actions to display alongside the notification.
- * @param actionIcons Icons of actions to display alongside the notification.
- * @param actionTypes Types of actions to display alongside the notification.
- * @param actionPlaceholders Placeholders of actions to display alongside the notification.
+ * @param actions Action buttons to display alongside the notification.
* @see https://developer.android.com/reference/android/app/Notification.html
*/
@CalledByNative
private void displayNotification(String notificationId, String origin, String profileId,
boolean incognito, String tag, String webApkPackage, String title, String body,
Bitmap image, Bitmap icon, Bitmap badge, int[] vibrationPattern, long timestamp,
- boolean renotify, boolean silent, String[] actionTitles, Bitmap[] actionIcons,
- String[] actionTypes, String[] actionPlaceholders) {
- if (actionTitles.length != actionIcons.length) {
- throw new IllegalArgumentException("The number of action titles and icons must match.");
- }
-
+ boolean renotify, boolean silent, ActionInfo[] actions) {
Resources res = mAppContext.getResources();
// Record whether it's known whether notifications can be shown to the user at all.
@@ -541,19 +532,19 @@ public class NotificationPlatformBridge {
.setOrigin(UrlFormatter.formatUrlForSecurityDisplay(
origin, false /* showScheme */));
- for (int actionIndex = 0; actionIndex < actionTitles.length; actionIndex++) {
+ for (int actionIndex = 0; actionIndex < actions.length; actionIndex++) {
PendingIntent intent = makePendingIntent(
NotificationConstants.ACTION_CLICK_NOTIFICATION, notificationId, origin,
profileId, incognito, tag, webApkPackage, actionIndex);
+ ActionInfo action = actions[actionIndex];
// Don't show action button icons when there's an image, as then action buttons go on
// the same row as the Site Settings button, so icons wouldn't leave room for text.
- Bitmap actionIcon = hasImage ? null : actionIcons[actionIndex];
- // TODO(crbug.com/650302): Encode actionTypes with an enum, not a magic string!
- if (actionTypes[actionIndex].equals("text")) {
- notificationBuilder.addTextAction(actionIcon, actionTitles[actionIndex], intent,
- actionPlaceholders[actionIndex]);
+ Bitmap actionIcon = hasImage ? null : action.icon;
+ if (action.type == NotificationActionType.TEXT) {
+ notificationBuilder.addTextAction(
+ actionIcon, action.title, intent, action.placeholder);
} else {
- notificationBuilder.addButtonAction(actionIcon, actionTitles[actionIndex], intent);
+ notificationBuilder.addButtonAction(actionIcon, action.title, intent);
}
}
@@ -561,7 +552,7 @@ public class NotificationPlatformBridge {
// label and icon, so abbreviate it. This has the unfortunate side-effect of unnecessarily
// abbreviating it on Android Wear also (crbug.com/576656). If custom layouts are enabled,
// the label and icon provided here only affect Android Wear, so don't abbreviate them.
- boolean abbreviateSiteSettings = actionTitles.length > 0 && !useCustomLayouts(hasImage);
+ boolean abbreviateSiteSettings = actions.length > 0 && !useCustomLayouts(hasImage);
int settingsIconId = abbreviateSiteSettings ? 0 : R.drawable.settings_cog;
CharSequence settingsTitle = abbreviateSiteSettings
? res.getString(R.string.notification_site_settings_button)

Powered by Google App Engine
This is Rietveld 408576698