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

chrome/browser/engagement/site_engagement_service.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 <stddef.h>
 
 #include <algorithm>
 #include <utility>
@@ skipping 271 matching lines @@
   // Check if we need to reset last engagement times on startup - we want to
   // avoid doing this in AddPoints() if possible. It is still necessary to check
   // in AddPoints for people who never restart Chrome, but leave it open and
   // their computer on standby.
   CleanupEngagementScores(IsLastEngagementStale());
   RecordMetrics();
 }
 
 void SiteEngagementService::CleanupEngagementScores(
     bool update_last_engagement_time) const {
-  // This method should not be called with |update_last_engagement_time| = true
-  // if the last engagement time isn't stale.
-  DCHECK(!update_last_engagement_time || IsLastEngagementStale());
-
   HostContentSettingsMap* settings_map =
       HostContentSettingsMapFactory::GetForProfile(profile_);
   std::unique_ptr<ContentSettingsForOneType> engagement_settings =
       GetEngagementContentSettings(settings_map);
 
   // We want to rebase last engagement times relative to MaxDecaysPerScore
   // periods of decay in the past.
   base::Time now = clock_->Now();
   base::Time last_engagement_time = GetLastEngagementTime();
   base::Time rebase_time = now - GetMaxDecayPeriod();
   base::Time new_last_engagement_time;
+
+  // If |update_last_engagement_time| is true, we must have either:
+  //   a) last_engagement_time is in the future; OR
+  //   b) last_engagement_time < rebase_time < now
+  DCHECK(!update_last_engagement_time || last_engagement_time >= now ||
+         (last_engagement_time < rebase_time && rebase_time < now));
+
+  // Cap |last_engagement_time| at |now| if it is in the future. This ensures
+  // that we use sane offsets when a user has adjusted their clock backwards and
+  // have a mix of scores prior to and after |now|.
+  if (last_engagement_time > now)
+    last_engagement_time = now;
+
   for (const auto& site : *engagement_settings) {
     GURL origin(site.primary_pattern.ToString());
 
     if (origin.is_valid()) {
       SiteEngagementScore score = CreateEngagementScore(origin);
       if (update_last_engagement_time) {
-        // Work out the offset between this score's last engagement time and the
-        // last time the service recorded any engagement. Set the score's last
-        // engagement time to rebase_time - offset to preserve its state,
-        // relative to the rebase date. This ensures that the score will decay
-        // the next time it is used, but will not decay too much.
-        DCHECK_LE(score.last_engagement_time(), rebase_time);
-        base::TimeDelta offset =
-            last_engagement_time - score.last_engagement_time();
-        base::Time rebase_score_time = rebase_time - offset;
-        score.set_last_engagement_time(rebase_score_time);
-        if (rebase_score_time > new_last_engagement_time)
-          new_last_engagement_time = rebase_score_time;
+        // Catch cases of users moving their clocks, or a potential race where
+        // a score content setting is written out to prefs, but the updated
+        // |last_engagement_time| was not written, as both are lossy
+        // preferences. |rebase_time| is strictly in the past, so any score with
+        // a last updated time in the future is caught by this branch.
+        if (score.last_engagement_time() > rebase_time) {
+          score.set_last_engagement_time(now);
+        } else if (score.last_engagement_time() > last_engagement_time) {
+          // This score is newer than |last_engagement_time|, but older than
+          // |rebase_time|. It should still be rebased with no offset as we
+          // don't accurately know what the offset should be.
+          score.set_last_engagement_time(rebase_time);
+        } else {
+          // Work out the offset between this score's last engagement time and
+          // the last time the service recorded any engagement. Set the score's
+          // last engagement time to rebase_time - offset to preserve its state,
+          // relative to the rebase date. This ensures that the score will decay
+          // the next time it is used, but will not decay too much.
+          base::TimeDelta offset =
+              last_engagement_time - score.last_engagement_time();
+          base::Time rebase_score_time = rebase_time - offset;
+          score.set_last_engagement_time(rebase_score_time);
+        }
 
+        if (score.last_engagement_time() > new_last_engagement_time)
+          new_last_engagement_time = score.last_engagement_time();
         score.Commit();
       }
 
       if (score.GetScore() > SiteEngagementScore::GetScoreCleanupThreshold())
         continue;
     }
 
     // This origin has a score of 0. Wipe it from content settings.
     settings_map->SetWebsiteSettingDefaultScope(
         origin, GURL(), CONTENT_SETTINGS_TYPE_SITE_ENGAGEMENT, std::string(),
@@ skipping 125 matching lines @@
   SiteEngagementMetrics::RecordEngagement(type);
   AddPoints(url, SiteEngagementScore::GetUserInputPoints());
 
   RecordMetrics();
   FOR_EACH_OBSERVER(
       SiteEngagementObserver, observer_list_,
       OnEngagementIncreased(web_contents, url, GetScore(url)));
 }
 
 bool SiteEngagementService::IsLastEngagementStale() const {
-  // This only happens when Chrome is first run and the user has never recorded
-  // any engagement.
+  // Only happens on first run when no engagement has ever been recorded.
   base::Time last_engagement_time = GetLastEngagementTime();
   if (last_engagement_time.is_null())
     return false;
 
-  return (clock_->Now() - last_engagement_time) >= GetStalePeriod();
+  // Stale is either too *far* back, or any amount *forward* in time. This could
+  // occur due to a changed clock, or extended non-use of the browser.
+  return (clock_->Now() - last_engagement_time) >= GetStalePeriod() ||
+         (clock_->Now() < last_engagement_time);
 }
 
 void SiteEngagementService::OnURLsDeleted(
     history::HistoryService* history_service,
     bool all_history,
     bool expired,
     const history::URLRows& deleted_rows,
     const std::set<GURL>& favicon_urls) {
   std::multiset<GURL> origins;
   for (const history::URLRow& row : deleted_rows)
@@ skipping 109 matching lines @@
     if (!engagement_score.last_shortcut_launch_time().is_null() &&
         engagement_score.last_shortcut_launch_time() > last_visit) {
       engagement_score.set_last_shortcut_launch_time(last_visit);
     }
 
     engagement_score.Commit();
   }
 
   SetLastEngagementTime(now);
 }