Allow resource identifiers in content settings.

Plug-in specific content settings can only be set via the extension API. The --enable-resource-content-settings switch enables them in user preferences.

BUG=78428
TEST=none


Review URL: http://codereview.chromium.org/8202004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@105358 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 66 insertions, 144 deletions

diff --git a/chrome/browser/content_settings/content_settings_policy_provider_unittest.cc b/chrome/browser/content_settings/content_settings_policy_provider_unittest.cc
index 12ea264..76198fd 100644
--- a/chrome/browser/content_settings/content_settings_policy_provider_unittest.cc
+++ b/chrome/browser/content_settings/content_settings_policy_provider_unittest.cc
@@ -89,11 +89,6 @@ TEST_F(PolicyProviderTest, DefaultGeolocationContentSetting) {
 }
 
 TEST_F(PolicyProviderTest, ManagedDefaultContentSettings) {
-  // This feature is currently behind a flag.
-  CommandLine* cmd = CommandLine::ForCurrentProcess();
-  AutoReset<CommandLine> auto_reset(cmd, *cmd);
-  cmd->AppendSwitch(switches::kEnableResourceContentSettings);
-
   TestingProfile profile;
   TestingPrefService* prefs = profile.GetTestingPrefService();
   PolicyProvider provider(prefs);
@@ -193,10 +188,6 @@ TEST_F(PolicyProviderTest, GettingManagedContentSettings) {
 }
 
 TEST_F(PolicyProviderTest, ResourceIdentifier) {
-  CommandLine* cmd = CommandLine::ForCurrentProcess();
-  AutoReset<CommandLine> auto_reset(cmd, *cmd);
-  cmd->AppendSwitch(switches::kEnableResourceContentSettings);
-
   TestingProfile profile;
   TestingPrefService* prefs = profile.GetTestingPrefService();
 
@@ -217,9 +208,8 @@ TEST_F(PolicyProviderTest, ResourceIdentifier) {
                 CONTENT_SETTINGS_TYPE_PLUGINS,
                 "someplugin"));
 
-  // There is no policy support for resource content settings until the feature
-  // is enabled by default. Resource identifiers are simply ignored by the
-  // PolicyProvider.
+  // There is currently no policy support for resource content settings.
+  // Resource identifiers are simply ignored by the PolicyProvider.
   EXPECT_EQ(CONTENT_SETTING_ALLOW,
             provider.GetContentSetting(
                 google_url,
diff --git a/chrome/browser/content_settings/content_settings_pref_provider.cc b/chrome/browser/content_settings/content_settings_pref_provider.cc
index 0bf4dc8..03fe71d 100644
--- a/chrome/browser/content_settings/content_settings_pref_provider.cc
+++ b/chrome/browser/content_settings/content_settings_pref_provider.cc
@@ -9,6 +9,7 @@
 #include <utility>
 
 #include "base/auto_reset.h"
+#include "base/command_line.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/metrics/histogram.h"
 #include "chrome/browser/content_settings/content_settings_rule.h"
@@ -16,6 +17,7 @@
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/prefs/scoped_user_pref_update.h"
 #include "chrome/common/chrome_notification_types.h"
+#include "chrome/common/chrome_switches.h"
 #include "chrome/common/content_settings.h"
 #include "chrome/common/content_settings_pattern.h"
 #include "chrome/common/pref_names.h"
@@ -30,6 +32,8 @@ namespace {
 typedef std::pair<std::string, std::string> StringPair;
 typedef std::map<std::string, std::string> StringMap;
 
+const char kPerPluginPrefName[] = "per_plugin";
+
 ContentSetting FixObsoleteCookiePromptMode(ContentSettingsType content_type,
                                            ContentSetting setting) {
   if (content_type == CONTENT_SETTINGS_TYPE_COOKIES &&
@@ -57,6 +61,21 @@ void ClearSettings(ContentSettingsType type,
   }
 }
 
+// If the given content type supports resource identifiers in user preferences,
+// returns true and sets |pref_key| to the key in the content settings
+// dictionary under which per-resource content settings are stored.
+// Otherwise, returns false.
+bool GetResourceTypeName(ContentSettingsType content_type,
+                         std::string* pref_key) {
+  if (content_type == CONTENT_SETTINGS_TYPE_PLUGINS &&
+      CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableResourceContentSettings)) {
+    *pref_key = kPerPluginPrefName;
+    return true;
+  }
+  return false;
+}
+
 }  // namespace
 
 namespace content_settings {
@@ -414,12 +433,11 @@ void PrefProvider::ReadContentSettingsFromPref(bool overwrite) {
     for (size_t i = 0; i < CONTENT_SETTINGS_NUM_TYPES; ++i) {
       ContentSettingsType content_type = static_cast<ContentSettingsType>(i);
 
-      if (SupportsResourceIdentifier(content_type)) {
-        const std::string content_type_str =
-            GetResourceTypeName(ContentSettingsType(i));
+      std::string res_dictionary_path;
+      if (GetResourceTypeName(content_type, &res_dictionary_path)) {
         DictionaryValue* resource_dictionary = NULL;
         if (settings_dictionary->GetDictionary(
-                content_type_str, &resource_dictionary)) {
+                res_dictionary_path, &resource_dictionary)) {
           for (DictionaryValue::key_iterator j(
                    resource_dictionary->begin_keys());
                j != resource_dictionary->end_keys();
@@ -439,11 +457,9 @@ void PrefProvider::ReadContentSettingsFromPref(bool overwrite) {
           }
         }
       } else {
-        const std::string content_type_str =
-            GetTypeName(ContentSettingsType(i));
         int setting = CONTENT_SETTING_DEFAULT;
         if (settings_dictionary->GetIntegerWithoutPathExpansion(
-                content_type_str, &setting)) {
+                GetTypeName(ContentSettingsType(i)), &setting)) {
           DCHECK_NE(CONTENT_SETTING_DEFAULT, setting);
           setting = FixObsoleteCookiePromptMode(content_type,
                                                 ContentSetting(setting));
@@ -483,9 +499,8 @@ void PrefProvider::UpdateObsoletePatternsPref(
   }
 
   if (settings_dictionary) {
-    if (SupportsResourceIdentifier(content_type)) {
-      // Get resource dictionary.
-      std::string res_dictionary_path(GetResourceTypeName(content_type));
+    std::string res_dictionary_path;
+    if (GetResourceTypeName(content_type, &res_dictionary_path)) {
       DictionaryValue* resource_dictionary = NULL;
       found = settings_dictionary->GetDictionary(
           res_dictionary_path, &resource_dictionary);
@@ -517,11 +532,11 @@ void PrefProvider::UpdateObsoletePatternsPref(
         settings_dictionary->SetWithoutPathExpansion(
             setting_path, Value::CreateIntegerValue(setting));
       }
-    }
-    // Remove the settings dictionary if it is empty.
-    if (settings_dictionary->empty()) {
-      all_settings_dictionary->RemoveWithoutPathExpansion(
-          pattern_str, NULL);
+      // Remove the settings dictionary if it is empty.
+      if (settings_dictionary->empty()) {
+        all_settings_dictionary->RemoveWithoutPathExpansion(
+            pattern_str, NULL);
+      }
     }
   }
 }
@@ -547,9 +562,8 @@ void PrefProvider::UpdatePatternPairsSettings(
   }
 
   if (settings_dictionary) {
-    if (SupportsResourceIdentifier(content_type)) {
-      // Get resource dictionary.
-      std::string res_dictionary_path = GetResourceTypeName(content_type);
+    std::string res_dictionary_path;
+    if (GetResourceTypeName(content_type, &res_dictionary_path)) {
       DictionaryValue* resource_dictionary = NULL;
       found = settings_dictionary->GetDictionary(
           res_dictionary_path, &resource_dictionary);
@@ -721,9 +735,9 @@ void PrefProvider::MigrateObsoletePerhostPref() {
         prefs_->GetDictionary(prefs::kPerHostContentSettings);
     DCHECK(all_settings_dictionary);
     for (DictionaryValue::key_iterator
-         i(all_settings_dictionary->begin_keys());
-         i != all_settings_dictionary->end_keys(); ++i) {
-      const std::string& host(*i);
+         host_it(all_settings_dictionary->begin_keys());
+         host_it != all_settings_dictionary->end_keys(); ++host_it) {
+      const std::string& host(*host_it);
       ContentSettingsPattern pattern =
           ContentSettingsPattern::FromString(
               std::string(ContentSettingsPattern::kDomainWildcard) + host);
@@ -732,27 +746,12 @@ void PrefProvider::MigrateObsoletePerhostPref() {
           host, &host_settings_dictionary);
       DCHECK(found);
 
-      for (DictionaryValue::key_iterator i(
-               host_settings_dictionary->begin_keys());
-           i != host_settings_dictionary->end_keys();
-           ++i) {
-        const std::string& content_type_str(*i);
-        ContentSettingsType content_type =
-            StringToContentSettingsType(content_type_str);
-
-        if (content_type == CONTENT_SETTINGS_TYPE_DEFAULT) {
-          NOTREACHED();
-          LOG(DFATAL) << "Skip settings for invalid content settings type '"
-                      << content_type_str << "'";
-          continue;
-        }
+      for (size_t i = 0; i < CONTENT_SETTINGS_NUM_TYPES; ++i) {
+        ContentSettingsType content_type = static_cast<ContentSettingsType>(i);
 
-        if (!SupportsResourceIdentifier(content_type)) {
-          int setting_int_value = CONTENT_SETTING_DEFAULT;
-          bool found =
-              host_settings_dictionary->GetIntegerWithoutPathExpansion(
-                  content_type_str, &setting_int_value);
-          DCHECK(found);
+        int setting_int_value = CONTENT_SETTING_DEFAULT;
+        if (host_settings_dictionary->GetIntegerWithoutPathExpansion(
+                GetTypeName(content_type), &setting_int_value)) {
           ContentSetting setting = IntToContentSetting(setting_int_value);
 
           setting = FixObsoleteCookiePromptMode(content_type, setting);
diff --git a/chrome/browser/content_settings/content_settings_pref_provider_unittest.cc b/chrome/browser/content_settings/content_settings_pref_provider_unittest.cc
index e940293..f42e06c 100644
--- a/chrome/browser/content_settings/content_settings_pref_provider_unittest.cc
+++ b/chrome/browser/content_settings/content_settings_pref_provider_unittest.cc
@@ -304,11 +304,6 @@ TEST_F(PrefProviderTest, Patterns) {
 }
 
 TEST_F(PrefProviderTest, ResourceIdentifier) {
-  // This feature is currently behind a flag.
-  CommandLine* cmd = CommandLine::ForCurrentProcess();
-  AutoReset<CommandLine> auto_reset(cmd, *cmd);
-  cmd->AppendSwitch(switches::kEnableResourceContentSettings);
-
   TestingProfile testing_profile;
   PrefProvider pref_content_settings_provider(testing_profile.GetPrefs(),
                                               false);
diff --git a/chrome/browser/content_settings/content_settings_utils.cc b/chrome/browser/content_settings/content_settings_utils.cc
index 66ec547..25b38cf 100644
--- a/chrome/browser/content_settings/content_settings_utils.cc
+++ b/chrome/browser/content_settings/content_settings_utils.cc
@@ -14,39 +14,6 @@
 
 namespace {
 
-// True if a given content settings type requires additional resource
-// identifiers.
-const bool kSupportsResourceIdentifier[CONTENT_SETTINGS_NUM_TYPES] = {
-  false,  // CONTENT_SETTINGS_TYPE_COOKIES
-  false,  // CONTENT_SETTINGS_TYPE_IMAGES
-  false,  // CONTENT_SETTINGS_TYPE_JAVASCRIPT
-  true,   // CONTENT_SETTINGS_TYPE_PLUGINS
-  false,  // CONTENT_SETTINGS_TYPE_POPUPS
-  false,  // CONTENT_SETTINGS_TYPE_GEOLOCATION
-  false,  // CONTENT_SETTINGS_TYPE_NOTIFICATIONS
-  false,  // CONTENT_SETTINGS_TYPE_INTENTS
-  false,  // CONTENT_SETTINGS_TYPE_AUTO_SUBMIT_CERTIFICATE
-};
-COMPILE_ASSERT(arraysize(kSupportsResourceIdentifier) ==
-                   CONTENT_SETTINGS_NUM_TYPES,
-               resource_type_names_incorrect_size);
-
-// The preference keys where resource identifiers are stored for
-// ContentSettingsType values that support resource identifiers.
-const char* kResourceTypeNames[] = {
-  NULL,
-  NULL,
-  NULL,
-  "per_plugin",
-  NULL,
-  NULL,
-  NULL,
-  NULL,
-  NULL,
-};
-COMPILE_ASSERT(arraysize(kResourceTypeNames) == CONTENT_SETTINGS_NUM_TYPES,
-               resource_type_names_incorrect_size);
-
 // The names of the ContentSettingsType values, for use with dictionary prefs.
 const char* kTypeNames[] = {
   "cookies",
@@ -62,7 +29,7 @@ const char* kTypeNames[] = {
 COMPILE_ASSERT(arraysize(kTypeNames) == CONTENT_SETTINGS_NUM_TYPES,
                type_names_incorrect_size);
 
-const char* kPatternSeparator = ",";
+const char kPatternSeparator[] = ",";
 
 }  // namespace
 
@@ -72,32 +39,8 @@ std::string GetTypeName(ContentSettingsType type) {
   return std::string(kTypeNames[type]);
 }
 
-std::string GetResourceTypeName(ContentSettingsType type) {
-  return std::string(kResourceTypeNames[type]);
-}
-
-ContentSettingsType StringToContentSettingsType(
-    const std::string& content_type_str) {
-  for (size_t type = 0; type < arraysize(kTypeNames); ++type) {
-    if ((kTypeNames[type] != NULL) && (kTypeNames[type] == content_type_str))
-      return ContentSettingsType(type);
-  }
-  for (size_t type = 0; type < arraysize(kResourceTypeNames); ++type) {
-    if ((kResourceTypeNames[type] != NULL) &&
-        (kResourceTypeNames[type] == content_type_str)) {
-      return ContentSettingsType(type);
-    }
-  }
-  return CONTENT_SETTINGS_TYPE_DEFAULT;
-}
-
 bool SupportsResourceIdentifier(ContentSettingsType content_type) {
-  if (CommandLine::ForCurrentProcess()->HasSwitch(
-      switches::kEnableResourceContentSettings)) {
-    return kSupportsResourceIdentifier[content_type];
-  } else {
-    return false;
-  }
+  return content_type == CONTENT_SETTINGS_TYPE_PLUGINS;
 }
 
 ContentSetting ClickToPlayFixup(ContentSettingsType content_type,
diff --git a/chrome/browser/content_settings/content_settings_utils.h b/chrome/browser/content_settings/content_settings_utils.h
index 70c7955..7a616a5 100644
--- a/chrome/browser/content_settings/content_settings_utils.h
+++ b/chrome/browser/content_settings/content_settings_utils.h
@@ -22,12 +22,8 @@ typedef std::pair<ContentSettingsPattern, ContentSettingsPattern> PatternPair;
 
 std::string GetTypeName(ContentSettingsType type);
 
-std::string GetResourceTypeName(ContentSettingsType type);
-
-ContentSettingsType StringToContentSettingsType(
-    const std::string& content_type_str);
-
 // Returns true if the |content_type| supports a resource identifier.
+// Resource identifiers are supported (but not required) for plug-ins.
 bool SupportsResourceIdentifier(ContentSettingsType content_type);
 
 // Maps CONTENT_SETTING_ASK for the CONTENT_SETTINGS_TYPE_PLUGINS to
diff --git a/chrome/browser/content_settings/host_content_settings_map.h b/chrome/browser/content_settings/host_content_settings_map.h
index 30627ab..31d7817 100644
--- a/chrome/browser/content_settings/host_content_settings_map.h
+++ b/chrome/browser/content_settings/host_content_settings_map.h
@@ -87,10 +87,8 @@ class HostContentSettingsMap
       const std::string& resource_identifier) const;
 
   // Returns a content setting |Value| which applies to the given URLs. Note
-  // that certain internal schemes are whitelisted. For ContentSettingsTypes
-  // that require a resource identifier to be specified, the
-  // |resource_identifier| must be non-empty. Ownership of the returned |Value|
-  // is transfered to the caller.
+  // that certain internal schemes are whitelisted. Ownership of the returned
+  // |Value| is transfered to the caller.
   //
   // This may be called on any thread.
   base::Value* GetContentSettingValue(
diff --git a/chrome/browser/content_settings/host_content_settings_map_unittest.cc b/chrome/browser/content_settings/host_content_settings_map_unittest.cc
index 598c103..53fce95 100644
--- a/chrome/browser/content_settings/host_content_settings_map_unittest.cc
+++ b/chrome/browser/content_settings/host_content_settings_map_unittest.cc
@@ -680,11 +680,6 @@ TEST_F(HostContentSettingsMapTest, OffTheRecord) {
 }
 
 TEST_F(HostContentSettingsMapTest, MigrateObsoletePrefs) {
-  // This feature is currently behind a flag.
-  CommandLine* cmd = CommandLine::ForCurrentProcess();
-  AutoReset<CommandLine> auto_reset(cmd, *cmd);
-  cmd->AppendSwitch(switches::kEnableResourceContentSettings);
-
   TestingProfile profile;
   PrefService* prefs = profile.GetPrefs();
 
@@ -764,20 +759,15 @@ TEST_F(HostContentSettingsMapTest, CanonicalizeExceptionsUnicodeOnly) {
 // If both Unicode and its punycode pattern exist, make sure we don't touch the
 // settings for the punycode, and that Unicode pattern gets deleted.
 TEST_F(HostContentSettingsMapTest, CanonicalizeExceptionsUnicodeAndPunycode) {
-  // This feature is currently behind a flag.
-  CommandLine* cmd = CommandLine::ForCurrentProcess();
-  AutoReset<CommandLine> auto_reset(cmd, *cmd);
-  cmd->AppendSwitch(switches::kEnableResourceContentSettings);
-
   TestingProfile profile;
 
   scoped_ptr<Value> value(base::JSONReader::Read(
-      "{\"[*.]\\xC4\\x87ira.com,*\":{\"per_plugin\":{\"pluginx\":2}}}", false));
+      "{\"[*.]\\xC4\\x87ira.com,*\":{\"images\":1}}", false));
   profile.GetPrefs()->Set(prefs::kContentSettingsPatternPairs, *value);
 
   // Set punycode equivalent, with different setting.
   scoped_ptr<Value> puny_value(base::JSONReader::Read(
-      "{\"[*.]xn--ira-ppa.com,*\":{\"per_plugin\":{\"pluginy\":2}}}", false));
+      "{\"[*.]xn--ira-ppa.com,*\":{\"images\":2}}", false));
   profile.GetPrefs()->Set(prefs::kContentSettingsPatternPairs, *puny_value);
 
   // Initialize the content map.
@@ -787,7 +777,7 @@ TEST_F(HostContentSettingsMapTest, CanonicalizeExceptionsUnicodeAndPunycode) {
       profile.GetPrefs()->GetDictionary(prefs::kContentSettingsPatternPairs);
   std::string prefs_as_json;
   base::JSONWriter::Write(content_setting_prefs, false, &prefs_as_json);
-  EXPECT_STREQ("{\"[*.]xn--ira-ppa.com,*\":{\"per_plugin\":{\"pluginy\":2}}}",
+  EXPECT_STREQ("{\"[*.]xn--ira-ppa.com,*\":{\"images\":2}}",
                prefs_as_json.c_str());
 }
 
@@ -871,7 +861,7 @@ TEST_F(HostContentSettingsMapTest, ResourceIdentifierPrefs) {
       profile.GetPrefs()->GetDictionary(prefs::kContentSettingsPatternPairs);
   std::string prefs_as_json;
   base::JSONWriter::Write(content_setting_prefs, false, &prefs_as_json);
-  EXPECT_STREQ("{}", prefs_as_json.c_str());
+  EXPECT_EQ("{}", prefs_as_json);
 
   host_content_settings_map->SetContentSetting(
       item_pattern,
@@ -883,8 +873,8 @@ TEST_F(HostContentSettingsMapTest, ResourceIdentifierPrefs) {
   content_setting_prefs =
       profile.GetPrefs()->GetDictionary(prefs::kContentSettingsPatternPairs);
   base::JSONWriter::Write(content_setting_prefs, false, &prefs_as_json);
-  EXPECT_STREQ("{\"[*.]example.com,*\":{\"per_plugin\":{\"otherplugin\":2}}}",
-               prefs_as_json.c_str());
+  EXPECT_EQ("{\"[*.]example.com,*\":{\"per_plugin\":{\"otherplugin\":2}}}",
+            prefs_as_json);
 }
 
 // If a default-content-setting is managed, the managed value should be used
diff --git a/chrome/browser/content_settings/tab_specific_content_settings.cc b/chrome/browser/content_settings/tab_specific_content_settings.cc
index 28b4376..3ba057c 100644
--- a/chrome/browser/content_settings/tab_specific_content_settings.cc
+++ b/chrome/browser/content_settings/tab_specific_content_settings.cc
@@ -6,6 +6,7 @@
 
 #include <list>
 
+#include "base/command_line.h"
 #include "base/lazy_instance.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/browsing_data_appcache_helper.h"
@@ -19,6 +20,7 @@
 #include "chrome/browser/cookies_tree_model.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_notification_types.h"
+#include "chrome/common/chrome_switches.h"
 #include "chrome/common/render_messages.h"
 #include "content/browser/renderer_host/render_process_host.h"
 #include "content/browser/renderer_host/render_view_host.h"
@@ -215,8 +217,17 @@ void TabSpecificContentSettings::OnContentBlocked(
   DCHECK(type != CONTENT_SETTINGS_TYPE_GEOLOCATION)
       << "Geolocation settings handled by OnGeolocationPermissionSet";
   content_accessed_[type] = true;
-  if (!resource_identifier.empty())
-    AddBlockedResource(type, resource_identifier);
+  // Unless UI for resource content settings is enabled, ignore the resource
+  // identifier.
+  // TODO(bauerb): The UI to unblock content should be disabled if the content
+  // setting was not set by the user.
+  std::string identifier;
+  if (CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnableResourceContentSettings)) {
+    identifier = resource_identifier;
+  }
+  if (!identifier.empty())
+    AddBlockedResource(type, identifier);
   if (!content_blocked_[type]) {
     content_blocked_[type] = true;
     // TODO: it would be nice to have a way of mocking this in tests.