Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2781)

Unified Diff: chrome/browser/notifications/notification_platform_bridge_linux.cc

Issue 2856753002: Linux native notifications: Add server capabilities metrics (Closed)
Patch Set: address peter@'s comments Created 3 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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..7e7c8f56aa7caa4206bace0fadd61bd3a12383cf 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"
@@ -35,6 +36,18 @@ const char kFreedesktopNotificationsPath[] = "/org/freedesktop/Notifications";
const char kDefaultButtonId[] = "default";
const char kSettingsButtonId[] = "settings";
+enum class ConnectionInitializationStatusCode {
Mark P 2017/05/03 19:20:40 Please follow the practices here: https://chromium
Tom (Use chromium acct) 2017/05/03 22:20:29 Done.
+ SUCCESS = 0,
+ NATIVE_NOTIFICATIONS_NOT_SUPPORTED,
+ MISSING_REQUIRED_CAPABILITIES,
Mark P 2017/05/03 19:20:40 This value doesn't seem to be used anywhere.
Tom (Use chromium acct) 2017/05/03 22:20:29 It will be used soon in the future
+ COULD_NOT_CONNECT_TO_SIGNALS,
+ // The values in this enumeration correspond to those of the
+ // Linux.NotificationPlatformBridge.InitializationStatus histogram,
+ // so the ordering should not be changed. New error codes should be
+ // added at the end, before NUM_ITEMS.
+ NUM_ITEMS
+};
+
gfx::Image DeepCopyImage(const gfx::Image& image) {
std::unique_ptr<gfx::ImageSkia> image_skia(image.CopyImageSkia());
return gfx::Image(*image_skia);
@@ -279,14 +292,30 @@ class NotificationPlatformBridgeLinuxImpl
bus_->GetObjectProxy(kFreedesktopNotificationsName,
dbus::ObjectPath(kFreedesktopNotificationsPath));
if (!notification_proxy_) {
- OnConnectionInitializationFinishedOnTaskRunner(false);
+ OnConnectionInitializationFinishedOnTaskRunner(
+ ConnectionInitializationStatusCode::
+ NATIVE_NOTIFICATIONS_NOT_SUPPORTED);
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);
+ }
+ RecordMetricsForCapabilities();
+
connected_signals_barrier_ = base::BarrierClosure(
2, base::Bind(&NotificationPlatformBridgeLinuxImpl::
OnConnectionInitializationFinishedOnTaskRunner,
- this, true));
+ this, ConnectionInitializationStatusCode::SUCCESS));
notification_proxy_->ConnectToSignal(
kFreedesktopNotificationsName, "ActionInvoked",
base::Bind(&NotificationPlatformBridgeLinuxImpl::OnActionInvoked, this),
@@ -555,12 +584,17 @@ class NotificationPlatformBridgeLinuxImpl
CleanUp();
}
- void OnConnectionInitializationFinishedOnTaskRunner(bool success) {
+ void OnConnectionInitializationFinishedOnTaskRunner(
+ ConnectionInitializationStatusCode status) {
DCHECK(task_runner_->RunsTasksOnCurrentThread());
- PostTaskToUiThread(
- base::BindOnce(&NotificationPlatformBridgeLinuxImpl::
- OnConnectionInitializationFinishedOnUiThread,
- this, success));
+ UMA_HISTOGRAM_ENUMERATION(
+ "Linux.NotificationPlatformBridge.InitializationStatus",
+ static_cast<int>(status),
+ static_cast<int>(ConnectionInitializationStatusCode::NUM_ITEMS));
+ PostTaskToUiThread(base::BindOnce(
+ &NotificationPlatformBridgeLinuxImpl::
+ OnConnectionInitializationFinishedOnUiThread,
+ this, status == ConnectionInitializationStatusCode::SUCCESS));
}
void OnSignalConnected(const std::string& interface_name,
@@ -568,12 +602,39 @@ class NotificationPlatformBridgeLinuxImpl
bool success) {
DCHECK(task_runner_->RunsTasksOnCurrentThread());
if (!success) {
- OnConnectionInitializationFinishedOnTaskRunner(false);
+ OnConnectionInitializationFinishedOnTaskRunner(
+ ConnectionInitializationStatusCode::COULD_NOT_CONNECT_TO_SIGNALS);
return;
}
connected_signals_barrier_.Run();
}
+ void RecordMetricsForCapabilities() {
+ // Histogram macros must be called with the same name for each
+ // callsite, so we can't roll the below into a nice loop.
Mark P 2017/05/03 19:20:40 Thank you for the comment.
+ 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"));
+ }
+
//////////////////////////////////////////////////////////////////////////////
// Members used only on the UI thread.
@@ -595,6 +656,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

Powered by Google App Engine
This is Rietveld 408576698