Linux native notifications: Add server capabilities metrics

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 cf71db8869d0a8a1c7c5f89085a7fd8338243d6c..dbad4e739bef41e8ddf40dbec8de9856b2746540 100644
--- a/chrome/browser/notifications/notification_platform_bridge_linux.cc
+++ b/chrome/browser/notifications/notification_platform_bridge_linux.cc
@@ -8,6 +8,7 @@
 
 #include "base/barrier_closure.h"
 #include "base/files/file_util.h"
+#include "base/metrics/histogram_macros.h"
 #include "base/strings/nullable_string16.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
@@ -278,11 +279,49 @@ class NotificationPlatformBridgeLinuxImpl
     notification_proxy_ =
         bus_->GetObjectProxy(kFreedesktopNotificationsName,
                              dbus::ObjectPath(kFreedesktopNotificationsPath));
+    UMA_HISTOGRAM_BOOLEAN("Freedesktop.Notifications",
+                          notification_proxy_ != nullptr);

[Peter Beverloo, 2017/05/02 14:15:24]

The difference between this one and the UMA defined in the
native_notification_platform_bridge.cc file is that this includes Linux
platforms that (a) expose the Freedesktop service, but (b) cannot bind to the
signals, or, in the future, (c) do not adhere to the required capabilities.

Would it be sensible to create an enum-based UMA to distinguish between those
four statuses?

- No freedesktop
- Freedesktop, but missing capabilities
- Freedesktop, but no connected signals
- Freedesktop w/ capabilities and signals

[Tom (Use chromium acct), 2017/05/02 18:49:41]

Done.

     if (!notification_proxy_) {
       OnConnectionInitializationFinishedOnTaskRunner(false);
       return;
     }
 
+    dbus::MethodCall get_capabilities_call(kFreedesktopNotificationsName,
+                                           "GetCapabilities");
+    std::unique_ptr<dbus::Response> capabilities_response =
+        notification_proxy_->CallMethodAndBlock(
+            &get_capabilities_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT);
+    if (capabilities_response) {
+      dbus::MessageReader reader(capabilities_response.get());
+      std::vector<std::string> capabilities;
+      reader.PopArrayOfStrings(&capabilities);
+      for (const std::string& capability : capabilities)
+        capabilities_.insert(capability);
+    }
+    // Histogram macros must be called with the same name for each
+    // callsite, so we can't roll the below into a nice loop.
+    UMA_HISTOGRAM_BOOLEAN("Freedesktop.Notifications.Capabilities.ActionIcons",
+                          capabilities_.count("action-icons"));
+    UMA_HISTOGRAM_BOOLEAN("Freedesktop.Notifications.Capabilities.Actions",
+                          capabilities_.count("actions"));
+    UMA_HISTOGRAM_BOOLEAN("Freedesktop.Notifications.Capabilities.Body",
+                          capabilities_.count("body"));
+    UMA_HISTOGRAM_BOOLEAN(
+        "Freedesktop.Notifications.Capabilities.BodyHyperlinks",
+        capabilities_.count("body-hyperlinks"));
+    UMA_HISTOGRAM_BOOLEAN("Freedesktop.Notifications.Capabilities.BodyImages",
+                          capabilities_.count("body-images"));
+    UMA_HISTOGRAM_BOOLEAN("Freedesktop.Notifications.Capabilities.BodyMarkup",
+                          capabilities_.count("body-markup"));
+    UMA_HISTOGRAM_BOOLEAN("Freedesktop.Notifications.Capabilities.IconMulti",
+                          capabilities_.count("icon-multi"));
+    UMA_HISTOGRAM_BOOLEAN("Freedesktop.Notifications.Capabilities.IconStatic",
+                          capabilities_.count("icon-static"));
+    UMA_HISTOGRAM_BOOLEAN("Freedesktop.Notifications.Capabilities.Persistence",
+                          capabilities_.count("persistence"));
+    UMA_HISTOGRAM_BOOLEAN("Freedesktop.Notifications.Capabilities.Sound",
+                          capabilities_.count("sound"));

[Peter Beverloo, 2017/05/02 14:15:24]

nit: maybe move this to another method so that we can separate metrics from
functional code?

[Tom (Use chromium acct), 2017/05/02 18:49:41]

Done.

+
     connected_signals_barrier_ = base::BarrierClosure(
         2, base::Bind(&NotificationPlatformBridgeLinuxImpl::
                           OnConnectionInitializationFinishedOnTaskRunner,
@@ -595,6 +634,8 @@ class NotificationPlatformBridgeLinuxImpl
 
   dbus::ObjectProxy* notification_proxy_ = nullptr;
 
+  std::unordered_set<std::string> capabilities_;
+
   base::Closure connected_signals_barrier_;
 
   // A std::set<std::unique_ptr<T>> doesn't work well because