Write last_modified date to Content Settings in the PrefProvider

chrome/browser/content_settings/content_settings_pref_provider_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/content_settings/core/browser/content_settings_pref_provider.h"
 
 #include <memory>
 
 #include "base/auto_reset.h"
 #include "base/command_line.h"
@@ skipping 96 matching lines @@
     // Ensure all content settings are initialized.
     ContentSettingsRegistry::GetInstance();
   }
 
  private:
   content::TestBrowserThreadBundle thread_bundle_;
 };
 
 TEST_F(PrefProviderTest, Observer) {
   TestingProfile profile;
-  PrefProvider pref_content_settings_provider(profile.GetPrefs(), false);
+  PrefProvider pref_content_settings_provider(profile.GetPrefs(), false, true);

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

nit (here and below): please document these boolean parameters, e.g.  
, true /* param_name */);

If you could document the existing param it would be great as well!

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

Done.

 
   ContentSettingsPattern pattern =
       ContentSettingsPattern::FromString("[*.]example.com");
   MockObserver mock_observer;
   EXPECT_CALL(mock_observer, OnContentSettingChanged(
                                  pattern, ContentSettingsPattern::Wildcard(),
                                  CONTENT_SETTINGS_TYPE_COOKIES, ""));
 
   pref_content_settings_provider.AddObserver(&mock_observer);
 
@@ skipping 27 matching lines @@
   data_for_pattern->SetInteger("setting", CONTENT_SETTING_ALLOW);
   pref_data.SetWithoutPathExpansion(kPattern, std::move(data_for_pattern));
   prefs->Set(kFullscreenPrefPath, pref_data);
 #if !defined(OS_ANDROID)
   prefs->Set(kMouselockPrefPath, pref_data);
 #endif
   prefs->Set(kGeolocationPrefPath, pref_data);
 
   // Instantiate a new PrefProvider here, because we want to test the
   // constructor's behavior after setting the above.
-  PrefProvider provider(prefs, false);
+  PrefProvider provider(prefs, false, true);
   provider.ShutdownOnUIThread();
 
   // Check that fullscreen and mouselock have been deleted.
   EXPECT_FALSE(prefs->HasPrefPath(kFullscreenPrefPath));
 #if !defined(OS_ANDROID)
   EXPECT_FALSE(prefs->HasPrefPath(kMouselockPrefPath));
 #endif
   EXPECT_TRUE(prefs->HasPrefPath(kGeolocationPrefPath));
   GURL primary_url("http://example.com/");
   EXPECT_EQ(CONTENT_SETTING_ALLOW,
@@ skipping 37 matching lines @@
 
   auto mic_pattern_data = base::MakeUnique<base::DictionaryValue>();
   mic_pattern_data->SetInteger("setting", CONTENT_SETTING_ALLOW);
   mic_pattern_data->SetDouble(kObsoleteLastUsed, 1480000000.0);
   base::DictionaryValue mic_pref_data;
   mic_pref_data.SetWithoutPathExpansion(kPattern, std::move(mic_pattern_data));
   prefs->Set(kMicPrefPath, mic_pref_data);
 
   // Instantiate a new PrefProvider here, because we want to test the
   // constructor's behavior after setting the above.
-  PrefProvider provider(prefs, false);
+  PrefProvider provider(prefs, false, true);
 
   // Check that last_used data has been deleted.
   EXPECT_TRUE(prefs->GetDictionary(kGeolocationPrefPath)->empty());
   auto* mic_prefs = prefs->GetDictionary(kMicPrefPath);
   const base::DictionaryValue* mic_result_pattern_data;
   ASSERT_TRUE(mic_prefs->GetDictionaryWithoutPathExpansion(
       kPattern, &mic_result_pattern_data));
   EXPECT_EQ(static_cast<size_t>(1), mic_result_pattern_data->size());
   int mic_result_setting;
   EXPECT_TRUE(
@@ skipping 32 matching lines @@
   chrome::RegisterUserProfilePrefs(otr_registry.get());
 
   TestingProfile::Builder profile_builder;
   profile_builder.SetPrefService(base::WrapUnique(regular_prefs));
   std::unique_ptr<TestingProfile> profile = profile_builder.Build();
 
   TestingProfile::Builder otr_profile_builder;
   otr_profile_builder.SetPrefService(base::WrapUnique(otr_prefs));
   otr_profile_builder.BuildIncognito(profile.get());
 
-  PrefProvider pref_content_settings_provider(regular_prefs, false);
-  PrefProvider pref_content_settings_provider_incognito(otr_prefs, true);
+  PrefProvider pref_content_settings_provider(regular_prefs, false, true);
+  PrefProvider pref_content_settings_provider_incognito(otr_prefs, true, true);
   ContentSettingsPattern pattern =
       ContentSettingsPattern::FromString("[*.]example.com");
   pref_content_settings_provider.SetWebsiteSetting(
       pattern, pattern, CONTENT_SETTINGS_TYPE_COOKIES, std::string(),
       new base::Value(CONTENT_SETTING_ALLOW));
 
   GURL host("http://example.com/");
   // The value should of course be visible in the regular PrefProvider.
   EXPECT_EQ(CONTENT_SETTING_ALLOW,
             TestUtils::GetContentSetting(&pref_content_settings_provider, host,
                                          host, CONTENT_SETTINGS_TYPE_COOKIES,
                                          std::string(), false));
   // And also in the OTR version.
   EXPECT_EQ(CONTENT_SETTING_ALLOW,
             TestUtils::GetContentSetting(
                 &pref_content_settings_provider_incognito, host, host,
                 CONTENT_SETTINGS_TYPE_COOKIES, std::string(), false));
   const WebsiteSettingsInfo* info = WebsiteSettingsRegistry::GetInstance()->Get(
       CONTENT_SETTINGS_TYPE_COOKIES);
   // But the value should not be overridden in the OTR user prefs accidentally.
   EXPECT_FALSE(otr_user_prefs->IsSetInOverlay(info->pref_name()));
 
   pref_content_settings_provider.ShutdownOnUIThread();
   pref_content_settings_provider_incognito.ShutdownOnUIThread();
 }
 
 TEST_F(PrefProviderTest, GetContentSettingsValue) {
   TestingProfile testing_profile;
-  PrefProvider provider(testing_profile.GetPrefs(), false);
+  PrefProvider provider(testing_profile.GetPrefs(), false, true);
 
   GURL primary_url("http://example.com/");
   ContentSettingsPattern primary_pattern =
       ContentSettingsPattern::FromString("[*.]example.com");
 
   EXPECT_EQ(CONTENT_SETTING_DEFAULT,
             TestUtils::GetContentSetting(&provider, primary_url, primary_url,
                                          CONTENT_SETTINGS_TYPE_COOKIES,
                                          std::string(), false));
 
@@ skipping 19 matching lines @@
                              CONTENT_SETTINGS_TYPE_COOKIES, std::string(),
                              NULL);
   EXPECT_EQ(NULL, TestUtils::GetContentSettingValue(
                       &provider, primary_url, primary_url,
                       CONTENT_SETTINGS_TYPE_COOKIES, std::string(), false));
   provider.ShutdownOnUIThread();
 }
 
 TEST_F(PrefProviderTest, Patterns) {
   TestingProfile testing_profile;
-  PrefProvider pref_content_settings_provider(testing_profile.GetPrefs(),
-                                              false);
+  PrefProvider pref_content_settings_provider(testing_profile.GetPrefs(), false,
+                                              true);
 
   GURL host1("http://example.com/");
   GURL host2("http://www.example.com/");
   GURL host3("http://example.org/");
   GURL host4("file:///tmp/test.html");
   ContentSettingsPattern pattern1 =
       ContentSettingsPattern::FromString("[*.]example.com");
   ContentSettingsPattern pattern2 =
       ContentSettingsPattern::FromString("example.org");
   ContentSettingsPattern pattern3 =
@@ skipping 38 matching lines @@
             TestUtils::GetContentSetting(&pref_content_settings_provider, host4,
                                          host4, CONTENT_SETTINGS_TYPE_COOKIES,
                                          std::string(), false));
 
   pref_content_settings_provider.ShutdownOnUIThread();
 }
 
 #if BUILDFLAG(ENABLE_PLUGINS)
 TEST_F(PrefProviderTest, ResourceIdentifier) {
   TestingProfile testing_profile;
-  PrefProvider pref_content_settings_provider(testing_profile.GetPrefs(),
-                                              false);
+  PrefProvider pref_content_settings_provider(testing_profile.GetPrefs(), false,
+                                              true);
 
   GURL host("http://example.com/");
   ContentSettingsPattern pattern =
       ContentSettingsPattern::FromString("[*.]example.com");
   std::string resource1("someplugin");
   std::string resource2("otherplugin");
 
   EXPECT_EQ(CONTENT_SETTING_DEFAULT,
             TestUtils::GetContentSetting(&pref_content_settings_provider, host,
                                          host, CONTENT_SETTINGS_TYPE_PLUGINS,
@@ skipping 19 matching lines @@
   sync_preferences::TestingPrefServiceSyncable prefs;
   PrefProvider::RegisterProfilePrefs(prefs.registry());
 
   // Chain of events: a preference changes, |PrefProvider| notices it, and reads
   // and writes the preference. When the preference is written, a notification
   // is sent, and this used to happen when |PrefProvider| was still holding its
   // lock.
 
   const WebsiteSettingsInfo* info = WebsiteSettingsRegistry::GetInstance()->Get(
       CONTENT_SETTINGS_TYPE_COOKIES);
-  PrefProvider provider(&prefs, false);
+  PrefProvider provider(&prefs, false, true);
   DeadlockCheckerObserver observer(&prefs, &provider);
   {
     DictionaryPrefUpdate update(&prefs, info->pref_name());
     base::DictionaryValue* mutable_settings = update.Get();
     mutable_settings->SetWithoutPathExpansion(
         "www.example.com,*", base::MakeUnique<base::DictionaryValue>());
   }
   EXPECT_TRUE(observer.notification_received());
 
   provider.ShutdownOnUIThread();
 }
 
 TEST_F(PrefProviderTest, IncognitoInheritsValueMap) {
   sync_preferences::TestingPrefServiceSyncable prefs;
   PrefProvider::RegisterProfilePrefs(prefs.registry());
 
   ContentSettingsPattern pattern_1 =
       ContentSettingsPattern::FromString("google.com");
   ContentSettingsPattern pattern_2 =
       ContentSettingsPattern::FromString("www.google.com");
   ContentSettingsPattern wildcard =
       ContentSettingsPattern::FromString("*");
   std::unique_ptr<base::Value> value(new base::Value(CONTENT_SETTING_ALLOW));
 
   // Create a normal provider and set a setting.
-  PrefProvider normal_provider(&prefs, false);
+  PrefProvider normal_provider(&prefs, false, true);
   normal_provider.SetWebsiteSetting(pattern_1, wildcard,
                                     CONTENT_SETTINGS_TYPE_COOKIES,
                                     std::string(), value->DeepCopy());
 
   // Non-OTR provider, Non-OTR iterator has one setting (pattern 1).
   {
     std::unique_ptr<RuleIterator> it(normal_provider.GetRuleIterator(
         CONTENT_SETTINGS_TYPE_COOKIES, std::string(), false));
     EXPECT_TRUE(it->HasNext());
     EXPECT_EQ(pattern_1, it->Next().primary_pattern);
     EXPECT_FALSE(it->HasNext());
   }
 
   // Non-OTR provider, OTR iterator has no settings.
   {
     std::unique_ptr<RuleIterator> it(normal_provider.GetRuleIterator(
         CONTENT_SETTINGS_TYPE_COOKIES, std::string(), true));
     EXPECT_FALSE(it);
   }
 
   // Create an incognito provider and set a setting.
-  PrefProvider incognito_provider(&prefs, true);
+  PrefProvider incognito_provider(&prefs, true, true);
   incognito_provider.SetWebsiteSetting(pattern_2, wildcard,
                                        CONTENT_SETTINGS_TYPE_COOKIES,
                                        std::string(), value->DeepCopy());
 
   // OTR provider, non-OTR iterator has one setting (pattern 1).
   {
     std::unique_ptr<RuleIterator> it(incognito_provider.GetRuleIterator(
         CONTENT_SETTINGS_TYPE_COOKIES, std::string(), false));
     EXPECT_TRUE(it->HasNext());
     EXPECT_EQ(pattern_1, it->Next().primary_pattern);
@@ skipping 17 matching lines @@
   sync_preferences::TestingPrefServiceSyncable prefs;
   PrefProvider::RegisterProfilePrefs(prefs.registry());
 
   ContentSettingsPattern pattern =
       ContentSettingsPattern::FromString("google.com");
   ContentSettingsPattern wildcard =
       ContentSettingsPattern::FromString("*");
   std::unique_ptr<base::Value> value(new base::Value(CONTENT_SETTING_ALLOW));
   ResourceIdentifier res_id("abcde");
 
-  PrefProvider provider(&prefs, false);
+  PrefProvider provider(&prefs, false, true);
 
   // Non-empty pattern, syncable, empty resource identifier.
   provider.SetWebsiteSetting(pattern, wildcard,
                              CONTENT_SETTINGS_TYPE_JAVASCRIPT,
                              ResourceIdentifier(), value->DeepCopy());
 
   // Non-empty pattern, non-syncable, empty resource identifier.
   provider.SetWebsiteSetting(pattern, wildcard,
                              CONTENT_SETTINGS_TYPE_GEOLOCATION,
                              ResourceIdentifier(), value->DeepCopy());
@@ skipping 45 matching lines @@
 
   for (const char* pref : nonempty_prefs) {
     DictionaryPrefUpdate update(&prefs, pref);
     const base::DictionaryValue* dictionary = update.Get();
     EXPECT_EQ(1u, dictionary->size());
   }
 
   provider.ShutdownOnUIThread();
 }
 
+TEST_F(PrefProviderTest, DeleteWebsiteSetting) {
+  sync_preferences::TestingPrefServiceSyncable prefs;
+  PrefProvider::RegisterProfilePrefs(prefs.registry());
+
+  ContentSettingsPattern pattern_1 =
+      ContentSettingsPattern::FromString("google.com");
+  ContentSettingsPattern pattern_2 =
+      ContentSettingsPattern::FromString("www.google.com");
+  std::unique_ptr<base::Value> value(new base::Value(CONTENT_SETTING_ALLOW));
+
+  // Create a  provider and set a setting.
+  PrefProvider provider(&prefs, false, /*store_last_modified=*/true);

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

The usual way to do this is:
true /* store_last_modified */);

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

I changed it but do you know if this style is documented somewhere? 
The chromium style guide doesn't mention parameter comments all and the google
style guide suggests the /*comment=*/ style. 

https://google.github.io/styleguide/cppguide.html#Implementation_Comments -->
Function Argument Comments

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

Ah I actually just found out today that the style you had is now considered
preferable. The reason is because clang will check it. I'm sorry about that, my
experience on Chrome has always been to use the style I suggested.

Don't feel you need to change this now, it's already a style that's used in many
places.

+  provider.SetWebsiteSetting(pattern_1, ContentSettingsPattern::Wildcard(),
+                             CONTENT_SETTINGS_TYPE_COOKIES, std::string(),
+                             value->DeepCopy());
+  provider.SetWebsiteSetting(pattern_2, ContentSettingsPattern::Wildcard(),
+                             CONTENT_SETTINGS_TYPE_COOKIES, std::string(),
+                             value->DeepCopy());
+  base::Time t = base::Time::Now();
+  // These clear calls shouldn't do anything because both rules were created
+  // before |t|.
+  provider.DeleteWebsiteSetting(pattern_1, ContentSettingsPattern::Wildcard(),
+                                CONTENT_SETTINGS_TYPE_COOKIES, t);
+  provider.DeleteWebsiteSetting(pattern_2, ContentSettingsPattern::Wildcard(),
+                                CONTENT_SETTINGS_TYPE_COOKIES, t);
+  {
+    std::unique_ptr<RuleIterator> it(provider.GetRuleIterator(
+        CONTENT_SETTINGS_TYPE_COOKIES, std::string(), false));
+    EXPECT_TRUE(it->HasNext());
+    Rule rule = it->Next();
+    EXPECT_EQ(pattern_1, rule.primary_pattern);
+    EXPECT_TRUE(it->HasNext());
+    rule = it->Next();
+    EXPECT_EQ(pattern_2, rule.primary_pattern);
+    EXPECT_FALSE(it->HasNext());
+  }
+
+  // Update pattern_1 and check that the timestamp changes.
+  std::unique_ptr<base::Value> value2(new base::Value(CONTENT_SETTING_BLOCK));
+  provider.SetWebsiteSetting(pattern_1, ContentSettingsPattern::Wildcard(),
+                             CONTENT_SETTINGS_TYPE_COOKIES, std::string(),
+                             value2->DeepCopy());
+  // This time pattern_1 should be deleted because it was edited after |t|.
+  provider.DeleteWebsiteSetting(pattern_1, ContentSettingsPattern::Wildcard(),
+                                CONTENT_SETTINGS_TYPE_COOKIES, t);
+  provider.DeleteWebsiteSetting(pattern_2, ContentSettingsPattern::Wildcard(),
+                                CONTENT_SETTINGS_TYPE_COOKIES, t);
+  {
+    std::unique_ptr<RuleIterator> it(provider.GetRuleIterator(
+        CONTENT_SETTINGS_TYPE_COOKIES, std::string(), false));
+    EXPECT_TRUE(it->HasNext());
+    Rule rule = it->Next();
+    EXPECT_EQ(pattern_2, rule.primary_pattern);
+    EXPECT_FALSE(it->HasNext());
+  }
+
+  provider.ShutdownOnUIThread();
+}
+
 }  // namespace content_settings