Commit 1c794d97 authored by Christian Dullweber's avatar Christian Dullweber Committed by Commit Bot

Remove origins with zero engagement

When urls are removed from chrome://history, the site-engagement score
is not removed if it reaches zero. This CL fixes the issue as it is not
neccessary to keep a score of zero around.

Bug: 838875
Change-Id: Ia580ee49f2c4df8cda46f33c36b6551580dafef7
Reviewed-on: https://chromium-review.googlesource.com/1046765Reviewed-by: default avatarcalamity <calamity@chromium.org>
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556740}
parent c0e58526
...@@ -641,6 +641,9 @@ void SiteEngagementService::GetCountsAndLastVisitForOriginsComplete( ...@@ -641,6 +641,9 @@ void SiteEngagementService::GetCountsAndLastVisitForOriginsComplete(
base::Time four_weeks_ago = base::Time four_weeks_ago =
now - base::TimeDelta::FromDays(FOUR_WEEKS_IN_DAYS); now - base::TimeDelta::FromDays(FOUR_WEEKS_IN_DAYS);
HostContentSettingsMap* settings_map =
HostContentSettingsMapFactory::GetForProfile(profile_);
for (const auto& origin_to_count : remaining_origins) { for (const auto& origin_to_count : remaining_origins) {
GURL origin = origin_to_count.first; GURL origin = origin_to_count.first;
// It appears that the history service occasionally sends bad URLs to us. // It appears that the history service occasionally sends bad URLs to us.
...@@ -657,6 +660,14 @@ void SiteEngagementService::GetCountsAndLastVisitForOriginsComplete( ...@@ -657,6 +660,14 @@ void SiteEngagementService::GetCountsAndLastVisitForOriginsComplete(
if ((expired && remaining != 0) || deleted == 0) if ((expired && remaining != 0) || deleted == 0)
continue; continue;
// Remove origins that have no urls left.
if (remaining == 0) {
settings_map->SetWebsiteSettingDefaultScope(
origin, GURL(), CONTENT_SETTINGS_TYPE_SITE_ENGAGEMENT,
content_settings::ResourceIdentifier(), nullptr);
continue;
}
// Remove engagement proportional to the urls expired from the origin's // Remove engagement proportional to the urls expired from the origin's
// entire history. // entire history.
double proportion_remaining = double proportion_remaining =
......
...@@ -1134,6 +1134,7 @@ TEST_F(SiteEngagementServiceTest, CleanupOriginsOnHistoryDeletion) { ...@@ -1134,6 +1134,7 @@ TEST_F(SiteEngagementServiceTest, CleanupOriginsOnHistoryDeletion) {
AssertInRange(5.0, service_->GetScore(origin2)); AssertInRange(5.0, service_->GetScore(origin2));
AssertInRange(5.0, service_->GetScore(origin3)); AssertInRange(5.0, service_->GetScore(origin3));
AssertInRange(5.0, service_->GetScore(origin4)); AssertInRange(5.0, service_->GetScore(origin4));
EXPECT_EQ(4U, service_->GetAllDetails().size());
{ {
SiteEngagementChangeWaiter waiter(profile()); SiteEngagementChangeWaiter waiter(profile());
...@@ -1154,6 +1155,7 @@ TEST_F(SiteEngagementServiceTest, CleanupOriginsOnHistoryDeletion) { ...@@ -1154,6 +1155,7 @@ TEST_F(SiteEngagementServiceTest, CleanupOriginsOnHistoryDeletion) {
AssertInRange(5.0, service_->GetScore(origin3)); AssertInRange(5.0, service_->GetScore(origin3));
AssertInRange(2.5, service_->GetScore(origin4)); AssertInRange(2.5, service_->GetScore(origin4));
AssertInRange(9.5, service_->GetTotalEngagementPoints()); AssertInRange(9.5, service_->GetTotalEngagementPoints());
EXPECT_EQ(3U, service_->GetAllDetails().size());
} }
{ {
...@@ -1177,6 +1179,7 @@ TEST_F(SiteEngagementServiceTest, CleanupOriginsOnHistoryDeletion) { ...@@ -1177,6 +1179,7 @@ TEST_F(SiteEngagementServiceTest, CleanupOriginsOnHistoryDeletion) {
AssertInRange(5.0, service_->GetScore(origin3)); AssertInRange(5.0, service_->GetScore(origin3));
AssertInRange(2.5, service_->GetScore(origin4)); AssertInRange(2.5, service_->GetScore(origin4));
AssertInRange(8.5, service_->GetTotalEngagementPoints()); AssertInRange(8.5, service_->GetTotalEngagementPoints());
EXPECT_EQ(3U, service_->GetAllDetails().size());
} }
{ {
...@@ -1199,6 +1202,7 @@ TEST_F(SiteEngagementServiceTest, CleanupOriginsOnHistoryDeletion) { ...@@ -1199,6 +1202,7 @@ TEST_F(SiteEngagementServiceTest, CleanupOriginsOnHistoryDeletion) {
AssertInRange(5.0, service_->GetScore(origin3)); AssertInRange(5.0, service_->GetScore(origin3));
AssertInRange(2.5, service_->GetScore(origin4)); AssertInRange(2.5, service_->GetScore(origin4));
AssertInRange(7.5, service_->GetTotalEngagementPoints()); AssertInRange(7.5, service_->GetTotalEngagementPoints());
EXPECT_EQ(2U, service_->GetAllDetails().size());
} }
} }
......
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