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 3d88eaba661ebfea12a1b66a50108843de37db0b..b7c73ee2290975ca63276b5152f31a79e0c74df1 100644 |
--- a/chrome/browser/engagement/site_engagement_service.cc |
+++ b/chrome/browser/engagement/site_engagement_service.cc |
@@ -51,24 +51,6 @@ std::unique_ptr<ContentSettingsForOneType> GetEngagementContentSettings( |
return engagement_settings; |
} |
-std::unique_ptr<base::DictionaryValue> GetScoreDictForOrigin( |
- HostContentSettingsMap* settings, |
- const GURL& origin_url) { |
- if (!settings) |
- return std::unique_ptr<base::DictionaryValue>(); |
- |
- std::unique_ptr<base::Value> value = settings->GetWebsiteSetting( |
- origin_url, origin_url, CONTENT_SETTINGS_TYPE_SITE_ENGAGEMENT, |
- std::string(), NULL); |
- if (!value.get()) |
- return base::WrapUnique(new base::DictionaryValue()); |
- |
- if (!value->IsType(base::Value::TYPE_DICTIONARY)) |
- return base::WrapUnique(new base::DictionaryValue()); |
- |
- return base::WrapUnique(static_cast<base::DictionaryValue*>(value.release())); |
-} |
- |
// Only accept a navigation event for engagement if it is one of: |
// a. direct typed navigation |
// b. clicking on an omnibox suggestion brought up by typing a keyword |
@@ -173,16 +155,13 @@ std::map<GURL, double> SiteEngagementService::GetScoreMap() const { |
if (!origin.is_valid()) |
continue; |
- std::unique_ptr<base::DictionaryValue> score_dict = |
- GetScoreDictForOrigin(settings_map, origin); |
- SiteEngagementScore score(clock_.get(), *score_dict); |
- score_map[origin] = score.GetScore(); |
+ score_map[origin] = GetScore(origin); |
} |
return score_map; |
} |
-bool SiteEngagementService::IsBootstrapped() { |
+bool SiteEngagementService::IsBootstrapped() const { |
return GetTotalEngagementPoints() >= |
SiteEngagementScore::GetBootstrapPoints(); |
} |
@@ -210,15 +189,13 @@ bool SiteEngagementService::IsEngagementAtLeast( |
} |
void SiteEngagementService::ResetScoreForURL(const GURL& url, double score) { |
- ResetScoreAndAccessTimesForURL(url, score, nullptr); |
+ SiteEngagementScore engagement_score = CreateEngagementScore(url); |
+ engagement_score.Reset(score, clock_->Now()); |
+ engagement_score.Commit(); |
} |
void SiteEngagementService::SetLastShortcutLaunchTime(const GURL& url) { |
- HostContentSettingsMap* settings_map = |
- HostContentSettingsMapFactory::GetForProfile(profile_); |
- std::unique_ptr<base::DictionaryValue> score_dict = |
- GetScoreDictForOrigin(settings_map, url); |
- SiteEngagementScore score(clock_.get(), *score_dict); |
+ SiteEngagementScore score = CreateEngagementScore(url); |
// Record the number of days since the last launch in UMA. If the user's clock |
// has changed back in time, set this to 0. |
@@ -232,21 +209,11 @@ void SiteEngagementService::SetLastShortcutLaunchTime(const GURL& url) { |
SiteEngagementMetrics::ENGAGEMENT_WEBAPP_SHORTCUT_LAUNCH); |
score.set_last_shortcut_launch_time(now); |
- if (score.UpdateScoreDict(score_dict.get())) { |
- settings_map->SetWebsiteSettingDefaultScope( |
- url, GURL(), CONTENT_SETTINGS_TYPE_SITE_ENGAGEMENT, std::string(), |
- score_dict.release()); |
- } |
+ score.Commit(); |
} |
double SiteEngagementService::GetScore(const GURL& url) const { |
- HostContentSettingsMap* settings_map = |
- HostContentSettingsMapFactory::GetForProfile(profile_); |
- std::unique_ptr<base::DictionaryValue> score_dict = |
- GetScoreDictForOrigin(settings_map, url); |
- SiteEngagementScore score(clock_.get(), *score_dict); |
- |
- return score.GetScore(); |
+ return CreateEngagementScore(url).GetScore(); |
} |
double SiteEngagementService::GetTotalEngagementPoints() const { |
@@ -270,18 +237,10 @@ SiteEngagementService::SiteEngagementService(Profile* profile, |
} |
void SiteEngagementService::AddPoints(const GURL& url, double points) { |
- HostContentSettingsMap* settings_map = |
- HostContentSettingsMapFactory::GetForProfile(profile_); |
- std::unique_ptr<base::DictionaryValue> score_dict = |
- GetScoreDictForOrigin(settings_map, url); |
- SiteEngagementScore score(clock_.get(), *score_dict); |
+ SiteEngagementScore score = CreateEngagementScore(url); |
score.AddPoints(points); |
- if (score.UpdateScoreDict(score_dict.get())) { |
- settings_map->SetWebsiteSettingDefaultScope( |
- url, GURL(), CONTENT_SETTINGS_TYPE_SITE_ENGAGEMENT, std::string(), |
- score_dict.release()); |
- } |
+ score.Commit(); |
} |
void SiteEngagementService::AfterStartupTask() { |
@@ -298,10 +257,7 @@ void SiteEngagementService::CleanupEngagementScores() { |
for (const auto& site : *engagement_settings) { |
GURL origin(site.primary_pattern.ToString()); |
if (origin.is_valid()) { |
dominickn
2016/05/26 09:33:20
Strictly speaking this could be one if statement,
calamity
2016/05/31 05:44:15
Done.
|
- std::unique_ptr<base::DictionaryValue> score_dict = |
- GetScoreDictForOrigin(settings_map, origin); |
- SiteEngagementScore score(clock_.get(), *score_dict); |
- if (score.GetScore() != 0) |
+ if (GetScore(origin) != 0) |
continue; |
} |
@@ -412,6 +368,20 @@ void SiteEngagementService::OnURLsDeleted( |
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( |
+ const GURL& origin) { |
+ return SiteEngagementScore( |
+ clock_.get(), origin, |
+ HostContentSettingsMapFactory::GetForProfile(profile_)); |
+} |
+ |
int SiteEngagementService::OriginsWithMaxDailyEngagement() const { |
HostContentSettingsMap* settings_map = |
HostContentSettingsMapFactory::GetForProfile(profile_); |
@@ -426,10 +396,7 @@ int SiteEngagementService::OriginsWithMaxDailyEngagement() const { |
if (!origin.is_valid()) |
continue; |
- std::unique_ptr<base::DictionaryValue> score_dict = |
- GetScoreDictForOrigin(settings_map, origin); |
- SiteEngagementScore score(clock_.get(), *score_dict); |
- if (score.MaxPointsPerDayAdded()) |
+ if (CreateEngagementScore(origin).MaxPointsPerDayAdded()) |
++total_origins; |
} |
@@ -462,6 +429,11 @@ void SiteEngagementService::GetCountsAndLastVisitForOriginsComplete( |
for (const auto& origin_to_count : remaining_origins) { |
GURL origin = origin_to_count.first; |
+ // It appears that the history service occasionally sends bad URLs to us. |
+ // See crbug.com/612881. |
+ if (!origin.is_valid()) |
+ continue; |
+ |
int remaining = origin_to_count.second.first; |
base::Time last_visit = origin_to_count.second.second; |
int deleted = deleted_origins.count(origin); |
@@ -496,40 +468,17 @@ void SiteEngagementService::GetCountsAndLastVisitForOriginsComplete( |
undecay = periods * SiteEngagementScore::GetDecayPoints(); |
} |
- double score = |
- std::min(SiteEngagementScore::kMaxPoints, |
- (proportion_remaining * GetScore(origin)) + undecay); |
- ResetScoreAndAccessTimesForURL(origin, score, &last_visit); |
- } |
-} |
- |
-void SiteEngagementService::ResetScoreAndAccessTimesForURL( |
- const GURL& url, double score, const base::Time* updated_time) { |
- // It appears that the history service occassionally sends bad URLs to us. |
- // See crbug.com/612881. |
- if (!url.is_valid()) |
- return; |
- |
- DCHECK_GE(score, 0); |
- DCHECK_LE(score, SiteEngagementScore::kMaxPoints); |
+ SiteEngagementScore engagement_score = CreateEngagementScore(origin); |
- HostContentSettingsMap* settings_map = |
- HostContentSettingsMapFactory::GetForProfile(profile_); |
- std::unique_ptr<base::DictionaryValue> score_dict = |
- GetScoreDictForOrigin(settings_map, url); |
- SiteEngagementScore engagement_score(clock_.get(), *score_dict); |
- |
- engagement_score.Reset(score, updated_time); |
- if (score == 0) { |
- settings_map->SetWebsiteSettingDefaultScope( |
- url, GURL(), CONTENT_SETTINGS_TYPE_SITE_ENGAGEMENT, std::string(), |
- nullptr); |
- return; |
- } |
+ double score = std::min( |
+ SiteEngagementScore::kMaxPoints, |
+ (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); |
+ } |
- if (engagement_score.UpdateScoreDict(score_dict.get())) { |
- settings_map->SetWebsiteSettingDefaultScope( |
- url, GURL(), CONTENT_SETTINGS_TYPE_SITE_ENGAGEMENT, std::string(), |
- score_dict.release()); |
+ engagement_score.Commit(); |
} |
} |