Prevent site engagement scores from decaying when Chrome isn't in use.

chrome/browser/engagement/site_engagement_service.cc

Index: chrome/browser/engagement/site_engagement_service.cc
diff --git a/chrome/browser/engagement/site_engagement_service.cc b/chrome/browser/engagement/site_engagement_service.cc
index 3780227d8785054211d8ef993c99ce054c036b3b..53519c53ed3e30fc4078597da82d1b353cabc4ad 100644
--- a/chrome/browser/engagement/site_engagement_service.cc
+++ b/chrome/browser/engagement/site_engagement_service.cc
@@ -25,10 +25,13 @@
 #include "chrome/browser/engagement/site_engagement_score.h"
 #include "chrome/browser/engagement/site_engagement_service_factory.h"
 #include "chrome/browser/history/history_service_factory.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_switches.h"
+#include "chrome/common/pref_names.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/content_settings/core/common/content_settings_pattern.h"
 #include "components/history/core/browser/history_service.h"
+#include "components/prefs/pref_service.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/web_contents.h"
 #include "url/gurl.h"
@@ -118,6 +121,10 @@ SiteEngagementService::SiteEngagementService(Profile* profile)
 }
 
 SiteEngagementService::~SiteEngagementService() {
+  // Persist the last engagement time to disk.
+  profile_->GetPrefs()->SetInt64(prefs::kSiteEngagementLastUpdateTime,
+                                 last_engagement_time_.ToInternalValue());
+
   history::HistoryService* history = HistoryServiceFactory::GetForProfile(
       profile_, ServiceAccessType::IMPLICIT_ACCESS);
   if (history)
@@ -230,6 +237,10 @@ double SiteEngagementService::GetTotalEngagementPoints() const {
 SiteEngagementService::SiteEngagementService(Profile* profile,
                                              std::unique_ptr<base::Clock> clock)
     : profile_(profile), clock_(std::move(clock)), weak_factory_(this) {
+  // Read in the last engagement time (or a null time on first run) from disk.
+  last_engagement_time_ = base::Time::FromInternalValue(
+      profile->GetPrefs()->GetInt64(prefs::kSiteEngagementLastUpdateTime));
+
   // May be null in tests.
   history::HistoryService* history = HistoryServiceFactory::GetForProfile(
       profile, ServiceAccessType::IMPLICIT_ACCESS);
@@ -238,32 +249,74 @@ SiteEngagementService::SiteEngagementService(Profile* profile,
 }
 
 void SiteEngagementService::AddPoints(const GURL& url, double points) {
-  SiteEngagementScore score = CreateEngagementScore(url);
+  // Trigger a cleanup and date adjustment if it has been a substantial length
+  // of time since *any* engagement was recorded by the service. This will
+  // ensure that we do not decay scores when the user did not use the browser.
+  // IsLastEngagementObsolete is tested separately so that we do not run cleanup
+  // in this method unless absolutely necessary.
+  if (IsLastEngagementObsolete())
+    CleanupEngagementScores(true);
 
+  SiteEngagementScore score = CreateEngagementScore(url);
   score.AddPoints(points);
   score.Commit();
+
+  last_engagement_time_ = score.last_engagement_time();
 }
 
 void SiteEngagementService::AfterStartupTask() {
-  CleanupEngagementScores();
+  // 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(IsLastEngagementObsolete());
   RecordMetrics();
 }
 
-void SiteEngagementService::CleanupEngagementScores() {
+void SiteEngagementService::CleanupEngagementScores(

[calamity, 2016/06/23 05:18:40]

It might be worth renaming this method now since it does 2 things.

[dominickn, 2016/06/27 02:53:35]

I think Cleanup is still the right name for the method - rebasing times is part
of cleaning up.

+    bool update_last_engagement_time) {
+  // This method should not be called with |update_last_engagement_time| = true
+  // if the last engagement time isn't obsolete.
+  DCHECK(!update_last_engagement_time || IsLastEngagementObsolete());
+
   HostContentSettingsMap* settings_map =
     HostContentSettingsMapFactory::GetForProfile(profile_);
   std::unique_ptr<ContentSettingsForOneType> engagement_settings =
       GetEngagementContentSettings(settings_map);
 
+  base::Time now = clock_->Now();
   for (const auto& site : *engagement_settings) {
     GURL origin(site.primary_pattern.ToString());
-    if (origin.is_valid() && GetScore(origin) != 0)
+    SiteEngagementScore score = CreateEngagementScore(origin);
+
+    if (origin.is_valid()) {
+      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 now - offset to preserve its state, but relative
+        // to now.
+        base::TimeDelta offset =
+            last_engagement_time_ - score.last_engagement_time();
+        score.set_last_engagement_time(now - offset);

[calamity, 2016/06/23 05:18:40]

I think that all scores should be rebased to the obsoletion time limit. This
will mean that all sites take a hit equal to the obsoletion time limit which
seems more continuous than '9 days, lose your engagement' but '10 days, keep it
all'.

[dominickn, 2016/06/27 02:53:35]

Done.

+      }
+
+      if (score.GetScore() != 0) {
+        score.Commit();
         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(),
         nullptr);
   }
+
+  // Set the last engagement time to now for consistency and to ensure we do not
+  // double-reset. Persist the time to disk.
+  last_engagement_time_ = now;
+  profile_->GetPrefs()->SetInt64(prefs::kSiteEngagementLastUpdateTime,
+                                last_engagement_time_.ToInternalValue());
 }
 
 void SiteEngagementService::RecordMetrics() {
@@ -299,6 +352,19 @@ void SiteEngagementService::RecordMetrics() {
   }
 }
 
+bool SiteEngagementService::IsLastEngagementObsolete() {
+  // This only happens when Chrome is first run and the user has never recorded
+  // any engagement.
+  if (last_engagement_time_.is_null())
+    return false;
+
+  // Last engagement is obsolete when more than
+  // OBSOLETE_LAST_ENGAGEMENT_PERIOD_IN_HOURS have passed.
+  return (clock_->Now() - last_engagement_time_) >=
+         base::TimeDelta::FromHours(
+             SiteEngagementScore::GetObsoleteLastEngagementPeriodInHours());
+}
+
 double SiteEngagementService::GetMedianEngagement(
     const std::map<GURL, double>& score_map) const {
   if (score_map.size() == 0)