UMA for Notification action buttons

chrome/browser/notifications/platform_notification_service_impl.cc

 // Copyright 2014 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/platform_notification_service_impl.h"
 
 #include "base/command_line.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/user_metrics_action.h"
 #include "base/prefs/pref_service.h"
@@ skipping 83 matching lines @@
     : notification_ui_manager_for_tests_(nullptr) {}
 
 PlatformNotificationServiceImpl::~PlatformNotificationServiceImpl() {}
 
 void PlatformNotificationServiceImpl::OnPersistentNotificationClick(
     BrowserContext* browser_context,
     int64_t persistent_notification_id,
     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(
             browser_context,
             persistent_notification_id,
             origin,
             action_index,
             base::Bind(&OnEventDispatchComplete));
 }
 
@@ skipping 172 matching lines @@
   Notification notification = CreateNotificationFromData(
       profile, origin, icon, notification_data, delegate);
 
   // TODO(peter): Remove this mapping when we have reliable id generation for
   // the message_center::Notification objects.
   persistent_notifications_[persistent_notification_id] = notification.id();
 
   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);
 }
 
 void PlatformNotificationServiceImpl::ClosePersistentNotification(
     BrowserContext* browser_context,
     int64_t persistent_notification_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
@@ skipping 126 matching lines @@
         extensions::ExtensionRegistry::Get(profile)->GetExtensionById(
             origin.host(), extensions::ExtensionRegistry::EVERYTHING);
     DCHECK(extension);
 
     return base::UTF8ToUTF16(extension->name());
   }
 #endif
 
   return base::string16();
 }