Write last_modified date to Content Settings in the PrefProvider

components/content_settings/core/browser/host_content_settings_map.cc

Index: components/content_settings/core/browser/host_content_settings_map.cc
diff --git a/components/content_settings/core/browser/host_content_settings_map.cc b/components/content_settings/core/browser/host_content_settings_map.cc
index 3879f3096c0b32f26bb8077fd333adb59a9e1e78..1eb70488fc0843aecd901852b72b7f4e0800ebd8 100644
--- a/components/content_settings/core/browser/host_content_settings_map.cc
+++ b/components/content_settings/core/browser/host_content_settings_map.cc
@@ -178,13 +178,15 @@ content_settings::PatternPair GetPatternsForContentSettingsType(
 
 HostContentSettingsMap::HostContentSettingsMap(PrefService* prefs,
                                                bool is_incognito_profile,
-                                               bool is_guest_profile)
+                                               bool is_guest_profile,
+                                               bool store_last_modified)
     : RefcountedKeyedService(base::ThreadTaskRunnerHandle::Get()),
 #ifndef NDEBUG
       used_from_thread_id_(base::PlatformThread::CurrentId()),
 #endif
       prefs_(prefs),
       is_incognito_(is_incognito_profile || is_guest_profile),
+      store_last_modified_(store_last_modified),
       weak_ptr_factory_(this) {
   DCHECK(!(is_incognito_profile && is_guest_profile));
 
@@ -194,8 +196,8 @@ HostContentSettingsMap::HostContentSettingsMap(PrefService* prefs,
       base::WrapUnique(policy_provider);
   policy_provider->AddObserver(this);
 
-  pref_provider_ =
-      new content_settings::PrefProvider(prefs_, is_incognito_);
+  pref_provider_ = new content_settings::PrefProvider(prefs_, is_incognito_,
+                                                      store_last_modified_);
   content_settings_providers_[PREF_PROVIDER] = base::WrapUnique(pref_provider_);
   pref_provider_->AddObserver(this);
 
@@ -658,22 +660,23 @@ void HostContentSettingsMap::ClearSettingsForOneType(
 
 void HostContentSettingsMap::ClearSettingsForOneTypeWithPredicate(
     ContentSettingsType content_type,
-    const base::Callback<bool(const ContentSettingsPattern& primary_pattern,
-                              const ContentSettingsPattern& secondary_pattern)>&
-        pattern_predicate) {
-  if (pattern_predicate.is_null()) {
+    base::Time begin_time,
+    const PatternSourcePredicate& pattern_predicate) {
+  DCHECK(!SupportsResourceIdentifier(content_type));

[raymes, 2017/04/24 03:20:23]

Could this potentially get called for plugins? Won't this fail in that case?

[dullweber, 2017/04/25 10:50:44]

This method is called by
HostContentSettingsMap::ClearSettingsForOneTypeWithPredicate. This method is
currently not called for plugins and the method also doesn't have the
resource_identifier parameter.

[raymes, 2017/04/26 01:54:17]

But won't we be calling this for plugins when we implement clearing Site
Settings? 

The resource identifier gives you a subset of the Plugins content setting. So
clearing it still makes sense - it should just clear all resource identifiers.

[dullweber, 2017/04/26 11:01:41]

That's true. I changed the PrefProvider to support deletion with resource
identifier. This method probably needs some changes too because it calls
GetSettingsForOneType with std::strings() as resource which doesn't return
settings with a resource but I think that can be fixed when I actually use it to
delete settings.

+  if (pattern_predicate.is_null() && begin_time.is_null()) {
     ClearSettingsForOneType(content_type);
     return;
   }
-
+  UsedContentSettingsProviders();
   ContentSettingsForOneType settings;
   GetSettingsForOneType(content_type, std::string(), &settings);
   for (const ContentSettingPatternSource& setting : settings) {
-    if (pattern_predicate.Run(setting.primary_pattern,
+    if (pattern_predicate.is_null() ||
+        pattern_predicate.Run(setting.primary_pattern,
                               setting.secondary_pattern)) {
-      SetWebsiteSettingCustomScope(setting.primary_pattern,
-                                   setting.secondary_pattern, content_type,
-                                   std::string(), nullptr);
+      pref_provider_->DeleteWebsiteSetting(setting.primary_pattern,

[raymes, 2017/04/24 03:20:23]

Just checking this approach seems ok with msramek? Maybe we could clarify the
name of this function, e.g.
DeleteWebsiteSettingAfterDate

[dullweber, 2017/04/25 10:50:44]

I renamed it. Martin is ooo until next week but we discussed this approach
before I implemented it, so I think he is ok with it.

+                                           setting.secondary_pattern,
+                                           content_type, begin_time);
     }
   }
 }