Add UMA metrics to the site engagement service.

chrome/browser/engagement/site_engagement_service.cc

Index: chrome/browser/engagement/site_engagement_service.cc
diff --git a/chrome/browser/engagement/site_engagement_service.cc b/chrome/browser/engagement/site_engagement_service.cc
index 5f0e2fc915717ca337c1793058497b30c62f07ed..12a9879123b851530a5a751d305e1a055cb77824 100644
--- a/chrome/browser/engagement/site_engagement_service.cc
+++ b/chrome/browser/engagement/site_engagement_service.cc
@@ -12,6 +12,7 @@
 #include "base/values.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/engagement/site_engagement_helper.h"
+#include "chrome/browser/engagement/site_engagement_metrics.h"
 #include "chrome/browser/engagement/site_engagement_service_factory.h"
 #include "chrome/common/chrome_switches.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
@@ -102,6 +103,15 @@ void SiteEngagementScore::AddPoints(double points) {
   last_engagement_time_ = now;
 }
 
+bool SiteEngagementScore::MaxPointsPerDayAdded() {
+  if (!last_engagement_time_.is_null() &&
+      clock_->Now().LocalMidnight() != last_engagement_time_.LocalMidnight()) {
+    points_added_today_ = 0;
+  }
+
+  return points_added_today_ == kMaxPointsPerDay;
+}
+
 bool SiteEngagementScore::UpdateScoreDict(base::DictionaryValue* score_dict) {
   double raw_score_orig = 0;
   double points_added_today_orig = 0;
@@ -163,6 +173,7 @@ bool SiteEngagementService::IsEnabled() {
 
 SiteEngagementService::SiteEngagementService(Profile* profile)
     : profile_(profile) {
+  RecordStartupUmaStats();
 }
 
 SiteEngagementService::~SiteEngagementService() {
@@ -188,22 +199,12 @@ double SiteEngagementService::GetScore(const GURL& url) {
 }
 
 double SiteEngagementService::GetTotalEngagementPoints() {
-  HostContentSettingsMap* settings_map =
-    HostContentSettingsMapFactory::GetForProfile(profile_);
-  ContentSettingsForOneType engagement_settings;
-  settings_map->GetSettingsForOneType(CONTENT_SETTINGS_TYPE_SITE_ENGAGEMENT,
-                                      std::string(), &engagement_settings);
+  std::map<GURL, double> score_map = GetScoreMap();
+
   double total_score = 0;
-  for (const auto& site : engagement_settings) {
-    GURL origin(site.primary_pattern.ToString());
-    if (!origin.is_valid())
-      continue;
+  for (const auto& value : score_map)
+    total_score += value.second;
 
-    scoped_ptr<base::DictionaryValue> score_dict =
-        GetScoreDictForOrigin(settings_map, origin);
-    SiteEngagementScore score(&clock_, *score_dict);
-    total_score += score.Score();
-  }
   return total_score;
 }
 
@@ -245,4 +246,63 @@ void SiteEngagementService::AddPoints(const GURL& url, double points) {
                                     CONTENT_SETTINGS_TYPE_SITE_ENGAGEMENT,
                                     std::string(), score_dict.release());
   }
+
+  // If the origin has now received the maximum number of points it can today,
+  // record a UMA statistic of all of the origins with maxed daily engagement.
+  if (score.MaxPointsPerDayAdded()) {
+    SiteEngagementMetrics::RecordOriginsWithMaxDailyEngagement(
+        OriginsWithMaxDailyEngagement());

[calamity, 2015/09/25 07:08:33]

Hmm. It's kind of weird to log this each time a score gets to the max. We'll get
a triangle effect to account for. I'm not entirely sure what a nice way around
this is...

I think it's customary to either count one event each it happens (and therefore
lose distribution data) or to add a 'last_logged' thing to prefs and then log if
more than a day has passed since the last report was sent. I'm wary about doing
that sort of logging though because if we complicate the decay function at all,
we might get weird phase effects in our data....

Maybe just keep it simple and log once per event, WDYT?

[dominickn, 2015/09/28 03:34:50]

I wanted to try and avoid the overhead of finding all of the maxed out origins
per each increment in engagement, which is why I only logged this when a score
reached the maximum. I agree that having a last_logged date in prefs is
undesirable.

What I'm aiming for with this statistic is having a histogram of the total
number of origins with max engagement per client. By logging the total origins
every time an origin reaches the max, you get a CDF where each increasing number
of origins is a subset of all of the previous numbers (putting aside when people
clear the prefs). Does that make sense?

[calamity, 2015/09/28 07:33:47]

Ok, I'll defer to the histograms reviewer on this. Also, the histogram comment
ought to make note of this since it's fairly unusual. Just as a note, the UMA
system has a CDF column which will not be the actual CDF for the stat.

+  }
+
+  // If the origin has now received the maximum total number of points,
+  // record a UMA statistic of all of the origins with maximum total points.
+  if (score.Score() == SiteEngagementScore::kMaxPoints) {
+    SiteEngagementMetrics::RecordOriginsWithMaxEngagement(
+        OriginsWithMaxEngagement());

[calamity, 2015/09/25 07:08:33]

Ditto.

[dominickn, 2015/09/28 03:34:50]

Acknowledged.

+  }
+}
+
+int SiteEngagementService::OriginsWithMaxDailyEngagement() {
+  HostContentSettingsMap* settings_map =
+      HostContentSettingsMapFactory::GetForProfile(profile_);
+  ContentSettingsForOneType engagement_settings;
+  settings_map->GetSettingsForOneType(CONTENT_SETTINGS_TYPE_SITE_ENGAGEMENT,
+                                      std::string(), &engagement_settings);

[calamity, 2015/09/25 07:08:33]

Worth factoring out a GetSiteEngagementSettings at this point I think.

[dominickn, 2015/09/28 03:34:50]

I see you've done it in your cleanup CL: I'll wait for that to land and rebase
this change on it (also addressing the PostTask so we avoid a double iteration
over the score map).

[calamity, 2015/09/28 07:33:47]

But that's based on your time on site stuff! Ugh, we're getting tangled. =(

+  int total_origins = 0;
+  for (const auto& site : engagement_settings) {
+    GURL origin(site.primary_pattern.ToString());
+    if (!origin.is_valid())
+      continue;
+
+    scoped_ptr<base::DictionaryValue> score_dict =
+        GetScoreDictForOrigin(settings_map, origin);
+    SiteEngagementScore score(&clock_, *score_dict);
+    if (score.MaxPointsPerDayAdded())
+      total_origins += 1;
+  }
+
+  return total_origins;
+}
+
+int SiteEngagementService::OriginsWithMaxEngagement() {
+  std::map<GURL, double> score_map = GetScoreMap();
+  int total_origins = 0;
+
+  for (const auto& value : score_map)
+    if (value.second == SiteEngagementScore::kMaxPoints)
+      ++total_origins;
+
+  return total_origins;
+}
+
+void SiteEngagementService::RecordStartupUmaStats() {
+  std::map<GURL, double> score_map = GetScoreMap();
+
+  double total_score = 0;
+  for (const auto& value : score_map)
+    total_score += value.second;
+
+  SiteEngagementMetrics::RecordTotalOriginsEngaged(score_map.size());
+  SiteEngagementMetrics::RecordTotalCumulativeSiteEngagement(total_score);
+  SiteEngagementMetrics::RecordEngagementPerOrigin(score_map);
 }