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

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>
 #include <vector>
 
 #include "base/command_line.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/field_trial.h"
 #include "base/strings/string_util.h"
 #include "base/time/clock.h"
 #include "base/time/default_clock.h"
 #include "base/time/time.h"
 #include "base/values.h"
 #include "chrome/browser/banners/app_banner_settings_helper.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/engagement/site_engagement_eviction_policy.h"
 #include "chrome/browser/engagement/site_engagement_metrics.h"
 #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"
 
 namespace {
 
 const int FOUR_WEEKS_IN_DAYS = 28;
 
 // Global bool to ensure we only update the parameters from variations once.
 bool g_updated_from_variations = false;
@@ skipping 69 matching lines @@
       base::Bind(&SiteEngagementService::AfterStartupTask,
                  weak_factory_.GetWeakPtr()));
 
   if (!g_updated_from_variations) {
     SiteEngagementScore::UpdateFromVariations(kEngagementParams);
     g_updated_from_variations = true;
   }
 }
 
 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)
     history->RemoveObserver(this);
 }
 
 SiteEngagementService::EngagementLevel
 SiteEngagementService::GetEngagementLevel(const GURL& url) const {
   DCHECK_LT(SiteEngagementScore::GetMediumEngagementBoundary(),
             SiteEngagementScore::GetHighEngagementBoundary());
@@ skipping 92 matching lines @@
   double total_score = 0;
   for (const auto& value : score_map)
     total_score += value.second;
 
   return total_score;
 }
 
 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);
   if (history)
     history->AddObserver(this);
 }
 
 void SiteEngagementService::AddPoints(const GURL& url, double points) {
+  // 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() {
   base::Time now = clock_->Now();
   if (last_metrics_time_.is_null() ||
       (now - last_metrics_time_).InMinutes() >= kMetricsIntervalInMinutes) {
     last_metrics_time_ = now;
     std::map<GURL, double> score_map = GetScoreMap();
 
     int origins_with_max_engagement = OriginsWithMaxEngagement(score_map);
@@ skipping 15 matching lines @@
 
     SiteEngagementMetrics::RecordOriginsWithMaxDailyEngagement(
         OriginsWithMaxDailyEngagement());
     SiteEngagementMetrics::RecordOriginsWithMaxEngagement(
         origins_with_max_engagement);
     SiteEngagementMetrics::RecordPercentOriginsWithMaxEngagement(
         percent_origins_with_max_engagement);
   }
 }
 
+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)
     return 0;
 
   std::vector<double> scores;
   scores.reserve(score_map.size());
   for (const auto& value : score_map)
     scores.push_back(value.second);
 
@@ skipping 165 matching lines @@
         (proportion_remaining * engagement_score.GetScore()) + undecay);
     engagement_score.Reset(score, last_visit);
     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();
   }
 }