Linux native notifications: Add support for custom notification buttons

chrome/browser/notifications/notification_platform_bridge_linux.cc

 // Copyright 2017 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/notifications/notification_platform_bridge_linux.h"
 
 #include <algorithm>
 
 #include "base/files/file_util.h"
 #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"
 #include "chrome/browser/notifications/native_notification_display_service.h"
 #include "chrome/browser/notifications/notification.h"
 #include "chrome/browser/notifications/notification_display_service_factory.h"
 #include "chrome/browser/profiles/profile_manager.h"
 
 namespace {
 
@@ skipping 146 matching lines @@
   NotificationCommon::Type notification_type;
   const std::string notification_id;
   const std::string profile_id;
   const bool is_incognito;
 
   // A copy of the origin_url from the underlying
   // message_center::Notification.  Used to pass back to
   // 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;
 
   // Used to cancel the initial "Notify" message so we don't call
   // NotificationPlatformBridgeLinux::NotifyCompleteInternal() with a
   // destroyed Notification.
   ScopedGObject<GCancellable> cancellable;
 
   // If not null, the data to update the notification with once
@@ skipping 135 matching lines @@
 
   if (data->dbus_id)
     DCHECK(!data->cancellable);
 
   data->ResetResourceFiles();
 
   GVariantBuilder actions_builder;
   // 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.
     AddActionToNotification(&actions_builder, "default", "");
   }
   // Always add a settings button.
   AddActionToNotification(&actions_builder, "settings", "Settings");
 
   GVariantBuilder hints_builder;
   g_variant_builder_init(&hints_builder, G_VARIANT_TYPE("a{sv}"));
@@ skipping 87 matching lines @@
     ForwardNotificationOperation(dbus_id, NotificationCommon::CLOSE, -1);
     // std::unordered_map::erase(nullptr) is safe here.
     notifications_.erase(FindNotificationData(dbus_id));
   } else if (strcmp("ActionInvoked", sender_signal) == 0 &&
              g_variant_is_of_type(parameters, G_VARIANT_TYPE("(us)"))) {
     const gchar* action = nullptr;
     g_variant_get(parameters, "(u&s)", &dbus_id, &action);
     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);
     }
   }
 }