Enable history counting for time ranges.

components/history/core/browser/visit_database_unittest.cc

Index: components/history/core/browser/visit_database_unittest.cc
diff --git a/components/history/core/browser/visit_database_unittest.cc b/components/history/core/browser/visit_database_unittest.cc
index be2594a1ba4363da10af992eb78173cfdf61bf42..677dd8fe81613c57e7e351dfc380368c49d336ad 100644
--- a/components/history/core/browser/visit_database_unittest.cc
+++ b/components/history/core/browser/visit_database_unittest.cc
@@ -416,4 +416,175 @@ TEST_F(VisitDatabaseTest, GetVisibleVisitsForURL) {
   EXPECT_TRUE(IsVisitInfoEqual(results[2], test_visit_rows[0]));
 }
 
+TEST_F(VisitDatabaseTest, GetHistoryCount) {
+  Time today = Time::Now().LocalMidnight();
+  // Find the beginning of tomorrow and the day after tomorrow. We cannot use
+  // TimeDelta::FromDays(1), as this simply adds 24 hours and thus does not work
+  // correctly with DST shifts. Instead, we'll jump 36 hours (i.e. somewhere
+  // in the middle of the next day), and use |LocalMidnight()| to round down
+  // to the beginning of the day in the local time, taking timezones and DST
+  // into account. This is necessary to achieve the same equivalence class
+  // on days as the DATE(..., 'localtime') function in SQL.
+  Time tomorrow = (today + TimeDelta::FromHours(36)).LocalMidnight();

[sdefresne, 2015/09/29 08:21:49]

nit: this confused me, as there can never be visit in the future, what about
renaming the variables to "two_days_ago", "one_day_ago" and "today"? then use
ranges like [one_day_ago, today] instead of [today, tomorrow].

[msramek, 2015/09/29 11:04:41]

Done. Changed to the two previous days. And in the part about DST, we now
iterate a year backwards.

+  Time after_tomorrow = (tomorrow + TimeDelta::FromHours(36)).LocalMidnight();
+  Time now = today;
+
+  ui::PageTransition standard_transition = ui::PageTransitionFromInt(
+      ui::PAGE_TRANSITION_TYPED |
+      ui::PAGE_TRANSITION_CHAIN_START |
+      ui::PAGE_TRANSITION_CHAIN_END);
+
+  // Add 5 visits (3 distinct URLs) for today. Whether the URL was browsed
+  // on this machine or synced has no effect.
+  VisitRow today_1(1, now, 0, standard_transition, 0);
+  today_1.visit_id = 1;
+  AddVisit(&today_1, SOURCE_BROWSED);
+  now += TimeDelta::FromHours(1);
+
+  VisitRow today_2(2, now, 0, standard_transition, 0);
+  today_2.visit_id = 2;
+  AddVisit(&today_2, SOURCE_BROWSED);
+  now += TimeDelta::FromHours(1);
+
+  VisitRow today_3(1, now, 0, standard_transition, 0);
+  today_3.visit_id = 3;
+  AddVisit(&today_3, SOURCE_SYNCED);
+  now += TimeDelta::FromHours(1);
+
+  VisitRow today_4(3, now, 0, standard_transition, 0);
+  today_4.visit_id = 4;
+  AddVisit(&today_4, SOURCE_SYNCED);
+  now += TimeDelta::FromHours(1);
+
+  VisitRow today_5(2, now, 0, standard_transition, 0);
+  today_5.visit_id = 5;
+  AddVisit(&today_5, SOURCE_BROWSED);
+  now += TimeDelta::FromHours(1);
+
+  // Add 4 more visits for tomorrow. One of them is invalid, as it's not
+  // a user-visible navigation. Of the remaining 3, only 2 are unique.
+  now = tomorrow;
+
+  VisitRow tomorrow_1(1, now, 0, standard_transition, 0);
+  tomorrow_1.visit_id = 6;
+  AddVisit(&tomorrow_1, SOURCE_BROWSED);
+  now += TimeDelta::FromHours(1);
+
+  VisitRow tomorrow_2(1, now, 0, standard_transition, 0);
+  tomorrow_2.visit_id = 7;
+  AddVisit(&tomorrow_2, SOURCE_BROWSED);
+  now += TimeDelta::FromHours(1);
+
+  VisitRow tomorrow_3(2, now, 0, ui::PAGE_TRANSITION_AUTO_SUBFRAME, 0);
+  tomorrow_3.visit_id = 8;
+  AddVisit(&tomorrow_3, SOURCE_BROWSED);
+  now += TimeDelta::FromHours(1);
+
+  VisitRow tomorrow_4(3, now, 0, standard_transition, 0);
+  tomorrow_4.visit_id = 9;
+  AddVisit(&tomorrow_4, SOURCE_BROWSED);
+  now += TimeDelta::FromHours(1);
+
+  int result;
+
+  // There are 3 distinct URLs today.
+  EXPECT_TRUE(GetHistoryCount(today, tomorrow, &result));
+  EXPECT_EQ(3, result);
+
+  // For today and tomorrow, there should be 5 per-day unique URLs.
+  EXPECT_TRUE(GetHistoryCount(today, after_tomorrow, &result));
+  EXPECT_EQ(5, result);
+
+  // Since we only have entries for today and tomorrow, the infinite time
+  // range should yield the same result.
+  EXPECT_TRUE(GetHistoryCount(Time(), Time::Max(), &result));
+  EXPECT_EQ(5, result);
+
+  // Narrowing the range to exclude |today_1| will still return 5,
+  // because |today_1| is not unique.
+  EXPECT_TRUE(GetHistoryCount(
+      today + TimeDelta::FromHours(2), after_tomorrow, &result));
+  EXPECT_EQ(5, result);
+
+  // Narrowing the range to exclude |tomorrow_4| will return 4,
+  // because |tomorrow_4| is unique.
+  EXPECT_TRUE(GetHistoryCount(
+      today, tomorrow + TimeDelta::FromHours(3), &result));
+  EXPECT_EQ(4, result);
+
+  // Narrowing the range to exclude both |today_1| and |tomorrow_4| will
+  // still return 4.
+  EXPECT_TRUE(GetHistoryCount(today + TimeDelta::FromHours(2),
+                              tomorrow + TimeDelta::FromHours(3),
+                              &result));
+  EXPECT_EQ(4, result);
+
+  // A range that contains no visits will return 0.
+  EXPECT_TRUE(GetHistoryCount(today + TimeDelta::FromMicroseconds(1),
+                              today + TimeDelta::FromHours(1),
+                              &result));
+  EXPECT_EQ(0, result);
+
+  // If this timezone uses DST, test the behavior on days when the time
+  // is shifted forward and backward.
+  Time shift_forward;
+  Time shift_backward;
+  for (int i = 0; i < 366; i++) {

[lwchkg, 2015/09/28 18:14:51]

Forward at least one day before the loop start. Otherwise the data with ids 6-9
can interfere with the test, which results in failure due to extra counts.

[msramek, 2015/09/29 11:04:41]

Thanks for catching this. Since now after sdefresne@'s comment I changed the
iteration to go backwards, I skipped one day in the other direction. I.e. the
data above are in the range [today - 2 days, today], and the iteration tests
days starting at [today - 4 days, today - 4 days + 24hrs] and backwards.

+    today = (today + TimeDelta::FromHours(36)).LocalMidnight();
+    tomorrow = today + TimeDelta::FromHours(24);

[lwchkg, 2015/09/28 18:14:51]

Is it better to name it after_24_hours?

[msramek, 2015/09/29 11:04:41]

Done.

+
+    if (today.LocalMidnight() == tomorrow.LocalMidnight()) {

[lwchkg, 2015/09/28 18:14:51]

nits: today.LocalMidnight() can be replaced by just today.

[msramek, 2015/09/29 11:04:41]

Done.

+      // More than 24 hours. Shift backward.
+      shift_backward = today;
+    } else if (tomorrow > tomorrow.LocalMidnight()) {
+      // Less than 24 hours. Shift forward.
+      shift_forward = today;
+    }
+
+    if (!shift_backward.is_null() && !shift_forward.is_null())
+      break;
+  }
+
+  // Test the backward shift. Add two visits for the same page on midnight and
+  // 24 hours later. The count should be 1, not 2, because the day is longer
+  // than 24 hours, and the two visits will be regarded as duplicate.
+  if (!shift_backward.is_null()) {
+    VisitRow backward_1(1, shift_backward, 0, standard_transition, 0);
+    backward_1.visit_id = 10;
+    AddVisit(&backward_1, SOURCE_BROWSED);
+
+    VisitRow backward_2(1, shift_backward + TimeDelta::FromHours(24),
+                        0, standard_transition, 0);
+    backward_2.visit_id = 11;
+    AddVisit(&backward_2, SOURCE_BROWSED);
+
+    EXPECT_TRUE(GetHistoryCount(shift_backward,
+                                shift_backward + TimeDelta::FromHours(25),
+                                &result));
+    EXPECT_EQ(1, result);
+  }
+
+  // Test the forward shift. Add two visits for the same page at midnight and
+  // almost 24 hours later. The count should be 2, not 1. The visits would be
+  // regarded as duplicate in a normal 24 hour day, but in this case the second
+  // visit is already in the next day.
+  if (!shift_forward.is_null()) {
+    VisitRow forward_1(1, shift_forward, 0, standard_transition, 0);
+    forward_1.visit_id = 12;
+    AddVisit(&forward_1, SOURCE_BROWSED);
+
+    Time almost_24_hours_later = shift_forward +
+                                 TimeDelta::FromHours(24) -
+                                 TimeDelta::FromMicroseconds(1);
+    VisitRow forward_2(1, almost_24_hours_later, 0, standard_transition, 0);
+    forward_2.visit_id = 13;
+    AddVisit(&forward_2, SOURCE_BROWSED);
+
+    EXPECT_TRUE(GetHistoryCount(shift_forward,
+                                shift_forward + TimeDelta::FromHours(24),
+                                &result));
+    EXPECT_EQ(2, result);
+  }
+}
+
 }  // namespace history