Add DCHECKS to ensure content settings API isn't used with resource ids

Using resource ids with the content settings API is supported but is
apparently not being used. This change adds some DCHECKS to check that
this is true as preparation for removing resource ids from the API.

Bug: 754178
Change-Id: I6ac49bf697fc342332905c7e814ef4e61f32695f
Reviewed-on: https://chromium-review.googlesource.com/790075
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Raymes Khoury <raymes@chromium.org>
Commit-Queue: Ben Wells <benwells@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521243}

chrome/browser/content_settings/content_settings_pref_provider_unittest.cc

@@ -345,39 +345,6 @@ TEST_F(PrefProviderTest, Patterns) {
   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 /* incognito */,
-                                              true /* store_last_modified */);
-
-  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,
-                                         resource1, false));
-  pref_content_settings_provider.SetWebsiteSetting(
-      pattern, pattern, CONTENT_SETTINGS_TYPE_PLUGINS, resource1,
-      new base::Value(CONTENT_SETTING_BLOCK));
-  EXPECT_EQ(CONTENT_SETTING_BLOCK,
-            TestUtils::GetContentSetting(&pref_content_settings_provider, host,
-                                         host, CONTENT_SETTINGS_TYPE_PLUGINS,
-                                         resource1, false));
-  EXPECT_EQ(CONTENT_SETTING_DEFAULT,
-            TestUtils::GetContentSetting(&pref_content_settings_provider, host,
-                                         host, CONTENT_SETTINGS_TYPE_PLUGINS,
-                                         resource2, false));
-
-  pref_content_settings_provider.ShutdownOnUIThread();
-}
-#endif
-
 // http://crosbug.com/17760
 TEST_F(PrefProviderTest, Deadlock) {
   sync_preferences::TestingPrefServiceSyncable prefs;
@@ -477,7 +444,6 @@ TEST_F(PrefProviderTest, ClearAllContentSettingsRules) {
   ContentSettingsPattern wildcard =
       ContentSettingsPattern::FromString("*");
   std::unique_ptr<base::Value> value(new base::Value(CONTENT_SETTING_ALLOW));
-  ResourceIdentifier res_id("abcde");
 
   PrefProvider provider(&prefs, false /* incognito */,
                         true /* store_last_modified */);
@@ -494,11 +460,7 @@ TEST_F(PrefProviderTest, ClearAllContentSettingsRules) {
 #if BUILDFLAG(ENABLE_PLUGINS)
   // Non-empty pattern, plugins, non-empty resource identifier.
   provider.SetWebsiteSetting(pattern, wildcard, CONTENT_SETTINGS_TYPE_PLUGINS,
-                             res_id, value->DeepCopy());
-
-  // Empty pattern, plugins, non-empty resource identifier.
-  provider.SetWebsiteSetting(wildcard, wildcard, CONTENT_SETTINGS_TYPE_PLUGINS,
-                             res_id, value->DeepCopy());
+                             ResourceIdentifier(), value->DeepCopy());
 #endif
   // Non-empty pattern, syncable, empty resource identifier.
   provider.SetWebsiteSetting(pattern, wildcard, CONTENT_SETTINGS_TYPE_COOKIES,

chrome/browser/plugins/plugin_info_host_impl_unittest.cc

@@ -324,37 +324,20 @@ TEST_F(PluginInfoHostImplTest, GetPluginContentSetting) {
       host, GURL(), CONTENT_SETTINGS_TYPE_PLUGINS, std::string(),
       CONTENT_SETTING_DETECT_IMPORTANT_CONTENT);
 
-  // Allow plugin "foo" on all sites.
-  map->SetContentSettingCustomScope(
-      ContentSettingsPattern::Wildcard(), ContentSettingsPattern::Wildcard(),
-      CONTENT_SETTINGS_TYPE_PLUGINS, "foo", CONTENT_SETTING_ALLOW);
-
   GURL unmatched_host("https://www.google.com");
-  ASSERT_EQ(
+  EXPECT_EQ(
       CONTENT_SETTING_BLOCK,
       map->GetContentSetting(unmatched_host, unmatched_host,
                              CONTENT_SETTINGS_TYPE_PLUGINS, std::string()));
-  ASSERT_EQ(CONTENT_SETTING_DETECT_IMPORTANT_CONTENT,
+  EXPECT_EQ(CONTENT_SETTING_DETECT_IMPORTANT_CONTENT,
             map->GetContentSetting(host, host, CONTENT_SETTINGS_TYPE_PLUGINS,
                                    std::string()));
-  ASSERT_EQ(
-      CONTENT_SETTING_ALLOW,
-      map->GetContentSetting(host, host, CONTENT_SETTINGS_TYPE_PLUGINS, "foo"));
-  ASSERT_EQ(
-      CONTENT_SETTING_DEFAULT,
-      map->GetContentSetting(host, host, CONTENT_SETTINGS_TYPE_PLUGINS, "bar"));
-
-  // "foo" is allowed everywhere.
-  VerifyPluginContentSetting(host, "foo", CONTENT_SETTING_ALLOW, false, false);
-
-  // There is no specific content setting for "bar", so the general setting
-  // for example.com applies.
-  VerifyPluginContentSetting(
-      host, "bar", CONTENT_SETTING_DETECT_IMPORTANT_CONTENT, false, false);
-
-  // Otherwise, use the default.
-  VerifyPluginContentSetting(unmatched_host, "bar", CONTENT_SETTING_BLOCK, true,
+
+  VerifyPluginContentSetting(host, std::string(),
+                             CONTENT_SETTING_DETECT_IMPORTANT_CONTENT, false,
                              false);
+  VerifyPluginContentSetting(unmatched_host, std::string(),
+                             CONTENT_SETTING_BLOCK, false, false);
 
   // Block plugins via policy.
   sync_preferences::TestingPrefServiceSyncable* prefs =
@@ -363,8 +346,8 @@ TEST_F(PluginInfoHostImplTest, GetPluginContentSetting) {
                         base::MakeUnique<base::Value>(CONTENT_SETTING_BLOCK));
 
   // All plugins should be blocked now.
-  VerifyPluginContentSetting(host, "foo", CONTENT_SETTING_BLOCK, true, true);
-  VerifyPluginContentSetting(host, "bar", CONTENT_SETTING_BLOCK, true, true);
-  VerifyPluginContentSetting(unmatched_host, "bar", CONTENT_SETTING_BLOCK, true,
+  VerifyPluginContentSetting(host, std::string(), CONTENT_SETTING_BLOCK, true,
                              true);
+  VerifyPluginContentSetting(unmatched_host, std::string(),
+                             CONTENT_SETTING_BLOCK, true, true);
 }

components/content_settings/core/browser/content_settings_pref.cc

@@ -118,8 +118,12 @@ 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());
+
+  // Resource Identifiers have been supported by the API but never used by any
+  // users of the API.
+  // TODO(crbug.com/754178): remove |resource_identifier| from the API.
+  DCHECK(resource_identifier.empty());
 
   // At this point take the ownership of the |in_value|.
   std::unique_ptr<base::Value> value(in_value);
@@ -273,6 +277,11 @@ void ContentSettingsPref::ReadContentSettingsFromPref() {
       const base::DictionaryValue* resource_dictionary = nullptr;
       if (settings_dictionary->GetDictionary(
               kPerResourceIdentifierPrefName, &resource_dictionary)) {
+        // Resource Identifiers have been supported by the API but never used by
+        // any users of the API.
+        // TODO(crbug.com/754178): remove |resource_identifier| from the API.
+        NOTREACHED();
+
         base::Time last_modified = GetTimeStamp(settings_dictionary);
         for (base::DictionaryValue::Iterator j(*resource_dictionary);
              !j.IsAtEnd();