Commit 709246a0 authored by dominickn's avatar dominickn Committed by Commit bot

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

The site engagement service decays origins which have not been accessed
in a week. However, if a user isn't using Chrome at all, the decay still
occurs.

This CL addresses this issue by persisting the last recorded time that
any engagement occurred. If too long a period passes (currently 10 days)
without *any* engagement, all scores have their last accessed time
adjusted to be relative to some number of decay periods prior to now
(currently set at 1 decay period), based on their offset to the last known
engagement time.

BUG=575504

Review-Url: https://codereview.chromium.org/2082953002
Cr-Commit-Position: refs/heads/master@{#402653}
parent e6ea446f
...@@ -42,6 +42,8 @@ const char* kVariationNames[] = { ...@@ -42,6 +42,8 @@ const char* kVariationNames[] = {
"first_daily_engagement_points", "first_daily_engagement_points",
"medium_engagement_boundary", "medium_engagement_boundary",
"high_engagement_boundary", "high_engagement_boundary",
"max_decays_per_score",
"last_engagement_grace_period_in_hours",
}; };
bool DoublesConsideredDifferent(double value1, double value2, double delta) { bool DoublesConsideredDifferent(double value1, double value2, double delta) {
...@@ -83,6 +85,8 @@ double SiteEngagementScore::param_values[] = { ...@@ -83,6 +85,8 @@ double SiteEngagementScore::param_values[] = {
8, // BOOTSTRAP_POINTS 8, // BOOTSTRAP_POINTS
5, // MEDIUM_ENGAGEMENT_BOUNDARY 5, // MEDIUM_ENGAGEMENT_BOUNDARY
50, // HIGH_ENGAGEMENT_BOUNDARY 50, // HIGH_ENGAGEMENT_BOUNDARY
1, // MAX_DECAYS_PER_SCORE
72, // LAST_ENGAGEMENT_GRACE_PERIOD_IN_HOURS
}; };
const char* SiteEngagementScore::kRawScoreKey = "rawScore"; const char* SiteEngagementScore::kRawScoreKey = "rawScore";
...@@ -139,6 +143,14 @@ double SiteEngagementScore::GetHighEngagementBoundary() { ...@@ -139,6 +143,14 @@ double SiteEngagementScore::GetHighEngagementBoundary() {
return param_values[HIGH_ENGAGEMENT_BOUNDARY]; return param_values[HIGH_ENGAGEMENT_BOUNDARY];
} }
double SiteEngagementScore::GetMaxDecaysPerScore() {
return param_values[MAX_DECAYS_PER_SCORE];
}
double SiteEngagementScore::GetLastEngagementGracePeriodInHours() {
return param_values[LAST_ENGAGEMENT_GRACE_PERIOD_IN_HOURS];
}
// static // static
void SiteEngagementScore::UpdateFromVariations(const char* param_name) { void SiteEngagementScore::UpdateFromVariations(const char* param_name) {
double param_vals[MAX_VARIATION]; double param_vals[MAX_VARIATION];
...@@ -337,6 +349,8 @@ void SiteEngagementScore::SetParamValuesForTesting() { ...@@ -337,6 +349,8 @@ void SiteEngagementScore::SetParamValuesForTesting() {
param_values[BOOTSTRAP_POINTS] = 8; param_values[BOOTSTRAP_POINTS] = 8;
param_values[MEDIUM_ENGAGEMENT_BOUNDARY] = 5; param_values[MEDIUM_ENGAGEMENT_BOUNDARY] = 5;
param_values[HIGH_ENGAGEMENT_BOUNDARY] = 50; param_values[HIGH_ENGAGEMENT_BOUNDARY] = 50;
param_values[MAX_DECAYS_PER_SCORE] = 1;
param_values[LAST_ENGAGEMENT_GRACE_PERIOD_IN_HOURS] = 72;
// This is set to zero to avoid interference with tests and is set when // This is set to zero to avoid interference with tests and is set when
// testing this functionality. // testing this functionality.
......
...@@ -62,6 +62,18 @@ class SiteEngagementScore { ...@@ -62,6 +62,18 @@ class SiteEngagementScore {
MEDIUM_ENGAGEMENT_BOUNDARY, MEDIUM_ENGAGEMENT_BOUNDARY,
HIGH_ENGAGEMENT_BOUNDARY, HIGH_ENGAGEMENT_BOUNDARY,
// The maximum number of decays that a SiteEngagementScore can incur before
// entering a grace period. MAX_DECAYS_PER_SCORE * DECAY_PERIOD_IN_DAYS is
// the max decay period, i.e. the maximum duration permitted for
// (clock_->Now() - score.last_engagement_time()).
MAX_DECAYS_PER_SCORE,
// If a SiteEngagamentScore has not been accessed or updated for a period
// longer than the max decay period + LAST_ENGAGEMENT_GRACE_PERIOD_IN_HOURS
// (see above), its last engagement time will be reset to be max decay
// period prior to clock_->Now().
LAST_ENGAGEMENT_GRACE_PERIOD_IN_HOURS,
MAX_VARIATION MAX_VARIATION
}; };
...@@ -80,6 +92,8 @@ class SiteEngagementScore { ...@@ -80,6 +92,8 @@ class SiteEngagementScore {
static double GetBootstrapPoints(); static double GetBootstrapPoints();
static double GetMediumEngagementBoundary(); static double GetMediumEngagementBoundary();
static double GetHighEngagementBoundary(); static double GetHighEngagementBoundary();
static double GetMaxDecaysPerScore();
static double GetLastEngagementGracePeriodInHours();
// Update the default engagement settings via variations. // Update the default engagement settings via variations.
static void UpdateFromVariations(const char* param_name); static void UpdateFromVariations(const char* param_name);
...@@ -108,10 +122,7 @@ class SiteEngagementScore { ...@@ -108,10 +122,7 @@ class SiteEngagementScore {
bool MaxPointsPerDayAdded() const; bool MaxPointsPerDayAdded() const;
// Resets the score to |points| and resets the daily point limit. If // Resets the score to |points| and resets the daily point limit. If
// |updated_time| is non-null, sets the last engagement time and last // |updated_time| is non-null, sets the last engagement time to that value.
// shortcut launch time (if it is non-null) to |updated_time|. Otherwise, last
// engagement time is set to the current time and last shortcut launch time is
// left unchanged.
void Reset(double points, const base::Time updated_time); void Reset(double points, const base::Time updated_time);
// Get/set the last time this origin was launched from an installed shortcut. // Get/set the last time this origin was launched from an installed shortcut.
...@@ -122,6 +133,14 @@ class SiteEngagementScore { ...@@ -122,6 +133,14 @@ class SiteEngagementScore {
last_shortcut_launch_time_ = time; last_shortcut_launch_time_ = time;
} }
// Get/set the last time this origin recorded an engagement change.
base::Time last_engagement_time() const {
return last_engagement_time_;
}
void set_last_engagement_time(const base::Time& time) {
last_engagement_time_ = time;
}
private: private:
FRIEND_TEST_ALL_PREFIXES(SiteEngagementScoreTest, PartiallyEmptyDictionary); FRIEND_TEST_ALL_PREFIXES(SiteEngagementScoreTest, PartiallyEmptyDictionary);
FRIEND_TEST_ALL_PREFIXES(SiteEngagementScoreTest, PopulatedDictionary); FRIEND_TEST_ALL_PREFIXES(SiteEngagementScoreTest, PopulatedDictionary);
......
...@@ -25,10 +25,13 @@ ...@@ -25,10 +25,13 @@
#include "chrome/browser/engagement/site_engagement_score.h" #include "chrome/browser/engagement/site_engagement_score.h"
#include "chrome/browser/engagement/site_engagement_service_factory.h" #include "chrome/browser/engagement/site_engagement_service_factory.h"
#include "chrome/browser/history/history_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/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/browser/host_content_settings_map.h"
#include "components/content_settings/core/common/content_settings_pattern.h" #include "components/content_settings/core/common/content_settings_pattern.h"
#include "components/history/core/browser/history_service.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/browser_thread.h"
#include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents.h"
#include "url/gurl.h" #include "url/gurl.h"
...@@ -221,6 +224,11 @@ void SiteEngagementService::SetLastShortcutLaunchTime(const GURL& url) { ...@@ -221,6 +224,11 @@ void SiteEngagementService::SetLastShortcutLaunchTime(const GURL& url) {
} }
double SiteEngagementService::GetScore(const GURL& url) const { double SiteEngagementService::GetScore(const GURL& url) const {
// Ensure that if engagement is stale, we clean things up before fetching the
// score.
if (IsLastEngagementStale())
CleanupEngagementScores(true);
return CreateEngagementScore(url).GetScore(); return CreateEngagementScore(url).GetScore();
} }
...@@ -245,32 +253,81 @@ SiteEngagementService::SiteEngagementService(Profile* profile, ...@@ -245,32 +253,81 @@ SiteEngagementService::SiteEngagementService(Profile* profile,
} }
void SiteEngagementService::AddPoints(const GURL& url, double points) { 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.
if (IsLastEngagementStale())
CleanupEngagementScores(true);
SiteEngagementScore score = CreateEngagementScore(url);
score.AddPoints(points); score.AddPoints(points);
score.Commit(); score.Commit();
SetLastEngagementTime(score.last_engagement_time());
} }
void SiteEngagementService::AfterStartupTask() { 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(IsLastEngagementStale());
RecordMetrics(); RecordMetrics();
} }
void SiteEngagementService::CleanupEngagementScores() { 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 = HostContentSettingsMap* settings_map =
HostContentSettingsMapFactory::GetForProfile(profile_); HostContentSettingsMapFactory::GetForProfile(profile_);
std::unique_ptr<ContentSettingsForOneType> engagement_settings = std::unique_ptr<ContentSettingsForOneType> engagement_settings =
GetEngagementContentSettings(settings_map); 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;
for (const auto& site : *engagement_settings) { for (const auto& site : *engagement_settings) {
GURL origin(site.primary_pattern.ToString()); GURL origin(site.primary_pattern.ToString());
if (origin.is_valid() && GetScore(origin) != 0)
continue;
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;
score.Commit();
}
if (score.GetScore() != 0)
continue;
}
// This origin has a score of 0. Wipe it from content settings.
settings_map->SetWebsiteSettingDefaultScope( settings_map->SetWebsiteSettingDefaultScope(
origin, GURL(), CONTENT_SETTINGS_TYPE_SITE_ENGAGEMENT, std::string(), origin, GURL(), CONTENT_SETTINGS_TYPE_SITE_ENGAGEMENT, std::string(),
nullptr); nullptr);
} }
// Set the last engagement time to be consistent with the scores. This will
// only occur if |update_last_engagement_time| is true.
if (!new_last_engagement_time.is_null())
SetLastEngagementTime(new_last_engagement_time);
} }
void SiteEngagementService::RecordMetrics() { void SiteEngagementService::RecordMetrics() {
...@@ -307,6 +364,29 @@ void SiteEngagementService::RecordMetrics() { ...@@ -307,6 +364,29 @@ void SiteEngagementService::RecordMetrics() {
} }
} }
base::Time SiteEngagementService::GetLastEngagementTime() const {
return base::Time::FromInternalValue(
profile_->GetPrefs()->GetInt64(prefs::kSiteEngagementLastUpdateTime));
}
void SiteEngagementService::SetLastEngagementTime(
base::Time last_engagement_time) const {
profile_->GetPrefs()->SetInt64(prefs::kSiteEngagementLastUpdateTime,
last_engagement_time.ToInternalValue());
}
base::TimeDelta SiteEngagementService::GetMaxDecayPeriod() const {
return base::TimeDelta::FromDays(
SiteEngagementScore::GetDecayPeriodInDays()) *
SiteEngagementScore::GetMaxDecaysPerScore();
}
base::TimeDelta SiteEngagementService::GetStalePeriod() const {
return GetMaxDecayPeriod() +
base::TimeDelta::FromHours(
SiteEngagementScore::GetLastEngagementGracePeriodInHours());
}
double SiteEngagementService::GetMedianEngagement( double SiteEngagementService::GetMedianEngagement(
const std::map<GURL, double>& score_map) const { const std::map<GURL, double>& score_map) const {
if (score_map.size() == 0) if (score_map.size() == 0)
...@@ -372,6 +452,16 @@ void SiteEngagementService::HandleUserInput( ...@@ -372,6 +452,16 @@ void SiteEngagementService::HandleUserInput(
OnEngagementIncreased(web_contents, url, GetScore(url))); 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.
base::Time last_engagement_time = GetLastEngagementTime();
if (last_engagement_time.is_null())
return false;
return (clock_->Now() - last_engagement_time) >= GetStalePeriod();
}
void SiteEngagementService::OnURLsDeleted( void SiteEngagementService::OnURLsDeleted(
history::HistoryService* history_service, history::HistoryService* history_service,
bool all_history, bool all_history,
...@@ -391,15 +481,8 @@ void SiteEngagementService::OnURLsDeleted( ...@@ -391,15 +481,8 @@ void SiteEngagementService::OnURLsDeleted(
weak_factory_.GetWeakPtr(), hs, origins, expired)); weak_factory_.GetWeakPtr(), hs, origins, expired));
} }
const SiteEngagementScore SiteEngagementService::CreateEngagementScore(
const GURL& origin) const {
return SiteEngagementScore(
clock_.get(), origin,
HostContentSettingsMapFactory::GetForProfile(profile_));
}
SiteEngagementScore SiteEngagementService::CreateEngagementScore( SiteEngagementScore SiteEngagementService::CreateEngagementScore(
const GURL& origin) { const GURL& origin) const {
return SiteEngagementScore( return SiteEngagementScore(
clock_.get(), origin, clock_.get(), origin,
HostContentSettingsMapFactory::GetForProfile(profile_)); HostContentSettingsMapFactory::GetForProfile(profile_));
...@@ -503,4 +586,6 @@ void SiteEngagementService::GetCountsAndLastVisitForOriginsComplete( ...@@ -503,4 +586,6 @@ void SiteEngagementService::GetCountsAndLastVisitForOriginsComplete(
engagement_score.Commit(); engagement_score.Commit();
} }
SetLastEngagementTime(now);
} }
...@@ -130,6 +130,7 @@ class SiteEngagementService : public KeyedService, ...@@ -130,6 +130,7 @@ class SiteEngagementService : public KeyedService,
FRIEND_TEST_ALL_PREFIXES(SiteEngagementServiceTest, EngagementLevel); FRIEND_TEST_ALL_PREFIXES(SiteEngagementServiceTest, EngagementLevel);
FRIEND_TEST_ALL_PREFIXES(SiteEngagementServiceTest, Observers); FRIEND_TEST_ALL_PREFIXES(SiteEngagementServiceTest, Observers);
FRIEND_TEST_ALL_PREFIXES(SiteEngagementServiceTest, ScoreDecayHistograms); FRIEND_TEST_ALL_PREFIXES(SiteEngagementServiceTest, ScoreDecayHistograms);
FRIEND_TEST_ALL_PREFIXES(SiteEngagementServiceTest, LastEngagementTime);
FRIEND_TEST_ALL_PREFIXES(AppBannerSettingsHelperTest, SiteEngagementTrigger); FRIEND_TEST_ALL_PREFIXES(AppBannerSettingsHelperTest, SiteEngagementTrigger);
// Only used in tests. // Only used in tests.
...@@ -140,15 +141,34 @@ class SiteEngagementService : public KeyedService, ...@@ -140,15 +141,34 @@ class SiteEngagementService : public KeyedService,
void AddPoints(const GURL& url, double points); void AddPoints(const GURL& url, double points);
// Retrieves the SiteEngagementScore object for |origin|. // Retrieves the SiteEngagementScore object for |origin|.
SiteEngagementScore CreateEngagementScore(const GURL& origin); SiteEngagementScore CreateEngagementScore(const GURL& origin) const;
const SiteEngagementScore CreateEngagementScore(const GURL& origin) const;
// Post startup tasks: cleaning up origins which have decayed to 0, and // Runs site engagement maintenance tasks.
// logging UMA statistics.
void AfterStartupTask(); void AfterStartupTask();
void CleanupEngagementScores();
// Removes any origins which have decayed to 0 engagement. If
// |update_last_engagement_time| is true, the last engagement time of all
// origins is reset by calculating the delta between the last engagement event
// recorded by the site engagement service and the origin. The origin's last
// engagement time is then set to clock_->Now() - delta.
//
// If a user does not use the browser at all for some period of time,
// engagement is not decayed, and the state is restored equivalent to how they
// left it once they return.
void CleanupEngagementScores(bool update_last_engagement_time) const;
// Records UMA metrics.
void RecordMetrics(); void RecordMetrics();
// Get and set the last engagement time from prefs.
base::Time GetLastEngagementTime() const;
void SetLastEngagementTime(base::Time last_engagement_time) const;
// Get the maximum decay period and the stale period for last engagement
// times.
base::TimeDelta GetMaxDecayPeriod() const;
base::TimeDelta GetStalePeriod() const;
// Returns the median engagement score of all recorded origins. // Returns the median engagement score of all recorded origins.
double GetMedianEngagement(const std::map<GURL, double>& score_map) const; double GetMedianEngagement(const std::map<GURL, double>& score_map) const;
...@@ -167,6 +187,12 @@ class SiteEngagementService : public KeyedService, ...@@ -167,6 +187,12 @@ class SiteEngagementService : public KeyedService,
void HandleUserInput(content::WebContents* web_contents, void HandleUserInput(content::WebContents* web_contents,
SiteEngagementMetrics::EngagementType type); SiteEngagementMetrics::EngagementType type);
// Returns true if the last engagement increasing event seen by the site
// engagement service was sufficiently long ago that we need to reset all
// scores to be relative to now. This ensures that users who do not use the
// browser for an extended period of time do not have their engagement decay.
bool IsLastEngagementStale() const;
// Overridden from history::HistoryServiceObserver: // Overridden from history::HistoryServiceObserver:
void OnURLsDeleted(history::HistoryService* history_service, void OnURLsDeleted(history::HistoryService* history_service,
bool all_history, bool all_history,
......
...@@ -87,6 +87,8 @@ void Profile::RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) { ...@@ -87,6 +87,8 @@ void Profile::RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {
#endif #endif
registry->RegisterBooleanPref(prefs::kSessionExitedCleanly, true); registry->RegisterBooleanPref(prefs::kSessionExitedCleanly, true);
registry->RegisterStringPref(prefs::kSessionExitType, std::string()); registry->RegisterStringPref(prefs::kSessionExitType, std::string());
registry->RegisterInt64Pref(prefs::kSiteEngagementLastUpdateTime, 0,
PrefRegistry::LOSSY_PREF);
registry->RegisterBooleanPref( registry->RegisterBooleanPref(
prefs::kSafeBrowsingEnabled, prefs::kSafeBrowsingEnabled,
true, true,
......
...@@ -78,6 +78,12 @@ const char kSessionExitedCleanly[] = "profile.exited_cleanly"; ...@@ -78,6 +78,12 @@ const char kSessionExitedCleanly[] = "profile.exited_cleanly";
// shutdown. Used to determine the exit type the last time the profile was open. // shutdown. Used to determine the exit type the last time the profile was open.
const char kSessionExitType[] = "profile.exit_type"; const char kSessionExitType[] = "profile.exit_type";
// The last time that the site engagement service recorded an engagement event
// for this profile for any URL. Recorded only during shutdown. Used to prevent
// the service from decaying engagement when a user does not use Chrome at all
// for an extended period of time.
const char kSiteEngagementLastUpdateTime[] = "profile.last_engagement_time";
// An integer pref. Holds one of several values: // An integer pref. Holds one of several values:
// 0: unused, previously indicated to open the homepage on startup // 0: unused, previously indicated to open the homepage on startup
// 1: restore the last session. // 1: restore the last session.
......
...@@ -35,6 +35,7 @@ extern const char kProfileIconVersion[]; ...@@ -35,6 +35,7 @@ extern const char kProfileIconVersion[];
extern const char kRestoreOnStartup[]; extern const char kRestoreOnStartup[];
extern const char kSessionExitedCleanly[]; extern const char kSessionExitedCleanly[];
extern const char kSessionExitType[]; extern const char kSessionExitType[];
extern const char kSiteEngagementLastUpdateTime[];
extern const char kSupervisedUserApprovedExtensions[]; extern const char kSupervisedUserApprovedExtensions[];
extern const char kSupervisedUserCustodianEmail[]; extern const char kSupervisedUserCustodianEmail[];
extern const char kSupervisedUserCustodianName[]; extern const char kSupervisedUserCustodianName[];
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment