Add UMA metrics to the site engagement service.

chrome/browser/engagement/site_engagement_service_unittest.cc

Index: chrome/browser/engagement/site_engagement_service_unittest.cc
diff --git a/chrome/browser/engagement/site_engagement_service_unittest.cc b/chrome/browser/engagement/site_engagement_service_unittest.cc
index 8db3b4114e60e6a5ac1537724b97c86508e787fc..3eea186950d6ccf6813714c6d5d5278c8807caa6 100644
--- a/chrome/browser/engagement/site_engagement_service_unittest.cc
+++ b/chrome/browser/engagement/site_engagement_service_unittest.cc
@@ -3,9 +3,11 @@
 // found in the LICENSE file.
 
 #include "base/command_line.h"
+#include "base/test/histogram_tester.h"
 #include "base/test/simple_test_clock.h"
 #include "base/values.h"
 #include "chrome/browser/engagement/site_engagement_helper.h"
+#include "chrome/browser/engagement/site_engagement_metrics.h"
 #include "chrome/browser/engagement/site_engagement_service.h"
 #include "chrome/browser/engagement/site_engagement_service_factory.h"
 #include "chrome/common/chrome_switches.h"
@@ -320,6 +322,70 @@ class SiteEngagementServiceTest : public BrowserWithTestWindowTest {
   }
 };
 
+TEST_F(SiteEngagementServiceTest, GetMedianEngagement) {
+  SiteEngagementService* service =
+      SiteEngagementServiceFactory::GetForProfile(profile());
+  DCHECK(service);
+
+  GURL url1("http://www.google.com/");
+  GURL url2("https://www.google.com/");
+  GURL url3("https://drive.google.com/");
+  GURL url4("https://maps.google.com/");
+  GURL url5("https://youtube.com/");
+  GURL url6("https://images.google.com/");
+
+  {
+    // For zero total sites, the median is 0.
+    std::map<GURL, double> score_map = service->GetScoreMap();
+    EXPECT_TRUE(0 == score_map.size());
+    EXPECT_DOUBLE_EQ(0, service->GetMedianEngagement(score_map));
+  }
+
+  {
+    // For odd total sites, the median is the middle score.
+    service->AddPoints(url1, 1);
+    std::map<GURL, double> score_map = service->GetScoreMap();
+    EXPECT_TRUE(1 == score_map.size());
+    EXPECT_DOUBLE_EQ(1, service->GetMedianEngagement(score_map));
+  }
+
+  {
+    // For even total sites, the median is the mean of the middle two scores.
+    service->AddPoints(url2, 2);
+    std::map<GURL, double> score_map = service->GetScoreMap();
+    EXPECT_TRUE(2 == score_map.size());
+    EXPECT_DOUBLE_EQ(1.5, service->GetMedianEngagement(score_map));
+  }
+
+  {
+    service->AddPoints(url3, 1.4);
+    std::map<GURL, double> score_map = service->GetScoreMap();
+    EXPECT_TRUE(3 == score_map.size());
+    EXPECT_DOUBLE_EQ(1.4, service->GetMedianEngagement(score_map));
+  }
+
+  {
+    service->AddPoints(url4, 1.8);
+    std::map<GURL, double> score_map = service->GetScoreMap();
+    EXPECT_TRUE(4 == score_map.size());
+    EXPECT_DOUBLE_EQ(1.6, service->GetMedianEngagement(score_map));
+  }
+
+  {
+    service->AddPoints(url5, 2.5);
+    std::map<GURL, double> score_map = service->GetScoreMap();
+    EXPECT_TRUE(5 == score_map.size());
+    EXPECT_DOUBLE_EQ(1.8, service->GetMedianEngagement(score_map));
+  }
+
+  {
+    service->AddPoints(url6, 3);
+    std::map<GURL, double> score_map = service->GetScoreMap();
+    EXPECT_TRUE(6 == score_map.size());
+    EXPECT_DOUBLE_EQ(1.9, service->GetMedianEngagement(score_map));
+  }
+}
+
 // Tests that the Site Engagement service is hooked up properly to navigations
 // by performing two navigations and checking the engagement score increases
 // both times.
@@ -342,12 +408,37 @@ TEST_F(SiteEngagementServiceTest, ScoreIncrementsOnPageRequest) {
   EXPECT_LT(prev_score, service->GetScore(url));
 }
 
-// Expect that site engagement scores for several sites are correctly aggregated
-// by GetTotalEngagementPoints().
-TEST_F(SiteEngagementServiceTest, GetTotalNavigationPoints) {
-  SiteEngagementService* service =
-      SiteEngagementServiceFactory::GetForProfile(profile());
-  DCHECK(service);
+// Expect that site engagement scores for several sites are correctly
+// aggregated during navigation events.
+TEST_F(SiteEngagementServiceTest, GetNavigationPointsAndHistograms) {
+  base::SimpleTestClock* clock = new base::SimpleTestClock();
+  scoped_ptr<SiteEngagementService> service(
+      new SiteEngagementService(profile(), make_scoped_ptr(clock)));
+
+  base::Time current_day = GetReferenceTime();
+  clock->SetNow(current_day);
+
+  base::HistogramTester histograms;
+
+  // Histograms should start empty as the testing SiteEngagementService
+  // constructor does not record metrics.
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kTotalEngagementHistogram,
+                              0);
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kTotalOriginsHistogram, 0);
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kMeanEngagementHistogram,
+                              0);
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kMedianEngagementHistogram,
+                              0);
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kEngagementScoreHistogram,
+                              0);
+  histograms.ExpectTotalCount(
+      SiteEngagementMetrics::kOriginsWithMaxEngagementHistogram, 0);
+  histograms.ExpectTotalCount(
+      SiteEngagementMetrics::kOriginsWithMaxDailyEngagementHistogram, 0);
+  histograms.ExpectTotalCount(
+      SiteEngagementMetrics::kPercentOriginsWithMaxEngagementHistogram, 0);
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kEngagementTypeHistogram,
+                              0);
 
   // The https and http versions of www.google.com should be separate.
   GURL url1("https://www.google.com/");
@@ -362,25 +453,138 @@ TEST_F(SiteEngagementServiceTest, GetTotalNavigationPoints) {
   EXPECT_EQ(0.5, service->GetScore(url1));
   EXPECT_EQ(0.5, service->GetTotalEngagementPoints());
 
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kTotalEngagementHistogram,
+                              1);
+  histograms.ExpectUniqueSample(SiteEngagementMetrics::kTotalOriginsHistogram,
+                                1, 1);
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kMeanEngagementHistogram,
+                              1);
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kMedianEngagementHistogram,
+                              1);
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kEngagementScoreHistogram,
+                              1);
+  histograms.ExpectUniqueSample(
+      SiteEngagementMetrics::kOriginsWithMaxEngagementHistogram, 0, 1);
+  histograms.ExpectUniqueSample(
+      SiteEngagementMetrics::kOriginsWithMaxDailyEngagementHistogram, 0, 1);
+  histograms.ExpectUniqueSample(
+      SiteEngagementMetrics::kPercentOriginsWithMaxEngagementHistogram, 0, 1);
+  histograms.ExpectUniqueSample(SiteEngagementMetrics::kEngagementTypeHistogram,
+                                SiteEngagementMetrics::ENGAGEMENT_NAVIGATION,
+                                1);
+
+  clock->SetNow(GetReferenceTime() + base::TimeDelta::FromMinutes(59));
+
   service->HandleNavigation(url2, ui::PAGE_TRANSITION_GENERATED);
   service->HandleNavigation(url2, ui::PAGE_TRANSITION_AUTO_TOPLEVEL);
   EXPECT_EQ(1, service->GetScore(url2));
   EXPECT_EQ(1.5, service->GetTotalEngagementPoints());
 
+  histograms.ExpectUniqueSample(SiteEngagementMetrics::kEngagementTypeHistogram,
+                                SiteEngagementMetrics::ENGAGEMENT_NAVIGATION,
+                                3);
+
+  clock->SetNow(GetReferenceTime() + base::TimeDelta::FromMinutes(60));
+
   service->HandleNavigation(url3, ui::PAGE_TRANSITION_TYPED);
   EXPECT_EQ(0.5, service->GetScore(url3));
   EXPECT_EQ(2, service->GetTotalEngagementPoints());
 
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kTotalEngagementHistogram,
+                              2);
+  histograms.ExpectBucketCount(SiteEngagementMetrics::kTotalOriginsHistogram, 1,
+                               1);
+  histograms.ExpectBucketCount(SiteEngagementMetrics::kTotalOriginsHistogram, 3,
+                               1);
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kMeanEngagementHistogram,
+                              2);
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kMedianEngagementHistogram,
+                              2);
+  // Recorded per origin.
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kEngagementScoreHistogram,
+                              4);
+  histograms.ExpectUniqueSample(
+      SiteEngagementMetrics::kOriginsWithMaxEngagementHistogram, 0, 2);
+  histograms.ExpectUniqueSample(
+      SiteEngagementMetrics::kOriginsWithMaxDailyEngagementHistogram, 0, 2);
+  histograms.ExpectUniqueSample(
+      SiteEngagementMetrics::kPercentOriginsWithMaxEngagementHistogram, 0, 2);
+  histograms.ExpectUniqueSample(SiteEngagementMetrics::kEngagementTypeHistogram,
+                                SiteEngagementMetrics::ENGAGEMENT_NAVIGATION,
+                                4);
+
   service->HandleNavigation(url1, ui::PAGE_TRANSITION_GENERATED);
   service->HandleNavigation(url1, ui::PAGE_TRANSITION_TYPED);
   EXPECT_EQ(1.5, service->GetScore(url1));
   EXPECT_EQ(3, service->GetTotalEngagementPoints());
+
+  histograms.ExpectUniqueSample(SiteEngagementMetrics::kEngagementTypeHistogram,
+                                SiteEngagementMetrics::ENGAGEMENT_NAVIGATION,
+                                6);
+
+  // Advance an origin to the max for a day and advance the clock an hour before
+  // the last increment before max. Expect the histogram to be updated.
+  for (int i = 0; i < 6; ++i)
+    service->HandleNavigation(url1, ui::PAGE_TRANSITION_TYPED);
+
+  clock->SetNow(GetReferenceTime() + base::TimeDelta::FromMinutes(121));
+  service->HandleNavigation(url1, ui::PAGE_TRANSITION_TYPED);
+
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kTotalEngagementHistogram,
+                              3);
+  histograms.ExpectBucketCount(SiteEngagementMetrics::kTotalOriginsHistogram, 1,
+                               1);
+  histograms.ExpectBucketCount(SiteEngagementMetrics::kTotalOriginsHistogram, 3,
+                               2);
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kMeanEngagementHistogram,
+                              3);
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kMedianEngagementHistogram,
+                              3);
+  // Recorded per origin.
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kEngagementScoreHistogram,
+                              7);
+  histograms.ExpectUniqueSample(
+      SiteEngagementMetrics::kOriginsWithMaxEngagementHistogram, 0, 3);
+  histograms.ExpectBucketCount(
+      SiteEngagementMetrics::kOriginsWithMaxDailyEngagementHistogram, 0, 2);
+  histograms.ExpectBucketCount(
+      SiteEngagementMetrics::kOriginsWithMaxDailyEngagementHistogram, 1, 1);
+  histograms.ExpectUniqueSample(
+      SiteEngagementMetrics::kPercentOriginsWithMaxEngagementHistogram, 0, 3);
+  histograms.ExpectUniqueSample(SiteEngagementMetrics::kEngagementTypeHistogram,
+                                SiteEngagementMetrics::ENGAGEMENT_NAVIGATION,
+                                13);
 }
 
-TEST_F(SiteEngagementServiceTest, GetTotalUserInputPoints) {
-  SiteEngagementService* service =
-      SiteEngagementServiceFactory::GetForProfile(profile());
-  DCHECK(service);
+TEST_F(SiteEngagementServiceTest, GetUserInputPointsAndHistograms) {
+  base::SimpleTestClock* clock = new base::SimpleTestClock();
+  scoped_ptr<SiteEngagementService> service(
+      new SiteEngagementService(profile(), make_scoped_ptr(clock)));
+
+  base::Time current_day = GetReferenceTime();
+  clock->SetNow(current_day);
+
+  base::HistogramTester histograms;
+
+  // Histograms should start empty as the testing SiteEngagementService
+  // constructor does not record metrics.
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kTotalEngagementHistogram,
+                              0);
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kTotalOriginsHistogram, 0);
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kMeanEngagementHistogram,
+                              0);
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kMedianEngagementHistogram,
+                              0);
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kEngagementScoreHistogram,
+                              0);
+  histograms.ExpectTotalCount(
+      SiteEngagementMetrics::kOriginsWithMaxEngagementHistogram, 0);
+  histograms.ExpectTotalCount(
+      SiteEngagementMetrics::kOriginsWithMaxDailyEngagementHistogram, 0);
+  histograms.ExpectTotalCount(
+      SiteEngagementMetrics::kPercentOriginsWithMaxEngagementHistogram, 0);
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kEngagementTypeHistogram,
+                              0);
 
   // The https and http versions of www.google.com should be separate.
   GURL url1("https://www.google.com/");
@@ -391,23 +595,123 @@ TEST_F(SiteEngagementServiceTest, GetTotalUserInputPoints) {
   EXPECT_EQ(0, service->GetScore(url2));
   EXPECT_EQ(0, service->GetScore(url3));
 
-  service->HandleUserInput(url1);
+  service->HandleUserInput(url1, SiteEngagementMetrics::ENGAGEMENT_MOUSE);
   EXPECT_DOUBLE_EQ(0.05, service->GetScore(url1));
   EXPECT_DOUBLE_EQ(0.05, service->GetTotalEngagementPoints());
 
-  service->HandleUserInput(url2);
-  service->HandleUserInput(url2);
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kTotalEngagementHistogram,
+                              1);
+  histograms.ExpectUniqueSample(SiteEngagementMetrics::kTotalOriginsHistogram,
+                                1, 1);
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kMeanEngagementHistogram,
+                              1);
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kMedianEngagementHistogram,
+                              1);
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kEngagementScoreHistogram,
+                              1);
+  histograms.ExpectUniqueSample(
+      SiteEngagementMetrics::kOriginsWithMaxEngagementHistogram, 0, 1);
+  histograms.ExpectUniqueSample(
+      SiteEngagementMetrics::kOriginsWithMaxDailyEngagementHistogram, 0, 1);
+  histograms.ExpectUniqueSample(
+      SiteEngagementMetrics::kPercentOriginsWithMaxEngagementHistogram, 0, 1);
+  histograms.ExpectUniqueSample(SiteEngagementMetrics::kEngagementTypeHistogram,
+                                SiteEngagementMetrics::ENGAGEMENT_MOUSE, 1);
+
+  clock->SetNow(GetReferenceTime() + base::TimeDelta::FromMinutes(45));
+  service->HandleUserInput(url2, SiteEngagementMetrics::ENGAGEMENT_MOUSE);
+  clock->SetNow(GetReferenceTime() + base::TimeDelta::FromMinutes(60));
+  service->HandleUserInput(url2, SiteEngagementMetrics::ENGAGEMENT_KEYPRESS);
   EXPECT_DOUBLE_EQ(0.1, service->GetScore(url2));
   EXPECT_DOUBLE_EQ(0.15, service->GetTotalEngagementPoints());
 
-  service->HandleUserInput(url3);
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kTotalEngagementHistogram,
+                              2);
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kTotalOriginsHistogram, 2);
+  histograms.ExpectBucketCount(SiteEngagementMetrics::kTotalOriginsHistogram, 1,
+                               1);
+  histograms.ExpectBucketCount(SiteEngagementMetrics::kTotalOriginsHistogram, 2,
+                               1);
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kMeanEngagementHistogram,
+                              2);
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kMedianEngagementHistogram,
+                              2);
+  // Per origin.
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kEngagementScoreHistogram,
+                              3);
+  histograms.ExpectUniqueSample(
+      SiteEngagementMetrics::kOriginsWithMaxEngagementHistogram, 0, 2);
+  histograms.ExpectUniqueSample(
+      SiteEngagementMetrics::kOriginsWithMaxDailyEngagementHistogram, 0, 2);
+  histograms.ExpectUniqueSample(
+      SiteEngagementMetrics::kPercentOriginsWithMaxEngagementHistogram, 0, 2);
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kEngagementTypeHistogram,
+                              3);
+  histograms.ExpectBucketCount(SiteEngagementMetrics::kEngagementTypeHistogram,
+                               SiteEngagementMetrics::ENGAGEMENT_MOUSE, 2);
+  histograms.ExpectBucketCount(SiteEngagementMetrics::kEngagementTypeHistogram,
+                               SiteEngagementMetrics::ENGAGEMENT_KEYPRESS, 1);
+
+  clock->SetNow(GetReferenceTime() + base::TimeDelta::FromMinutes(150));
+  service->HandleUserInput(url3, SiteEngagementMetrics::ENGAGEMENT_KEYPRESS);
   EXPECT_DOUBLE_EQ(0.05, service->GetScore(url3));
   EXPECT_DOUBLE_EQ(0.2, service->GetTotalEngagementPoints());
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kEngagementTypeHistogram,
+                              4);
+  histograms.ExpectBucketCount(SiteEngagementMetrics::kEngagementTypeHistogram,
+                               SiteEngagementMetrics::ENGAGEMENT_KEYPRESS, 2);
 
-  service->HandleUserInput(url1);
-  service->HandleUserInput(url1);
+  service->HandleUserInput(url1, SiteEngagementMetrics::ENGAGEMENT_KEYPRESS);
+  service->HandleUserInput(url1, SiteEngagementMetrics::ENGAGEMENT_MOUSE);
   EXPECT_DOUBLE_EQ(0.15, service->GetScore(url1));
   EXPECT_DOUBLE_EQ(0.3, service->GetTotalEngagementPoints());
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kEngagementTypeHistogram,
+                              6);
+  histograms.ExpectBucketCount(SiteEngagementMetrics::kEngagementTypeHistogram,
+                               SiteEngagementMetrics::ENGAGEMENT_MOUSE, 3);
+  histograms.ExpectBucketCount(SiteEngagementMetrics::kEngagementTypeHistogram,
+                               SiteEngagementMetrics::ENGAGEMENT_KEYPRESS, 3);
+
+  // Advance an origin to the max and ensure the histogram is correctly
+  // incremented.
+  for (int i = 0; i < 1996; ++i)
+    service->HandleUserInput(url1, SiteEngagementMetrics::ENGAGEMENT_MOUSE);
+
+  clock->SetNow(GetReferenceTime() + base::TimeDelta::FromMinutes(210));
+  service->HandleUserInput(url1, SiteEngagementMetrics::ENGAGEMENT_MOUSE);
+
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kTotalEngagementHistogram,
+                              4);
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kTotalOriginsHistogram, 4);
+  histograms.ExpectBucketCount(SiteEngagementMetrics::kTotalOriginsHistogram, 1,
+                               1);
+  histograms.ExpectBucketCount(SiteEngagementMetrics::kTotalOriginsHistogram, 2,
+                               1);
+  histograms.ExpectBucketCount(SiteEngagementMetrics::kTotalOriginsHistogram, 3,
+                               2);
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kMeanEngagementHistogram,
+                              4);
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kMedianEngagementHistogram,
+                              4);
+  // Recorded per origin.
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kEngagementScoreHistogram,
+                              9);
+  histograms.ExpectUniqueSample(
+      SiteEngagementMetrics::kOriginsWithMaxEngagementHistogram, 0, 4);
+  histograms.ExpectTotalCount(
+      SiteEngagementMetrics::kOriginsWithMaxDailyEngagementHistogram, 4);
+  histograms.ExpectBucketCount(
+      SiteEngagementMetrics::kOriginsWithMaxDailyEngagementHistogram, 0, 3);
+  histograms.ExpectBucketCount(
+      SiteEngagementMetrics::kOriginsWithMaxDailyEngagementHistogram, 1, 1);
+  histograms.ExpectUniqueSample(
+      SiteEngagementMetrics::kPercentOriginsWithMaxEngagementHistogram, 0, 4);
+  histograms.ExpectTotalCount(SiteEngagementMetrics::kEngagementTypeHistogram,
+                              2003);
+  histograms.ExpectBucketCount(SiteEngagementMetrics::kEngagementTypeHistogram,
+                               SiteEngagementMetrics::ENGAGEMENT_MOUSE, 2000);
+  histograms.ExpectBucketCount(SiteEngagementMetrics::kEngagementTypeHistogram,
+                               SiteEngagementMetrics::ENGAGEMENT_KEYPRESS, 3);

[calamity, 2015/10/02 05:31:18]

These tests are getting too unruly. Can you just factor out a separate
histograms test that just checks each histogram event simply and then one that
checks the 1 hour interval thing.

I think if these tests fail, it's going to be confusing to figure out where and
why things are going wrong. It'd be nicer to have a single test that will fail
for histograms stuff that is divorced as possible from other functionality.

In general, it would be good to have specific tests that don't overtest the same
codepaths.

[dominickn, 2015/10/02 06:06:17]

Done.

 }
 
 // Expect that sites that have reached zero engagement are cleaned up.