Plumb through notification action types and placeholders on Android

chrome/browser/notifications/notification_platform_bridge_android.cc

Index: chrome/browser/notifications/notification_platform_bridge_android.cc
diff --git a/chrome/browser/notifications/notification_platform_bridge_android.cc b/chrome/browser/notifications/notification_platform_bridge_android.cc
index 9c3075a749de185d950fd5247926f2325a76eb44..69ea8bf59b2688bcef44557abdfa44f01ab0e883 100644
--- a/chrome/browser/notifications/notification_platform_bridge_android.cc
+++ b/chrome/browser/notifications/notification_platform_bridge_android.cc
@@ -12,6 +12,8 @@
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/logging.h"
+#include "base/strings/string_number_conversions.h"

[Peter Beverloo, 2016/09/22 18:07:56]

nit: merge conflict? I removed this in a previous CL, we don't need it anymore
:).

[awdf, 2016/09/23 15:24:56]

Done.

+#include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/notifications/native_notification_display_service.h"
 #include "chrome/browser/notifications/notification.h"
@@ -227,6 +229,28 @@ void NotificationPlatformBridgeAndroid::Display(
   ScopedJavaLocalRef<jobjectArray> action_icons =
       ConvertToJavaBitmaps(notification.buttons());
 
+  std::vector<base::string16> action_types_vector;
+  for (const message_center::ButtonInfo& button : notification.buttons()) {
+    if (button.type == message_center::ButtonType::TEXT) {
+      action_types_vector.push_back(base::ASCIIToUTF16("text"));
+    } else {
+      action_types_vector.push_back(base::ASCIIToUTF16("button"));
+    }
+  }
+  ScopedJavaLocalRef<jobjectArray> action_types =
+      base::android::ToJavaArrayOfStrings(env, action_types_vector);
+
+  std::vector<base::string16> action_placeholders_vector;
+  for (const message_center::ButtonInfo& button : notification.buttons()) {
+    if (!button.placeholder.is_null()) {

[Peter Beverloo, 2016/09/22 18:07:56]

nit: why not unify the for loops into a single one?

[awdf, 2016/09/23 15:24:56]

Done.

+      action_placeholders_vector.push_back(button.placeholder.string());
+    } else {
+      action_placeholders_vector.push_back(base::ASCIIToUTF16(""));

[awdf, 2016/09/23 15:24:56]

here I should shouldn't be replacing NULL with an empty string, as discussed, we
should instead use a default placeholder if the developer didn't specify one.
But where does it make most sense to replace NULL with a default placeholder? I
guess we want the same string on all platforms, so we should replace it before
this point, maybe at 
chrome/browser/notifications/platform_notification_service_impl.cc ? (And then
it can be a regular string instead of a nullable one from there).

[Peter Beverloo, 2016/09/23 16:01:22]

I agree - adding it in PlatformNotificationServiceImpl sounds like the best
place to me, and being sure that it's not NULL beyond that point is a good
advantage.

[awdf, 2016/09/26 15:33:28]

Done.

+    }
+  }
+  ScopedJavaLocalRef<jobjectArray> action_placeholders =
+      base::android::ToJavaArrayOfStrings(env, action_placeholders_vector);
+
   ScopedJavaLocalRef<jintArray> vibration_pattern =
       base::android::ToJavaIntArray(env, notification.vibration_pattern());
 
@@ -238,7 +262,7 @@ void NotificationPlatformBridgeAndroid::Display(
       tag, webapk_package, title, body, image, notification_icon, badge,
       vibration_pattern, notification.timestamp().ToJavaTime(),
       notification.renotify(), notification.silent(), action_titles,
-      action_icons);
+      action_icons, action_types, action_placeholders);
 
   regenerated_notification_infos_[notification_id] =
       RegeneratedNotificationInfo(origin_url.spec(), notification.tag(),