SiteSettings counter update and deletion based on period endtime.

Updated the SiteSettingsCounter to use period endtime for counting.
HostContentSettingsMap::ClearSettingsForOneTypeWithPredicate() takes end_time.

Bug: 243833
Change-Id: I0a801beb4c415c0df08aa7b9b9d0f66d4f7695f7
Reviewed-on: https://chromium-review.googlesource.com/912949Reviewed-by: Christian Dullweber <dullweber@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Reviewed-by: Adam Langley <agl@chromium.org>
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Jialiu Lin <jialiul@chromium.org>
Commit-Queue: Sarath Singapati <sarath.singapati@huawei.com>
Cr-Commit-Position: refs/heads/master@{#548375}

chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc

@@ -702,7 +702,7 @@ void ChromeBrowsingDataRemoverDelegate::RemoveEmbedderData(
     base::RecordAction(UserMetricsAction("ClearBrowsingData_Cookies"));
 
     host_content_settings_map_->ClearSettingsForOneTypeWithPredicate(
-        CONTENT_SETTINGS_TYPE_CLIENT_HINTS, base::Time(),
+        CONTENT_SETTINGS_TYPE_CLIENT_HINTS, base::Time(), base::Time::Max(),
         base::BindRepeating(&WebsiteSettingsFilterAdapter, filter));
 
     // Clear the safebrowsing cookies only if time period is for "all time".  It
@@ -752,7 +752,7 @@ void ChromeBrowsingDataRemoverDelegate::RemoveEmbedderData(
         content_settings::ContentSettingsRegistry::GetInstance();
     for (const content_settings::ContentSettingsInfo* info : *registry) {
       host_content_settings_map_->ClearSettingsForOneTypeWithPredicate(
-          info->website_settings_info()->type(), delete_begin_,
+          info->website_settings_info()->type(), delete_begin_, delete_end_,
           HostContentSettingsMap::PatternSourcePredicate());
     }
 #if !defined(OS_ANDROID)
@@ -788,7 +788,7 @@ void ChromeBrowsingDataRemoverDelegate::RemoveEmbedderData(
   // DATA_TYPE_DURABLE_PERMISSION
   if (remove_mask & DATA_TYPE_DURABLE_PERMISSION) {
     host_content_settings_map_->ClearSettingsForOneTypeWithPredicate(
-        CONTENT_SETTINGS_TYPE_DURABLE_STORAGE, base::Time(),
+        CONTENT_SETTINGS_TYPE_DURABLE_STORAGE, base::Time(), base::Time::Max(),
         base::BindRepeating(&WebsiteSettingsFilterAdapter, filter));
   }
 
@@ -798,7 +798,7 @@ void ChromeBrowsingDataRemoverDelegate::RemoveEmbedderData(
     base::RecordAction(UserMetricsAction("ClearBrowsingData_SiteUsageData"));
 
     host_content_settings_map_->ClearSettingsForOneTypeWithPredicate(
-        CONTENT_SETTINGS_TYPE_SITE_ENGAGEMENT, base::Time(),
+        CONTENT_SETTINGS_TYPE_SITE_ENGAGEMENT, base::Time(), base::Time::Max(),
         base::BindRepeating(&WebsiteSettingsFilterAdapter, filter));
 
     if (MediaEngagementService::IsEnabled()) {
@@ -810,7 +810,7 @@ void ChromeBrowsingDataRemoverDelegate::RemoveEmbedderData(
   if ((remove_mask & DATA_TYPE_SITE_USAGE_DATA) ||
       (remove_mask & DATA_TYPE_HISTORY)) {
     host_content_settings_map_->ClearSettingsForOneTypeWithPredicate(
-        CONTENT_SETTINGS_TYPE_APP_BANNER, base::Time(),
+        CONTENT_SETTINGS_TYPE_APP_BANNER, base::Time(), base::Time::Max(),
         base::BindRepeating(&WebsiteSettingsFilterAdapter, filter));
 
     PermissionDecisionAutoBlocker::GetForProfile(profile_)->RemoveCountsByUrl(
@@ -818,7 +818,7 @@ void ChromeBrowsingDataRemoverDelegate::RemoveEmbedderData(
 
 #if BUILDFLAG(ENABLE_PLUGINS)
     host_content_settings_map_->ClearSettingsForOneTypeWithPredicate(
-        CONTENT_SETTINGS_TYPE_PLUGINS_DATA, base::Time(),
+        CONTENT_SETTINGS_TYPE_PLUGINS_DATA, base::Time(), base::Time::Max(),
         base::Bind(&WebsiteSettingsFilterAdapter, filter));
 #endif
   }

chrome/browser/browsing_data/counters/site_settings_counter_unittest.cc

@@ -108,6 +108,13 @@ TEST_F(SiteSettingsCounterTest, CountWithTimePeriod) {
       GURL("http://maps.google.com"), GURL("http://maps.google.com"),
       CONTENT_SETTINGS_TYPE_GEOLOCATION, std::string(), CONTENT_SETTING_ALLOW);
 
+  // Create a setting at Now()-31days.
+  test_clock.SetNow(base::Time::Now() - base::TimeDelta::FromDays(31));
+  map()->SetContentSettingDefaultScope(GURL("http://www.google.com"),
+                                       GURL("http://www.google.com"),
+                                       CONTENT_SETTINGS_TYPE_NOTIFICATIONS,
+                                       std::string(), CONTENT_SETTING_ALLOW);
+
   test_clock.SetNow(base::Time::Now());
   browsing_data::SiteSettingsCounter counter(map(), zoom_map());
   counter.Init(
@@ -119,6 +126,9 @@ TEST_F(SiteSettingsCounterTest, CountWithTimePeriod) {
   // Both settings were created during the last day.
   SetDeletionPeriodPref(browsing_data::TimePeriod::LAST_DAY);
   EXPECT_EQ(2, GetResult());
+  // One of the settings was created 31days ago.
+  SetDeletionPeriodPref(browsing_data::TimePeriod::OLDER_THAN_30_DAYS);
+  EXPECT_EQ(1, GetResult());
 }
 
 // Tests that the counter doesn't count website settings

chrome/browser/content_settings/host_content_settings_map_unittest.cc

@@ -1517,7 +1517,7 @@ TEST_F(HostContentSettingsMapTest, ClearSettingsForOneTypeWithPredicate) {
 
   // First, test that we clear only COOKIES (not APP_BANNER), and pattern2.
   host_content_settings_map->ClearSettingsForOneTypeWithPredicate(
-      CONTENT_SETTINGS_TYPE_COOKIES, base::Time(),
+      CONTENT_SETTINGS_TYPE_COOKIES, base::Time(), base::Time::Max(),
       base::Bind(&MatchPrimaryPattern, pattern2));
   host_content_settings_map->GetSettingsForOneType(
       CONTENT_SETTINGS_TYPE_COOKIES, std::string(), &host_settings);
@@ -1562,7 +1562,7 @@ TEST_F(HostContentSettingsMapTest, ClearSettingsForOneTypeWithPredicate) {
   ContentSettingsPattern http_pattern =
       ContentSettingsPattern::FromURLNoWildcard(url3_origin_only);
   host_content_settings_map->ClearSettingsForOneTypeWithPredicate(
-      CONTENT_SETTINGS_TYPE_SITE_ENGAGEMENT, base::Time(),
+      CONTENT_SETTINGS_TYPE_SITE_ENGAGEMENT, base::Time(), base::Time::Max(),
       base::Bind(&MatchPrimaryPattern, http_pattern));
   // Verify we only have one, and it's url1.
   host_content_settings_map->GetSettingsForOneType(
@@ -1575,49 +1575,71 @@ TEST_F(HostContentSettingsMapTest, ClearSettingsForOneTypeWithPredicate) {
 TEST_F(HostContentSettingsMapTest, ClearSettingsWithTimePredicate) {
   TestingProfile profile;
   auto* map = HostContentSettingsMapFactory::GetForProfile(&profile);
+  base::Time now = base::Time::Now();
+  base::Time back_1_hour = now - base::TimeDelta::FromHours(1);
+  base::Time back_30_days = now - base::TimeDelta::FromDays(30);
+  base::Time back_31_days = now - base::TimeDelta::FromDays(31);
 
   base::SimpleTestClock test_clock;
-  test_clock.SetNow(base::Time::Now());
+  test_clock.SetNow(now);
   map->SetClockForTesting(&test_clock);
 
   ContentSettingsForOneType host_settings;
 
   GURL url1("https://www.google.com/");
   GURL url2("https://maps.google.com/");
+  GURL url3("https://photos.google.com");
 
   // Add setting for url1.
   map->SetContentSettingDefaultScope(url1, GURL(), CONTENT_SETTINGS_TYPE_POPUPS,
                                      std::string(), CONTENT_SETTING_BLOCK);
 
-  // Make sure that the timestamp for url1 is different from |t|.
-  test_clock.Advance(base::TimeDelta::FromSeconds(1));
-  base::Time t = test_clock.Now();
-
   // Add setting for url2.
+  test_clock.SetNow(back_1_hour);
   map->SetContentSettingDefaultScope(url2, GURL(), CONTENT_SETTINGS_TYPE_POPUPS,
                                      std::string(), CONTENT_SETTING_BLOCK);
 
-  // Verify we have two pattern and the default.
+  // Add setting for url3 with the timestamp of 31 days old.
+  test_clock.SetNow(back_31_days);
+  map->SetContentSettingDefaultScope(url3, GURL(), CONTENT_SETTINGS_TYPE_POPUPS,
+                                     std::string(), CONTENT_SETTING_BLOCK);
+
+  // Verify we have three pattern and the default.
+  map->GetSettingsForOneType(CONTENT_SETTINGS_TYPE_POPUPS, std::string(),
+                             &host_settings);
+  EXPECT_EQ(4u, host_settings.size());
+
+  // Clear all settings since |now|.
+  map->ClearSettingsForOneTypeWithPredicate(
+      CONTENT_SETTINGS_TYPE_POPUPS, now, base::Time::Max(),
+      HostContentSettingsMap::PatternSourcePredicate());
+
+  // Verify we have two pattern (url2, url3) and the default.
   map->GetSettingsForOneType(CONTENT_SETTINGS_TYPE_POPUPS, std::string(),
                              &host_settings);
   EXPECT_EQ(3u, host_settings.size());
+  EXPECT_EQ("https://maps.google.com:443",
+            host_settings[0].primary_pattern.ToString());
+  EXPECT_EQ("https://photos.google.com:443",
+            host_settings[1].primary_pattern.ToString());
+  EXPECT_EQ("*", host_settings[2].primary_pattern.ToString());
 
-  // Clear all settings since |t|.
+  // Clear all settings since the beginning of time to 30 days old.
   map->ClearSettingsForOneTypeWithPredicate(
-      CONTENT_SETTINGS_TYPE_POPUPS, t,
+      CONTENT_SETTINGS_TYPE_POPUPS, base::Time(), back_30_days,
       HostContentSettingsMap::PatternSourcePredicate());
 
-  // Verify we only have one pattern (url1) and the default.
+  // Verify we only have one pattern (url2) and the default.
   map->GetSettingsForOneType(CONTENT_SETTINGS_TYPE_POPUPS, std::string(),
                              &host_settings);
   EXPECT_EQ(2u, host_settings.size());
-  EXPECT_EQ("https://www.google.com:443",
+  EXPECT_EQ("https://maps.google.com:443",
             host_settings[0].primary_pattern.ToString());
   EXPECT_EQ("*", host_settings[1].primary_pattern.ToString());
 
   // Clear all settings since the beginning of time.
   map->ClearSettingsForOneTypeWithPredicate(
-      CONTENT_SETTINGS_TYPE_POPUPS, base::Time(),
+      CONTENT_SETTINGS_TYPE_POPUPS, base::Time(), base::Time::Max(),
       HostContentSettingsMap::PatternSourcePredicate());
 
   // Verify we only have the default setting.

chrome/browser/media/media_engagement_service.cc

@@ -175,6 +175,7 @@ void MediaEngagementService::ClearDataBetweenTime(
   HostContentSettingsMapFactory::GetForProfile(profile_)
       ->ClearSettingsForOneTypeWithPredicate(
           CONTENT_SETTINGS_TYPE_MEDIA_ENGAGEMENT, base::Time(),
+          base::Time::Max(),
           base::Bind(&MediaEngagementTimeFilterAdapter, this, delete_begin,
                      delete_end));
 }
@@ -286,6 +287,7 @@ void MediaEngagementService::Clear(const GURL& url) {
   HostContentSettingsMapFactory::GetForProfile(profile_)
       ->ClearSettingsForOneTypeWithPredicate(
           CONTENT_SETTINGS_TYPE_MEDIA_ENGAGEMENT, base::Time(),
+          base::Time::Max(),
           base::Bind(&MediaEngagementFilterAdapter, base::ConstRef(url)));
 }
 

chrome/browser/ssl/chrome_ssl_host_state_delegate.cc

@@ -340,7 +340,7 @@ void ChromeSSLHostStateDelegate::Clear(
   HostContentSettingsMapFactory::GetForProfile(profile_)
       ->ClearSettingsForOneTypeWithPredicate(
           CONTENT_SETTINGS_TYPE_SSL_CERT_DECISIONS, base::Time(),
-          pattern_filter);
+          base::Time::Max(), pattern_filter);
 }
 
 content::SSLHostStateDelegate::CertJudgment

components/browsing_data/content/counters/site_settings_counter.cc

@@ -39,6 +39,7 @@ void SiteSettingsCounter::Count() {
   std::set<std::string> hosts;
   int empty_host_pattern = 0;
   base::Time period_start = GetPeriodStart();
+  base::Time period_end = GetPeriodEnd();
   auto* registry = content_settings::ContentSettingsRegistry::GetInstance();
   for (const content_settings::ContentSettingsInfo* info : *registry) {
     ContentSettingsType type = info->website_settings_info()->type();
@@ -53,7 +54,7 @@ void SiteSettingsCounter::Count() {
         base::Time last_modified = map_->GetSettingLastModifiedDate(
             content_setting.primary_pattern, content_setting.secondary_pattern,
             type);
-        if (last_modified >= period_start) {
+        if (last_modified >= period_start && last_modified < period_end) {
           if (content_setting.primary_pattern.GetHost().empty())
             empty_host_pattern++;
           else
@@ -67,7 +68,8 @@ void SiteSettingsCounter::Count() {
   for (const auto& zoom_level : zoom_map_->GetAllZoomLevels()) {
     // zoom_level with non-empty scheme are only used for some internal
     // features and not stored in preferences. They are not counted.
-    if (zoom_level.last_modified >= period_start && zoom_level.scheme.empty()) {
+    if (zoom_level.last_modified >= period_start &&
+        zoom_level.last_modified < period_end && zoom_level.scheme.empty()) {
       hosts.insert(zoom_level.host);
     }
   }

components/content_settings/core/browser/host_content_settings_map.cc

@@ -629,8 +629,10 @@ base::Time HostContentSettingsMap::GetSettingLastModifiedDate(
 void HostContentSettingsMap::ClearSettingsForOneTypeWithPredicate(
     ContentSettingsType content_type,
     base::Time begin_time,
+    base::Time end_time,
     const PatternSourcePredicate& pattern_predicate) {
-  if (pattern_predicate.is_null() && begin_time.is_null()) {
+  if (pattern_predicate.is_null() && begin_time.is_null() &&
+      (end_time.is_null() || end_time.is_max())) {
     ClearSettingsForOneType(content_type);
     return;
   }
@@ -645,7 +647,8 @@ void HostContentSettingsMap::ClearSettingsForOneTypeWithPredicate(
         base::Time last_modified = provider->GetWebsiteSettingLastModified(
             setting.primary_pattern, setting.secondary_pattern, content_type,
             std::string());
-        if (last_modified >= begin_time) {
+        if (last_modified >= begin_time &&
+            (last_modified < end_time || end_time.is_null())) {
           provider->SetWebsiteSetting(setting.primary_pattern,
                                       setting.secondary_pattern, content_type,
                                       std::string(), nullptr);

components/content_settings/core/browser/host_content_settings_map.h

@@ -260,10 +260,11 @@ class HostContentSettingsMap : public content_settings::Observer,
 
   // If |pattern_predicate| is null, this method is equivalent to the above.
   // Otherwise, it only deletes exceptions matched by |pattern_predicate| that
-  // were modified at or after |begin_time|.
+  // were modified at or after |begin_time| and before |end_time|.
   void ClearSettingsForOneTypeWithPredicate(
       ContentSettingsType content_type,
       base::Time begin_time,
+      base::Time end_time,
       const PatternSourcePredicate& pattern_predicate);
 
   // RefcountedKeyedService implementation.

components/safe_browsing/password_protection/password_protection_service.cc

@@ -349,6 +349,7 @@ void PasswordProtectionService::CleanUpExpiredVerdicts() {
     if (cache_dictionary->size() == 0u) {
       content_settings_->ClearSettingsForOneTypeWithPredicate(
           CONTENT_SETTINGS_TYPE_PASSWORD_PROTECTION, base::Time(),
+          base::Time::Max(),
           base::Bind(&OriginMatchPrimaryPattern, primary_pattern_url));
     } else if (has_expired_password_on_focus_entry ||
                has_expired_password_reuse_entry) {
@@ -605,7 +606,7 @@ void PasswordProtectionService::RemoveContentSettingsOnURLsDeleted(
             url_key, LoginReputationClientRequest::PASSWORD_REUSE_EVENT);
     content_settings_->ClearSettingsForOneTypeWithPredicate(
         CONTENT_SETTINGS_TYPE_PASSWORD_PROTECTION, base::Time(),
-        base::Bind(&OriginMatchPrimaryPattern, url_key));
+        base::Time::Max(), base::Bind(&OriginMatchPrimaryPattern, url_key));
   }
 }
 

content/browser/host_zoom_map_impl_unittest.cc

@@ -134,11 +134,19 @@ TEST_F(HostZoomMapTest, ClearZoomLevels) {
   host_zoom_map.SetZoomLevelForHost("zoomzoom.com", 3.5);
   test_clock.SetNow(now - base::TimeDelta::FromHours(1));
   host_zoom_map.SetZoomLevelForHost("zoom.com", 1.5);
-  EXPECT_EQ(2u, host_zoom_map.GetAllZoomLevels().size());
+  test_clock.SetNow(now - base::TimeDelta::FromDays(31));
+  host_zoom_map.SetZoomLevelForHost("zoom2.com", 2.5);
+  EXPECT_EQ(3u, host_zoom_map.GetAllZoomLevels().size());
 
   host_zoom_map.ClearZoomLevels(now - base::TimeDelta::FromHours(2),
                                 base::Time::Max());
-  EXPECT_EQ(1u, host_zoom_map.GetAllZoomLevels().size());
+  ASSERT_EQ(2u, host_zoom_map.GetAllZoomLevels().size());
+  EXPECT_EQ("zoom2.com", host_zoom_map.GetAllZoomLevels()[0].host);
+  EXPECT_EQ("zoomzoom.com", host_zoom_map.GetAllZoomLevels()[1].host);
+
+  host_zoom_map.ClearZoomLevels(base::Time(),
+                                now - base::TimeDelta::FromDays(30));
+  ASSERT_EQ(1u, host_zoom_map.GetAllZoomLevels().size());
   EXPECT_EQ("zoomzoom.com", host_zoom_map.GetAllZoomLevels()[0].host);
 
   host_zoom_map.ClearZoomLevels(base::Time(), base::Time::Max());