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

chrome/browser/engagement/site_engagement_service_unittest.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/engagement/site_engagement_service.h"
 
 #include <utility>
 
 #include "base/files/scoped_temp_dir.h"
 #include "base/macros.h"
@@ skipping 1483 matching lines @@
   // Add some more points and ensure the value is persisted.
   service->AddPoints(origin, 3);
 
   last_engagement_time = base::Time::FromInternalValue(
       profile()->GetPrefs()->GetInt64(prefs::kSiteEngagementLastUpdateTime));
 
   EXPECT_EQ(later_in_day, last_engagement_time);
   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);
 
   GURL url1("http://www.google.com/");
   GURL url2("https://www.google.com/");
   GURL url3("https://drive.google.com/");
   GURL url4("https://maps.google.com/");
 
   service->AddPoints(url1, 1);
@@ skipping 77 matching lines @@
 
   EXPECT_EQ(3, CheckScoreFromSettingsOnThread(content::BrowserThread::IO,
                                               settings_map, url1));
   EXPECT_EQ(3, CheckScoreFromSettingsOnThread(content::BrowserThread::FILE,
                                               settings_map, url2));
   EXPECT_EQ(4, CheckScoreFromSettingsOnThread(content::BrowserThread::FILE,
                                               incognito_settings_map, url1));
   EXPECT_EQ(3, CheckScoreFromSettingsOnThread(content::BrowserThread::IO,
                                               incognito_settings_map, url2));
 }