Remove old, unused per-plugin data from user prefs

components/content_settings/core/browser/content_settings_pref.cc

Index: components/content_settings/core/browser/content_settings_pref.cc
diff --git a/components/content_settings/core/browser/content_settings_pref.cc b/components/content_settings/core/browser/content_settings_pref.cc
index 2b37d5e33ccd3659e3eb07129415fdb7aca241df..b47948be10161b93cf50117d0c0c585325519e4b 100644
--- a/components/content_settings/core/browser/content_settings_pref.cc
+++ b/components/content_settings/core/browser/content_settings_pref.cc
@@ -102,8 +102,8 @@ bool ContentSettingsPref::SetWebsiteSetting(
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(prefs_);
   DCHECK(primary_pattern != ContentSettingsPattern::Wildcard() ||
-         secondary_pattern != ContentSettingsPattern::Wildcard() ||
-         !resource_identifier.empty());
+         secondary_pattern != ContentSettingsPattern::Wildcard());
+  DCHECK(resource_identifier.empty());
 
   // At this point take the ownership of the |in_value|.
   std::unique_ptr<base::Value> value(in_value);
@@ -116,30 +116,22 @@ bool ContentSettingsPref::SetWebsiteSetting(
   {
     base::AutoLock auto_lock(lock_);
     if (value.get()) {
-      map_to_modify->SetValue(
-          primary_pattern,
-          secondary_pattern,
-          content_type_,
-          resource_identifier,
-          value->DeepCopy());
+      map_to_modify->SetValue(primary_pattern, secondary_pattern, content_type_,
+                              std::string(), value->DeepCopy());
     } else {
-      map_to_modify->DeleteValue(
-          primary_pattern,
-          secondary_pattern,
-          content_type_,
-          resource_identifier);
+      map_to_modify->DeleteValue(primary_pattern, secondary_pattern,
+                                 content_type_, std::string());
     }
   }
   // Update the content settings preference.
   if (!is_incognito_) {
     UpdatePref(primary_pattern,
                secondary_pattern,
-               resource_identifier,
                value.get());
   }
 
-  notify_callback_.Run(
-      primary_pattern, secondary_pattern, content_type_, resource_identifier);
+  notify_callback_.Run(primary_pattern, secondary_pattern, content_type_,
+                       std::string());
 
   return true;
 }
@@ -198,7 +190,7 @@ void ContentSettingsPref::UpdateLastUsage(
 
     std::string pattern_str(
         CreatePatternString(primary_pattern, secondary_pattern));
-    base::DictionaryValue* settings_dictionary = NULL;
+    base::DictionaryValue* settings_dictionary = nullptr;
     bool found = pattern_pairs_settings->GetDictionaryWithoutPathExpansion(
         pattern_str, &settings_dictionary);
 
@@ -221,7 +213,7 @@ base::Time ContentSettingsPref::GetLastUsage(
   std::string pattern_str(
       CreatePatternString(primary_pattern, secondary_pattern));
 
-  const base::DictionaryValue* settings_dictionary = NULL;
+  const base::DictionaryValue* settings_dictionary = nullptr;
   bool found = pattern_pairs_settings->GetDictionaryWithoutPathExpansion(
       pattern_str, &settings_dictionary);
 
@@ -264,7 +256,8 @@ void ContentSettingsPref::ReadContentSettingsFromPref() {
 
   value_map_.clear();
 
-  // Careful: The returned value could be NULL if the pref has never been set.
+  // Careful: The returned value could be nullptr if the pref has never been

[Bernhard Bauer, 2016/09/27 09:34:44]

Tiny nit: I would use just "null" if it's more about the concept (so it reads a
bit more fluid, IMO).

+  // set.
   if (!all_settings_dictionary)
     return;
 
@@ -281,6 +274,7 @@ void ContentSettingsPref::ReadContentSettingsFromPref() {
   // Convert all Unicode patterns into punycode form, then read.
   CanonicalizeContentSettingsExceptions(mutable_settings);
 
+  std::vector<std::string> remove_items;
   size_t cookies_block_exception_count = 0;
   size_t cookies_allow_exception_count = 0;
   size_t cookies_session_only_exception_count = 0;
@@ -298,30 +292,16 @@ void ContentSettingsPref::ReadContentSettingsFromPref() {
 
     // Get settings dictionary for the current pattern string, and read
     // settings from the dictionary.
-    const base::DictionaryValue* settings_dictionary = NULL;
+    const base::DictionaryValue* settings_dictionary = nullptr;
     bool is_dictionary = i.value().GetAsDictionary(&settings_dictionary);
     DCHECK(is_dictionary);
 
+    // Remove the old per-plugin content settings which are no longer used.
+    // TODO(raymes): delete this code ~M57.

[msramek, 2017/01/12 23:56:07]

A lot of time has passed, so please update this :)

     if (SupportsResourceIdentifiers(content_type_)) {
-      const base::DictionaryValue* resource_dictionary = NULL;
-      if (settings_dictionary->GetDictionary(
-              kPerResourceIdentifierPrefName, &resource_dictionary)) {
-        for (base::DictionaryValue::Iterator j(*resource_dictionary);
-             !j.IsAtEnd();
-             j.Advance()) {
-          const std::string& resource_identifier(j.key());
-          int setting = CONTENT_SETTING_DEFAULT;
-          bool is_integer = j.value().GetAsInteger(&setting);
-          DCHECK(is_integer);
-          DCHECK_NE(CONTENT_SETTING_DEFAULT, setting);
-          std::unique_ptr<base::Value> setting_ptr(
-              new base::FundamentalValue(setting));
-          value_map_.SetValue(pattern_pair.first,
-                              pattern_pair.second,
-                              content_type_,
-                              resource_identifier,
-                              setting_ptr->DeepCopy());
-        }
+      if (settings_dictionary->GetWithoutPathExpansion(
+              kPerResourceIdentifierPrefName, nullptr)) {
+        remove_items.push_back(pattern_str);
       }
     }
 
@@ -353,7 +333,17 @@ void ContentSettingsPref::ReadContentSettingsFromPref() {
         }
       }
     }
+  }
 
+  for (std::string pattern_str : remove_items) {

[Bernhard Bauer, 2016/09/27 09:34:44]

const std::string& so you don't copy them?

+    base::DictionaryValue* settings_dictionary = nullptr;
+    if (mutable_settings->GetDictionaryWithoutPathExpansion(
+            pattern_str, &settings_dictionary)) {
+      settings_dictionary->RemoveWithoutPathExpansion(
+          kPerResourceIdentifierPrefName, nullptr);
+      if (settings_dictionary->empty())
+        mutable_settings->RemoveWithoutPathExpansion(pattern_str, nullptr);
+    }
   }
 
   if (content_type_ == CONTENT_SETTINGS_TYPE_COOKIES) {
@@ -383,7 +373,6 @@ void ContentSettingsPref::OnPrefChanged() {
 void ContentSettingsPref::UpdatePref(
     const ContentSettingsPattern& primary_pattern,
     const ContentSettingsPattern& secondary_pattern,
-    const ResourceIdentifier& resource_identifier,
     const base::Value* value) {
   // Ensure that |lock_| is not held by this thread, since this function will
   // send out notifications (by |~DictionaryPrefUpdate|).
@@ -397,7 +386,7 @@ void ContentSettingsPref::UpdatePref(
     // Get settings dictionary for the given patterns.
     std::string pattern_str(CreatePatternString(primary_pattern,
                                                 secondary_pattern));
-    base::DictionaryValue* settings_dictionary = NULL;
+    base::DictionaryValue* settings_dictionary = nullptr;
     bool found = pattern_pairs_settings->GetDictionaryWithoutPathExpansion(
         pattern_str, &settings_dictionary);
 
@@ -408,44 +397,10 @@ void ContentSettingsPref::UpdatePref(
     }
 
     if (settings_dictionary) {
-      if (SupportsResourceIdentifiers(content_type_) &&
-          !resource_identifier.empty()) {
-        base::DictionaryValue* resource_dictionary = NULL;
-        found = settings_dictionary->GetDictionary(
-            kPerResourceIdentifierPrefName, &resource_dictionary);
-        if (!found) {
-          if (value == NULL)
-            return;  // Nothing to remove. Exit early.
-          resource_dictionary = new base::DictionaryValue;
-          settings_dictionary->Set(
-              kPerResourceIdentifierPrefName, resource_dictionary);
-        }
-        // Update resource dictionary.
-        if (value == NULL) {
-          resource_dictionary->RemoveWithoutPathExpansion(resource_identifier,
-                                                          NULL);
-          if (resource_dictionary->empty()) {
-            settings_dictionary->RemoveWithoutPathExpansion(
-                kPerResourceIdentifierPrefName, NULL);
-          }
-        } else {
-          resource_dictionary->SetWithoutPathExpansion(
-              resource_identifier, value->DeepCopy());
-        }
-      } else {
-        // Update settings dictionary.
-        if (value == NULL) {
-          settings_dictionary->RemoveWithoutPathExpansion(kSettingPath, NULL);
-          settings_dictionary->RemoveWithoutPathExpansion(kLastUsed, NULL);
-        } else {
-          settings_dictionary->SetWithoutPathExpansion(
-              kSettingPath, value->DeepCopy());
-        }
-      }
       // Remove the settings dictionary if it is empty.
       if (settings_dictionary->empty()) {
-        pattern_pairs_settings->RemoveWithoutPathExpansion(
-            pattern_str, NULL);
+        pattern_pairs_settings->RemoveWithoutPathExpansion(pattern_str,
+                                                           nullptr);
       }
     }
   }
@@ -479,7 +434,7 @@ void ContentSettingsPref::CanonicalizeContentSettingsExceptions(
     }
 
     // Clear old pattern if prefs already have canonicalized pattern.
-    const base::DictionaryValue* new_pattern_settings_dictionary = NULL;
+    const base::DictionaryValue* new_pattern_settings_dictionary = nullptr;
     if (all_settings_dictionary->GetDictionaryWithoutPathExpansion(
             canonicalized_pattern_str, &new_pattern_settings_dictionary)) {
       remove_items.push_back(pattern_str);
@@ -487,7 +442,7 @@ void ContentSettingsPref::CanonicalizeContentSettingsExceptions(
     }
 
     // Move old pattern to canonicalized pattern.
-    const base::DictionaryValue* old_pattern_settings_dictionary = NULL;
+    const base::DictionaryValue* old_pattern_settings_dictionary = nullptr;
     if (i.value().GetAsDictionary(&old_pattern_settings_dictionary)) {
       move_items.push_back(
           std::make_pair(pattern_str, canonicalized_pattern_str));
@@ -495,7 +450,8 @@ void ContentSettingsPref::CanonicalizeContentSettingsExceptions(
   }
 
   for (size_t i = 0; i < remove_items.size(); ++i) {
-    all_settings_dictionary->RemoveWithoutPathExpansion(remove_items[i], NULL);
+    all_settings_dictionary->RemoveWithoutPathExpansion(remove_items[i],
+                                                        nullptr);
   }
 
   for (size_t i = 0; i < move_items.size(); ++i) {