Improve histogram emitting and tests for domain diversity metrics

Histogram History.DomainCountQueryTime is now emitted with
SCOPED_UMA_HISTOGRAM_TIMER instead of an explicitly maintained timer;
Typos and other minor nits in HistoryService unittests are also fixed.

Change-Id: Idbd3ff3fb676a00ae21db1ef126d67afe947a161
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2023233
Commit-Queue: Mingjing Zhang <mjzhang@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#736012}

components/history/core/browser/history_backend.cc

@@ -148,7 +148,7 @@ const int kExpireDaysThreshold = 90;
 
 // The maximum number of days for which domain visit metrics are computed
 // each time HistoryBackend::GetDomainDiversity() is called.
-constexpr int kDomainDiversityBacktrackMaxDays = 7;
+constexpr int kDomainDiversityMaxBacktrackedDays = 7;
 
 // An offset that corrects possible error in date/time arithmetic caused by
 // fluctuation of day length due to Daylight Saving Time (DST). For example,
@@ -1236,17 +1236,16 @@ DomainDiversityResults HistoryBackend::GetDomainDiversity(
     int number_of_days_to_report,
     DomainMetricBitmaskType metric_type_bitmask) {
   DCHECK_GE(number_of_days_to_report, 0);
-  DCHECK_LE(number_of_days_to_report, kDomainDiversityBacktrackMaxDays);
   DomainDiversityResults result;
 
   if (!db_)
     return result;
 
   number_of_days_to_report =
-      std::min(number_of_days_to_report, kDomainDiversityBacktrackMaxDays);
+      std::min(number_of_days_to_report, kDomainDiversityMaxBacktrackedDays);
 
   base::Time current_midnight = report_time.LocalMidnight();
-  base::ElapsedTimer db_timer;
+  SCOPED_UMA_HISTOGRAM_TIMER("History.DomainCountQueryTime");
 
   for (int days_back = 0; days_back < number_of_days_to_report; ++days_back) {
     DomainMetricSet single_metric_set;
@@ -1279,8 +1278,6 @@ DomainDiversityResults HistoryBackend::GetDomainDiversity(
     current_midnight = MidnightNDaysLater(current_midnight, -1);
   }
 
-  UMA_HISTOGRAM_COUNTS_10000("History.DomainCountQueryTime",
-                             db_timer.Elapsed().InMilliseconds());
   return result;
 }
 

components/history/core/browser/history_service_unittest.cc

@@ -729,7 +729,7 @@ void TestDomainMetric(const base::Optional<DomainMetricCountType>& metric,
                       int expected) {
   if (expected >= 0) {
     ASSERT_TRUE(metric.has_value());
-    EXPECT_EQ(metric.value().count, expected);
+    EXPECT_EQ(expected, metric.value().count);
   } else {
     EXPECT_FALSE(metric.has_value());
   }
@@ -780,7 +780,7 @@ TEST_F(HistoryServiceTest, GetDomainDiversityShortBasetimeRange) {
   base::Time query_time = base::Time::Now();
 
   // Make sure |query_time| is at least some time past the midnight so that
-  // some domain visits can be inserted between |query_time| and midnight\
+  // some domain visits can be inserted between |query_time| and midnight
   // for testing.
   query_time =
       std::max(query_time.LocalMidnight() + base::TimeDelta::FromMinutes(10),
@@ -816,16 +816,16 @@ TEST_F(HistoryServiceTest, GetDomainDiversityShortBasetimeRange) {
       history::kEnableLast1DayMetric | history::kEnableLast7DayMetric |
           history::kEnableLast28DayMetric,
       &tracker_);
-  EXPECT_EQ(res.size(), 0u);
+  EXPECT_EQ(0u, res.size());
 
-  // Metrics will be computed for each of the 4 continuous midnights .
+  // Metrics will be computed for each of the 4 continuous midnights.
   res = GetDomainDiversityHelper(
       history, GetTimeInThePast(query_time, 4, 0), query_time,
       history::kEnableLast1DayMetric | history::kEnableLast7DayMetric |
           history::kEnableLast28DayMetric,
       &tracker_);
 
-  ASSERT_EQ(res.size(), 4u);
+  ASSERT_EQ(4u, res.size());
 
   TestDomainMetricSet(res[0], 1, 2, 2);
   TestDomainMetricSet(res[1], 2, 2, 2);
@@ -863,12 +863,12 @@ TEST_F(HistoryServiceTest, GetDomainDiversityLongBasetimeRange) {
                 GetTimeInThePast(query_time, 1, 13));
 
   DomainDiversityResults res = GetDomainDiversityHelper(
-      history, GetTimeInThePast(query_time, 7, 12), query_time,
+      history, GetTimeInThePast(query_time, 10, 12), query_time,
       history::kEnableLast1DayMetric | history::kEnableLast7DayMetric |
           history::kEnableLast28DayMetric,
       &tracker_);
   // Only up to seven days will be considered.
-  ASSERT_EQ(res.size(), 7u);
+  ASSERT_EQ(7u, res.size());
 
   TestDomainMetricSet(res[0], 2, 3, 5);
   TestDomainMetricSet(res[1], 1, 2, 4);
@@ -898,7 +898,7 @@ TEST_F(HistoryServiceTest, GetDomainDiversityBitmaskTest) {
       history, GetTimeInThePast(query_time, 7, 12), query_time,
       history::kEnableLast1DayMetric | history::kEnableLast7DayMetric,
       &tracker_);
-  ASSERT_EQ(res.size(), 7u);
+  ASSERT_EQ(7u, res.size());
 
   TestDomainMetricSet(res[0], 1, 2, -1);
   TestDomainMetricSet(res[1], 0, 1, -1);
@@ -913,7 +913,7 @@ TEST_F(HistoryServiceTest, GetDomainDiversityBitmaskTest) {
       history::kEnableLast28DayMetric | history::kEnableLast7DayMetric,
       &tracker_);
 
-  ASSERT_EQ(res.size(), 6u);
+  ASSERT_EQ(6u, res.size());
   TestDomainMetricSet(res[0], -1, 2, 3);
   TestDomainMetricSet(res[1], -1, 1, 2);
   TestDomainMetricSet(res[2], -1, 1, 2);