UMA for Notification action buttons

chrome/browser/notifications/platform_notification_service_impl.cc

Index: chrome/browser/notifications/platform_notification_service_impl.cc
diff --git a/chrome/browser/notifications/platform_notification_service_impl.cc b/chrome/browser/notifications/platform_notification_service_impl.cc
index 04734d10174cc6a3e0742d56480536bf374d35ba..b9b1bee0e3a70b85da4177899e55e2441d52ffbd 100644
--- a/chrome/browser/notifications/platform_notification_service_impl.cc
+++ b/chrome/browser/notifications/platform_notification_service_impl.cc
@@ -101,8 +101,13 @@ void PlatformNotificationServiceImpl::OnPersistentNotificationClick(
     const GURL& origin,
     int action_index) const {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  content::RecordAction(
-      base::UserMetricsAction("Notifications.Persistent.Clicked"));
+  if (action_index == -1) {

[Peter Beverloo, 2015/10/28 16:27:07]

Please don't forget about https://codereview.chromium.org/1422673002/

[johnme, 2015/10/28 18:43:48]

Acknowledged. I already rebased my local 1422673002 branch onto this one to
replace this -1 :)

+    content::RecordAction(base::UserMetricsAction(
+        "Notifications.Persistent.Clicked"));
+  } else {
+    content::RecordAction(base::UserMetricsAction(
+        "Notifications.Persistent.ClickedActionButton"));
+  }
 
   content::NotificationEventDispatcher::GetInstance()
       ->DispatchNotificationClickEvent(
@@ -295,6 +300,9 @@ void PlatformNotificationServiceImpl::DisplayPersistentNotification(
   GetNotificationUIManager()->Add(notification, profile);
   content::RecordAction(
       base::UserMetricsAction("Notifications.Persistent.Shown"));
+  UMA_HISTOGRAM_CUSTOM_COUNTS("Notifications.PersistentNotificationActionCount",
+                              notification_data.actions.size(),
+                              0, 10, 12 /* one extra bucket for > 10 */);

[Peter Beverloo, 2015/10/28 16:27:07]

I don't understand, |notification_data.actions.size()| won't ever be >2 here
since Blink limits it in the renderer process, and SanitizeNotificationData() in
the browser process.

Did you instead intend to measure this in Blink *before* we apply the cap?

[johnme, 2015/10/28 18:43:48]

No, but that's a good idea. Done.

 
   HostContentSettingsMapFactory::GetForProfile(profile)->UpdateLastUsage(
       origin, origin, CONTENT_SETTINGS_TYPE_NOTIFICATIONS);