Linux native notifications: Add support for custom notification buttons

chrome/browser/notifications/notification_platform_bridge_linux.cc

Index: chrome/browser/notifications/notification_platform_bridge_linux.cc
diff --git a/chrome/browser/notifications/notification_platform_bridge_linux.cc b/chrome/browser/notifications/notification_platform_bridge_linux.cc
index ed8f4bc9406133dbead14d2e0b7e4e0c1bc58aaf..a24248586520851fdd8ba74eb10ca5dbddc51a91 100644
--- a/chrome/browser/notifications/notification_platform_bridge_linux.cc
+++ b/chrome/browser/notifications/notification_platform_bridge_linux.cc
@@ -10,6 +10,7 @@
 #include "base/memory/ptr_util.h"
 #include "base/stl_util.h"
 #include "base/strings/nullable_string16.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/task_scheduler/post_task.h"
 #include "chrome/browser/browser_process.h"
@@ -176,6 +177,11 @@ struct NotificationPlatformBridgeLinux::NotificationData {
   // NativeNotificationDisplayService.
   const GURL origin_url;
 
+  // Used to keep track of the IDs of the buttons currently displayed
+  // on this notification.
+  size_t action_start = 0;
+  size_t action_end = 0;
+
   // Temporary resource files associated with the notification that
   // should be cleaned up when the notification is closed.
   std::vector<base::FilePath> resource_files;
@@ -331,6 +337,15 @@ void NotificationPlatformBridgeLinux::NotifyNow(
   // Even-indexed elements in this array are action IDs passed back to
   // us in GSignalReceiver.  Odd-indexed ones contain the button text.
   g_variant_builder_init(&actions_builder, G_VARIANT_TYPE("as"));
+  data->action_start = data->action_end;
+  for (const auto& button_info : notification.buttons()) {
+    // FDO notification buttons can contain either an icon or a label,
+    // but not both, and the type of all buttons must be the same (all
+    // labels or all icons), so always use labels.
+    std::string id = base::SizeTToString(data->action_end++);
+    const std::string label = base::UTF16ToUTF8(button_info.title);
+    AddActionToNotification(&actions_builder, id.c_str(), label.c_str());

[Peter Beverloo, 2017/04/12 00:44:22]

Are we concerned about the OS passing us invalid button information? I think we
trust input elsewhere. Otherwise it may be simpler to use use a regular loop:

for (size_t i = 0; i < notification.buttons().size(); ++i) {
  const std::string id = base::SizeTToString(i);
  const std::string label = base::UTF16ToUTF8(
      notification.buttons()[i].title);

  AddActionToNotification(&actions_builder, id.c_str(), label.c_str());
}

Then we don't need to maintain the |action_start| and |action_end| properties,
and just change the check on line 467 to check against some other sensible
limit.

(You could add NotificationPlatformBridgeLinux::kMaxButtonCount set to 2.)

[Tom (Use chromium acct), 2017/04/13 02:23:34]

No, but I wanted to handle this case:
1. Notification is created with buttons "Yes" and "No"
2. User clicks on "Yes", event is being sent
3. Chrome updates notification buttons to say "No" "Yes" before it gets the
event
4. Chrome gets the event and thinks the user clicked "No"

It would be easier if the protocol were synchronous

+  }
   if (notification.clickable()) {
     // Special case: the pair ("default", "") will not add a button,
     // but instead makes the entire notification clickable.
@@ -438,11 +453,21 @@ void NotificationPlatformBridgeLinux::GSignalReceiver(GDBusProxy* proxy,
     DCHECK(action);
 
     if (strcmp(action, "default") == 0) {
-      ForwardNotificationOperation(dbus_id, NotificationCommon::CLICK, 0);
+      ForwardNotificationOperation(dbus_id, NotificationCommon::CLICK, -1);
     } else if (strcmp(action, "settings") == 0) {
       ForwardNotificationOperation(dbus_id, NotificationCommon::SETTINGS, -1);
     } else {
-      NOTIMPLEMENTED() << "No custom buttons just yet!";
+      size_t id;
+      if (!base::StringToSizeT(action, &id))
+        return;
+      NotificationData* data = FindNotificationData(dbus_id);
+      if (!data)
+        return;
+      size_t n_buttons = data->action_end - data->action_start;
+      if (id >= n_buttons)
+        return;
+      ForwardNotificationOperation(dbus_id, NotificationCommon::CLICK,
+                                   id - data->action_start);
     }
   }
 }