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/chrome_notification_types.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"
 #include "content/public/browser/notification_service.h"
 
@@ skipping 145 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.

[Lei Zhang, 2017/04/14 01:19:45]

Mention the valid range of action IDs is [action_start, action_end)

[Tom (Use chromium acct), 2017/04/14 18:19:01]

Done.

+  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 or on
   // shutdown.
   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;
 
@@ skipping 155 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());
+  }
   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)

[Lei Zhang, 2017/04/14 01:19:45]

On line 360, when it gets the first time, it's using [0, N). On the next call,
it's usng [N, M) for the IDs.

Later when we reach here, the ID string is being converted back to an ID number.
Is |id| going to be in the range [N, M) if the notification updates and then the
user clicks?

e.g. with 3 buttons initially and 2 in the updated notification, we'll end up
with |action_start| = 3 and |action_end| = 5. Valid |id| values would be 3 or 4,
right? Whereas |n_buttons| is going to be 2. Wouldn't that result in failing
this check?

[Tom (Use chromium acct), 2017/04/14 18:19:01]

Nice catch!  This should have been "id - data->action_start"

+        return;
+      ForwardNotificationOperation(dbus_id, NotificationCommon::CLICK,
+                                   id - data->action_start);
     }
   }
 }