Only activate radio buttons in ContentSettingsBubble for valid content...

Only activate radio buttons in ContentSettingsBubble for valid content settings. This prevents failed DCHECK e.g. for about:blank if you try to create popups from the console.

BUG=244176

Review-Url: https://codereview.chromium.org/2510363003
Cr-Commit-Position: refs/heads/master@{#436285}

chrome/browser/content_settings/host_content_settings_map_unittest.cc

@@ -1801,3 +1801,19 @@ TEST_F(HostContentSettingsMapTest, ClearSettingsForOneTypeWithPredicate) {
   EXPECT_EQ(ContentSettingsPattern::FromURLNoWildcard(url1),
             host_settings[0].primary_pattern);
 }
+
+TEST_F(HostContentSettingsMapTest, CanSetNarrowestSetting) {
+  TestingProfile profile;
+  const auto* map = HostContentSettingsMapFactory::GetForProfile(&profile);
+
+  GURL valid_url("http://google.com");
+  EXPECT_TRUE(map->CanSetNarrowestContentSetting(
+      valid_url, valid_url,
+      CONTENT_SETTINGS_TYPE_POPUPS));
+
+  GURL invalid_url("about:blank");
+  EXPECT_FALSE(map->CanSetNarrowestContentSetting(
+      invalid_url, invalid_url,
+      CONTENT_SETTINGS_TYPE_POPUPS));
+}
+

chrome/browser/ui/content_settings/content_setting_bubble_model.cc

@@ -401,7 +401,12 @@ void ContentSettingSingleRadioGroup::SetRadioGroup() {
       setting == CONTENT_SETTING_BLOCK &&
       PluginUtils::ShouldPreferHtmlOverPlugins(
           HostContentSettingsMapFactory::GetForProfile(profile()));
-  set_radio_group_enabled(setting_source == SETTING_SOURCE_USER &&
+
+  const auto* map = HostContentSettingsMapFactory::GetForProfile(profile());
+  // Prevent creation of content settings for illegal urls like about:blank
+  bool is_valid = map->CanSetNarrowestContentSetting(url, url, content_type());
+
+  set_radio_group_enabled(is_valid && setting_source == SETTING_SOURCE_USER &&
                           !flash_hidden_from_plugin_list);
 
   selected_item_ = radio_group.default_item;

chrome/browser/ui/content_settings/content_setting_bubble_model_unittest.cc

@@ -82,6 +82,8 @@ class ContentSettingBubbleModelTest : public ChromeRenderViewHostTestHarness {
 };
 
 TEST_F(ContentSettingBubbleModelTest, ImageRadios) {
+  WebContentsTester::For(web_contents())->
+      NavigateAndCommit(GURL("https://www.example.com"));
   TabSpecificContentSettings* content_settings =
       TabSpecificContentSettings::FromWebContents(web_contents());
   content_settings->OnContentBlocked(CONTENT_SETTINGS_TYPE_IMAGES);
@@ -99,6 +101,8 @@ TEST_F(ContentSettingBubbleModelTest, ImageRadios) {
 }
 
 TEST_F(ContentSettingBubbleModelTest, Cookies) {
+  WebContentsTester::For(web_contents())->
+      NavigateAndCommit(GURL("https://www.example.com"));
   TabSpecificContentSettings* content_settings =
       TabSpecificContentSettings::FromWebContents(web_contents());
   content_settings->OnContentBlocked(CONTENT_SETTINGS_TYPE_COOKIES);
@@ -713,6 +717,8 @@ TEST_F(ContentSettingBubbleModelTest, AccumulateMediastreamMicAndCamera) {
 }
 
 TEST_F(ContentSettingBubbleModelTest, Plugins) {
+  WebContentsTester::For(web_contents())->
+      NavigateAndCommit(GURL("https://www.example.com"));
   TabSpecificContentSettings* content_settings =
       TabSpecificContentSettings::FromWebContents(web_contents());
   const base::string16 plugin_name = base::ASCIIToUTF16("plugin_name");
@@ -737,6 +743,8 @@ TEST_F(ContentSettingBubbleModelTest, Plugins) {
 }
 
 TEST_F(ContentSettingBubbleModelTest, PepperBroker) {
+  WebContentsTester::For(web_contents())->
+      NavigateAndCommit(GURL("https://www.example.com"));
   TabSpecificContentSettings* content_settings =
       TabSpecificContentSettings::FromWebContents(web_contents());
   content_settings->OnContentBlocked(CONTENT_SETTINGS_TYPE_PPAPI_BROKER);
@@ -953,3 +961,37 @@ TEST_F(ContentSettingBubbleModelTest, SubresourceFilter) {
       l10n_util::GetStringUTF8(IDS_FILTERED_DECEPTIVE_CONTENT_PROMPT_RELOAD));
   EXPECT_EQ(0U, bubble_content.media_menus.size());
 }
+
+TEST_F(ContentSettingBubbleModelTest, ValidUrl) {
+  WebContentsTester::For(web_contents())->
+      NavigateAndCommit(GURL("https://www.example.com"));
+
+  TabSpecificContentSettings* content_settings =
+      TabSpecificContentSettings::FromWebContents(web_contents());
+  content_settings->OnContentBlocked(CONTENT_SETTINGS_TYPE_COOKIES);
+
+  std::unique_ptr<ContentSettingBubbleModel> content_setting_bubble_model(
+      ContentSettingBubbleModel::CreateContentSettingBubbleModel(
+          NULL, web_contents(), profile(), CONTENT_SETTINGS_TYPE_COOKIES));
+  const ContentSettingBubbleModel::BubbleContent& bubble_content =
+      content_setting_bubble_model->bubble_content();
+
+  EXPECT_TRUE(bubble_content.radio_group_enabled);
+}
+
+TEST_F(ContentSettingBubbleModelTest, InvalidUrl) {
+  WebContentsTester::For(web_contents())->
+      NavigateAndCommit(GURL("about:blank"));
+
+  TabSpecificContentSettings* content_settings =
+      TabSpecificContentSettings::FromWebContents(web_contents());
+  content_settings->OnContentBlocked(CONTENT_SETTINGS_TYPE_COOKIES);
+
+  std::unique_ptr<ContentSettingBubbleModel> content_setting_bubble_model(
+      ContentSettingBubbleModel::CreateContentSettingBubbleModel(
+          NULL, web_contents(), profile(), CONTENT_SETTINGS_TYPE_COOKIES));
+  const ContentSettingBubbleModel::BubbleContent& bubble_content =
+      content_setting_bubble_model->bubble_content();
+
+  EXPECT_FALSE(bubble_content.radio_group_enabled);
+}

components/content_settings/core/browser/host_content_settings_map.cc

@@ -161,6 +161,18 @@ content_settings::PatternPair GetPatternsFromScopingType(
   return patterns;
 }
 
+content_settings::PatternPair GetPatternsForContentSettingsType(
+    const GURL& primary_url,
+    const GURL& secondary_url,
+    ContentSettingsType type) {
+  const WebsiteSettingsInfo* website_settings_info =
+      content_settings::WebsiteSettingsRegistry::GetInstance()->Get(type);
+  DCHECK(website_settings_info);
+  content_settings::PatternPair patterns = GetPatternsFromScopingType(
+      website_settings_info->scoping_type(), primary_url, secondary_url);
+  return patterns;
+}
+
 }  // namespace
 
 HostContentSettingsMap::HostContentSettingsMap(PrefService* prefs,
@@ -349,11 +361,8 @@ void HostContentSettingsMap::SetWebsiteSettingDefaultScope(
     ContentSettingsType content_type,
     const std::string& resource_identifier,
     std::unique_ptr<base::Value> value) {
-  const WebsiteSettingsInfo* info =
-      content_settings::WebsiteSettingsRegistry::GetInstance()->Get(
-          content_type);
-  content_settings::PatternPair patterns = GetPatternsFromScopingType(
-      info->scoping_type(), primary_url, secondary_url);
+  content_settings::PatternPair patterns = GetPatternsForContentSettingsType(
+      primary_url, secondary_url, content_type);
   ContentSettingsPattern primary_pattern = patterns.first;
   ContentSettingsPattern secondary_pattern = patterns.second;
   if (!primary_pattern.IsValid() || !secondary_pattern.IsValid())
@@ -385,11 +394,34 @@ void HostContentSettingsMap::SetWebsiteSettingCustomScope(
   NOTREACHED();
 }
 
+bool HostContentSettingsMap::CanSetNarrowestContentSetting(
+    const GURL& primary_url,
+    const GURL& secondary_url,
+    ContentSettingsType type) const {
+  content_settings::PatternPair patterns =
+      GetNarrowestPatterns(primary_url, secondary_url, type);
+  return patterns.first.IsValid() && patterns.second.IsValid();
+}
+
 void HostContentSettingsMap::SetNarrowestContentSetting(
     const GURL& primary_url,
     const GURL& secondary_url,
     ContentSettingsType type,
     ContentSetting setting) {
+  content_settings::PatternPair patterns =
+      GetNarrowestPatterns(primary_url, secondary_url, type);
+
+  if (!patterns.first.IsValid() || !patterns.second.IsValid())
+    return;
+
+  SetContentSettingCustomScope(patterns.first, patterns.second, type,
+                               std::string(), setting);
+}
+
+content_settings::PatternPair HostContentSettingsMap::GetNarrowestPatterns (
+    const GURL& primary_url,
+    const GURL& secondary_url,
+    ContentSettingsType type) const {
   // Permission settings are specified via rules. There exists always at least
   // one rule for the default setting. Get the rule that currently defines
   // the permission for the given permission |type|. Then test whether the
@@ -401,29 +433,23 @@ void HostContentSettingsMap::SetNarrowestContentSetting(
       primary_url, secondary_url, type, std::string(), &info);
   DCHECK_EQ(content_settings::SETTING_SOURCE_USER, info.source);
 
-  const WebsiteSettingsInfo* website_settings_info =
-      content_settings::WebsiteSettingsRegistry::GetInstance()->Get(type);
-  content_settings::PatternPair patterns = GetPatternsFromScopingType(
-      website_settings_info->scoping_type(), primary_url, secondary_url);
-
-  ContentSettingsPattern narrow_primary = patterns.first;
-  ContentSettingsPattern narrow_secondary = patterns.second;
+  content_settings::PatternPair patterns = GetPatternsForContentSettingsType(
+      primary_url, secondary_url, type);
 
   ContentSettingsPattern::Relation r1 =
       info.primary_pattern.Compare(patterns.first);
   if (r1 == ContentSettingsPattern::PREDECESSOR) {
-    narrow_primary = info.primary_pattern;
+    patterns.first = info.primary_pattern;
   } else if (r1 == ContentSettingsPattern::IDENTITY) {
     ContentSettingsPattern::Relation r2 =
         info.secondary_pattern.Compare(patterns.second);
     DCHECK(r2 != ContentSettingsPattern::DISJOINT_ORDER_POST &&
            r2 != ContentSettingsPattern::DISJOINT_ORDER_PRE);
     if (r2 == ContentSettingsPattern::PREDECESSOR)
-      narrow_secondary = info.secondary_pattern;
+      patterns.second = info.secondary_pattern;
   }
 
-  SetContentSettingCustomScope(narrow_primary, narrow_secondary, type,
-                               std::string(), setting);
+  return patterns;
 }
 
 void HostContentSettingsMap::SetContentSettingCustomScope(
@@ -458,14 +484,9 @@ void HostContentSettingsMap::SetContentSettingDefaultScope(
     ContentSettingsType content_type,
     const std::string& resource_identifier,
     ContentSetting setting) {
-  const ContentSettingsInfo* info =
-      content_settings::ContentSettingsRegistry::GetInstance()->Get(
-          content_type);
-  DCHECK(info);
+  content_settings::PatternPair patterns = GetPatternsForContentSettingsType(
+      primary_url, secondary_url, content_type);
 
-  content_settings::PatternPair patterns =
-      GetPatternsFromScopingType(info->website_settings_info()->scoping_type(),
-                                 primary_url, secondary_url);
   ContentSettingsPattern primary_pattern = patterns.first;
   ContentSettingsPattern secondary_pattern = patterns.second;
   if (!primary_pattern.IsValid() || !secondary_pattern.IsValid())

components/content_settings/core/browser/host_content_settings_map.h

@@ -196,6 +196,13 @@ class HostContentSettingsMap : public content_settings::Observer,
       const std::string& resource_identifier,
       std::unique_ptr<base::Value> value);
 
+  // Check if a call to SetNarrowestContentSetting would succeed or if it would
+  // fail because of an invalid pattern.
+  bool CanSetNarrowestContentSetting(
+      const GURL& primary_url,
+      const GURL& secondary_url,
+      ContentSettingsType type) const;
+
   // Sets the most specific rule that currently defines the setting for the
   // given content type. TODO(raymes): Remove this once all content settings
   // are scoped to origin scope. There is no scope more narrow than origin
@@ -364,6 +371,11 @@ class HostContentSettingsMap : public content_settings::Observer,
       const std::string& resource_identifier,
       content_settings::SettingInfo* info) const;
 
+  content_settings::PatternPair GetNarrowestPatterns(
+      const GURL& primary_url,
+      const GURL& secondary_url,
+      ContentSettingsType type) const;
+
   static std::unique_ptr<base::Value> GetContentSettingValueAndPatterns(
       const content_settings::ProviderInterface* provider,
       const GURL& primary_url,