Add UMA histogram data representing the Synced Notifications feature.

chrome/browser/notifications/sync_notifier/synced_notification_stats.cc

Index: chrome/browser/notifications/sync_notifier/synced_notification_stats.cc
diff --git a/chrome/browser/notifications/sync_notifier/synced_notification_stats.cc b/chrome/browser/notifications/sync_notifier/synced_notification_stats.cc
new file mode 100644
index 0000000000000000000000000000000000000000..d7bb0723bb28dd8e81269e5bdf843c7ceb96403f
--- /dev/null
+++ b/chrome/browser/notifications/sync_notifier/synced_notification_stats.cc
@@ -0,0 +1,47 @@
+// Copyright (c) 2013 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/sync_notifier/synced_notification_stats.h"
+
+#include <string>
+
+#include "base/metrics/histogram.h"
+#include "content/public/browser/user_metrics.h"
+
+namespace notifier {
+
+SyncedNotificationStats::SyncedNotificationStats() {}
+
+SyncedNotificationStats::SyncedNotificationStats(
+    const std::string& id) : id_(id) {
+  for (size_t i = 0; i < SYNCED_NOTIFICATION_ACTION_COUNT; i++) {

[Alexei Svitkine (slow), 2013/10/11 14:50:12]

Nit: ++i and remove {}'s

[Pete Williamson, 2013/10/14 03:46:07]

Done.

+    actions_[i] = false;
+  }
+}
+
+SyncedNotificationStats::~SyncedNotificationStats() {}
+
+void SyncedNotificationStats::CollectAction(
+    SyncedNotificationActionType type) {
+  DCHECK(!id_.empty());
+
+  UMA_HISTOGRAM_ENUMERATION("Notifications.Actions",
+                            type,
+                            SYNCED_NOTIFICATION_ACTION_COUNT);
+  actions_[type] = true;
+}
+
+void SyncedNotificationStats::RecordAggregateStats() {
+  DCHECK(!id_.empty());
+
+  for (size_t i = 0; i < SYNCED_NOTIFICATION_ACTION_COUNT; i++) {
+    if (!actions_[i])
+      continue;
+    UMA_HISTOGRAM_ENUMERATION("Notifications.PerNotificationActions",
+                              i,
+                              SYNCED_NOTIFICATION_ACTION_COUNT);

[Alexei Svitkine (slow), 2013/10/11 14:50:12]

This looks strange to me. I'm not familiar with this "PerNotification" thing
either. Can you explain what this is trying to measure? (Not necessarily this
specific code, but your CL in general - what questions do you want the metrics
to answer?)

[Pete Williamson, 2013/10/14 03:46:07]

I was hoping to ask you if I needed it or not.  I copied this code from the
notification center changelist (https://codereview.chromium.org/23902057),
thinnking it was required to get 
    content::RecordAction(
                 content::UserMetricsAction("Notifications.ShowMessageCenter")
to work properly.  I'll be happy to remove it since I'm not quite sure what it
does.

Now, what I am trying to measure.  The idea of the synced notifications feature
is that now servers can send notifications directly to chrome clients.
We haven't done this before, and we want to measure the reaction of users to
this feature.  We want to know if users are turning it off.  We want to know if
users are dismissing the notifications while they are being shown as a "toast"
or waiting until later when they go into the notification center.  We want to
know if users are clicking on our notifications.  We want to know if users are
muting all notifications and leaving them turned off, or only turning off
notifications while they are presenting.  Each individual service which sends a
notification can be enabled or disabled separately, and my next changelist will
try to measure which sending services get turned off in practice.

[Alexei Svitkine (slow), 2013/10/15 17:12:03]

The other stuff should work on its own. Unless you actually need this info, I
suggest not adding this (which should allow you to simplify your code a lot -
e.g. possibly even removing this class and inlining its impl into the caller).
Got it.

It sounds like just having the Notifications.Actions enum histogram should be
enough. I don't see the purpose of Notifications.PerNotificationActions, so
please remove it (or else find out its purpose from the author of the CL you're
copying and clarify why it's needed.)

[Pete Williamson, 2013/10/16 02:24:21]

OK, after a closer reading of the code, the PerNotificationActions seems to be
for keeping track on a notification by notification basis about what happens to
the notification during its lifetime.  I think in my scenario we are find
without it, so I removed it.

+  }
+}
+
+}  // namespace notifier