Make the site engagement service more robust to clock changes and time inconsistencies.

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 96e3b9ab8a0e497e2a54a8993a459d891bd2e3e0..fd5968d0ea3afb2839b2dc1c48b1c09b83445eef 100644
--- a/chrome/browser/engagement/site_engagement_service_unittest.cc
+++ b/chrome/browser/engagement/site_engagement_service_unittest.cc
@@ -1501,6 +1501,87 @@ TEST_F(SiteEngagementServiceTest, LastEngagementTime) {
   EXPECT_EQ(later_in_day, service->GetLastEngagementTime());
 }
 
+TEST_F(SiteEngagementServiceTest, CleanupMovesScoreBackToNow) {
+  base::SimpleTestClock* clock = new base::SimpleTestClock();
+  std::unique_ptr<SiteEngagementService> service(
+      new SiteEngagementService(profile(), base::WrapUnique(clock)));
+  base::Time last_engagement_time;
+
+  base::Time current_day = GetReferenceTime();
+  clock->SetNow(current_day);
+
+  GURL origin("http://www.google.com/");
+  service->AddPoints(origin, 1);
+  EXPECT_EQ(1, service->GetScore(origin));
+  EXPECT_EQ(current_day, service->GetLastEngagementTime());
+
+  // Send the clock back in time before the stale period and add engagement for
+  // a new origin. Go forward in time so that the original origin is between
+  // rebase_time and now. Ensure that the original origin has its last
+  // engagement time updated to now.
+  base::Time before_stale_period =
+      clock->Now() - service->GetStalePeriod() - service->GetMaxDecayPeriod();
+  clock->SetNow(before_stale_period);

[benwells, 2016/10/19 23:54:06]

What would happen if CleanupEngagementScores(true) was called now? From my
understanding of the code there is still a DCHECK that would fire, is that
right?

+
+  GURL origin1("http://maps.google.com/");
+  service->AddPoints(origin1, 1);
+  EXPECT_EQ(before_stale_period, service->GetLastEngagementTime());
+
+  // Set the clock in the future, just under the rebase limit from
+  // |later_in_day|.
+  clock->SetNow(current_day + service->GetMaxDecayPeriod() -
+                base::TimeDelta::FromSeconds(30));
+  service->CleanupEngagementScores(true);
+  EXPECT_EQ(clock->Now(),
+            service->CreateEngagementScore(origin).last_engagement_time());
+  EXPECT_EQ(clock->Now(), service->GetLastEngagementTime());
+  EXPECT_EQ(1, service->GetScore(origin));
+  EXPECT_EQ(0, service->GetScore(origin1));
+}
+
+TEST_F(SiteEngagementServiceTest, CleanupMovesScoreBackToRebase) {
+  base::SimpleTestClock* clock = new base::SimpleTestClock();
+  std::unique_ptr<SiteEngagementService> service(
+      new SiteEngagementService(profile(), base::WrapUnique(clock)));
+  base::Time last_engagement_time;
+
+  base::Time current_day = GetReferenceTime();
+  clock->SetNow(current_day);
+
+  GURL origin("http://www.google.com/");
+  service->ResetScoreForURL(origin, 5);
+  service->AddPoints(origin, 5);
+  EXPECT_EQ(10, service->GetScore(origin));
+  EXPECT_EQ(current_day, service->GetLastEngagementTime());
+
+  // Send the clock back in time before the stale period and add engagement for
+  // a new origin. Go forward in time so that the original origin is between
+  // last_engagement_time and rebase_time. Ensure that the original origin has
+  // its last engagement time updated to rebase_time, and it has decayed when we
+  // access the score.
+  base::Time before_stale_period =
+      clock->Now() - service->GetStalePeriod() - service->GetMaxDecayPeriod();
+  clock->SetNow(before_stale_period);
+
+  GURL origin1("http://maps.google.com/");
+  service->AddPoints(origin1, 1);
+
+  EXPECT_EQ(before_stale_period, service->GetLastEngagementTime());
+
+  // Set the clock such that |origin|'s last engagement time is between
+  // last_engagement_time and rebase_time.
+  clock->SetNow(current_day + service->GetStalePeriod() +
+                service->GetMaxDecayPeriod() -
+                base::TimeDelta::FromSeconds((30)));
+  base::Time rebased_time = clock->Now() - service->GetMaxDecayPeriod();
+  service->CleanupEngagementScores(true);
+  EXPECT_EQ(rebased_time,
+            service->CreateEngagementScore(origin).last_engagement_time());
+  EXPECT_EQ(rebased_time, service->GetLastEngagementTime());
+  EXPECT_EQ(5, service->GetScore(origin));
+  EXPECT_EQ(0, service->GetScore(origin1));
+}
+
 TEST_F(SiteEngagementServiceTest, IncognitoEngagementService) {
   SiteEngagementService* service = SiteEngagementService::Get(profile());
   ASSERT_TRUE(service);