Linux native notifications: Add server capabilities metrics

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/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"
 #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"
@@ skipping 250 matching lines @@
     DCHECK(task_runner_->RunsTasksOnCurrentThread());
     dbus::Bus::Options bus_options;
     bus_options.bus_type = dbus::Bus::SESSION;
     bus_options.connection_type = dbus::Bus::PRIVATE;
     bus_options.dbus_task_runner = task_runner_;
     bus_ = make_scoped_refptr(new dbus::Bus(bus_options));
 
     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,
                       this, true));
     notification_proxy_->ConnectToSignal(
         kFreedesktopNotificationsName, "ActionInvoked",
         base::Bind(&NotificationPlatformBridgeLinuxImpl::OnActionInvoked, this),
         base::Bind(&NotificationPlatformBridgeLinuxImpl::OnSignalConnected,
                    this));
     notification_proxy_->ConnectToSignal(
@@ skipping 292 matching lines @@
 
   bool cleanup_posted_ = false;
 
   //////////////////////////////////////////////////////////////////////////////
   // Members used only on the task runner thread.
 
   scoped_refptr<dbus::Bus> bus_;
 
   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
   // eg. std::set::erase(T) would require a std::unique_ptr<T>
   // argument, so the data would get double-destructed.
   template <typename T>
   using UnorderedUniqueSet = std::unordered_map<T*, std::unique_ptr<T>>;
 
   UnorderedUniqueSet<NotificationData> notifications_;
 
@@ skipping 25 matching lines @@
     const std::string& profile_id,
     bool incognito,
     const GetDisplayedNotificationsCallback& callback) const {
   impl_->GetDisplayed(profile_id, incognito, callback);
 }
 
 void NotificationPlatformBridgeLinux::SetReadyCallback(
     NotificationBridgeReadyCallback callback) {
   impl_->SetReadyCallback(std::move(callback));
 }