content-settings: add devtools:// to ContentSettingsPattern::SchemeType

This CL adds SchemeType::SCHEME_DEVTOOLS to ContentSettingsPattern, so
it's easier to handle auto-granted permissions registered with
WebUIAllowlist in site settings.

This is a followup based on discussions in https://crrev.com/c/2262758.

Fixed: 1101889
Change-Id: I761a8df60f4fc05c1f92e0ca9761be749b29b88d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2279346Reviewed-by: calamity <calamity@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Commit-Queue: Jiewei Qian  <qjw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#790250}

chrome/browser/ui/webui/settings/site_settings_helper.cc

@@ -10,11 +10,8 @@
 #include <string>
 
 #include "base/feature_list.h"
-#include "base/no_destructor.h"
 #include "base/stl_util.h"
-#include "base/strings/strcat.h"
 #include "base/strings/string16.h"
-#include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/bluetooth/bluetooth_chooser_context.h"
@@ -47,7 +44,6 @@
 #include "extensions/browser/extension_registry.h"
 #include "extensions/common/constants.h"
 #include "url/origin.h"
-#include "url/url_constants.h"
 
 namespace site_settings {
 
@@ -199,13 +195,6 @@ static_assert(base::size(kPolicyIndicatorTypeStringMapping) ==
               "kPolicyIndicatorStringMapping should have "
               "PolicyIndicatorType::kNumIndicators elements");
 
-const std::string& GetDevtoolsPatternPrefix() {
-  static const base::NoDestructor<std::string> kDevtoolsPatternPrefix(
-      base::StrCat({content_settings::kChromeDevToolsScheme,
-                    url::kStandardSchemeSeparator}));
-  return *kDevtoolsPatternPrefix;
-}
-
 // Retrieves the corresponding string, according to the following precedence
 // order from highest to lowest priority:
 //    1. Allowlisted WebUI content setting.
@@ -306,9 +295,8 @@ bool PatternAppliesToWebUISchemes(const ContentSettingPatternSource& pattern) {
              ContentSettingsPattern::SchemeType::SCHEME_CHROME ||
          pattern.primary_pattern.GetScheme() ==
              ContentSettingsPattern::SchemeType::SCHEME_CHROMEUNTRUSTED ||
-         base::StartsWith(pattern.primary_pattern.ToString(),
-                          GetDevtoolsPatternPrefix(),
-                          base::CompareCase::INSENSITIVE_ASCII);
+         pattern.primary_pattern.GetScheme() ==
+             ContentSettingsPattern::SchemeType::SCHEME_DEVTOOLS;
 }
 
 // Retrieves the source of a chooser exception as a string. This method uses the

chrome/browser/ui/webui/webui_allowlist_provider_unittest.cc

@@ -224,3 +224,32 @@ TEST_F(WebUIAllowlistProviderTest, OnlyNotifyOnChange) {
                                            ContentSettingsType::GEOLOCATION);
   EXPECT_EQ(3U, change_observer.change_counter());
 }
+
+TEST_F(WebUIAllowlistProviderTest, RegisterDevtools) {
+  auto* map = GetHostContentSettingsMap(profile());
+  map->SetDefaultContentSetting(ContentSettingsType::BLUETOOTH_GUARD,
+                                CONTENT_SETTING_BLOCK);
+
+  // Check |url_allowed| is not affected by allowlisted_schemes. This mechanism
+  // take precedence over allowlist provider.
+  const GURL url_allowed = GURL("devtools://devtools");
+  ASSERT_EQ(CONTENT_SETTING_BLOCK,
+            map->GetContentSetting(url_allowed, url_allowed,
+                                   ContentSettingsType::BLUETOOTH_GUARD,
+                                   std::string()));
+
+  auto* allowlist = WebUIAllowlist::GetOrCreate(profile());
+  allowlist->RegisterAutoGrantedPermission(
+      url::Origin::Create(url_allowed), ContentSettingsType::BLUETOOTH_GUARD);
+
+  EXPECT_EQ(CONTENT_SETTING_ALLOW,
+            map->GetContentSetting(url_allowed, url_allowed,
+                                   ContentSettingsType::BLUETOOTH_GUARD,
+                                   std::string()));
+
+  const GURL url_no_permission_webui = GURL("devtools://other");
+  EXPECT_EQ(CONTENT_SETTING_BLOCK,
+            map->GetContentSetting(
+                url_no_permission_webui, url_no_permission_webui,
+                ContentSettingsType::BLUETOOTH_GUARD, std::string()));
+}

components/content_settings/core/common/content_settings_pattern.cc

@@ -31,10 +31,11 @@ size_t g_non_domain_wildcard_non_port_schemes_count = 0;
 // Keep it consistent with enum SchemeType in content_settings_pattern.h.
 // TODO(msramek): Layering violation: assemble this array from hardcoded
 // schemes and those injected via |SetNonWildcardDomainNonPortSchemes()|.
-const char* const kSchemeNames[] = {
-    "wildcard",        "other",          url::kHttpScheme,
-    url::kHttpsScheme, url::kFileScheme, "chrome-extension",
-    "chrome-search",   "chrome",         "chrome-untrusted"};
+const char* const kSchemeNames[] = {"wildcard",         "other",
+                                    url::kHttpScheme,   url::kHttpsScheme,
+                                    url::kFileScheme,   "chrome-extension",
+                                    "chrome-search",    "chrome",
+                                    "chrome-untrusted", "devtools"};
 
 static_assert(base::size(kSchemeNames) == ContentSettingsPattern::SCHEME_MAX,
               "kSchemeNames should have SCHEME_MAX elements");

components/content_settings/core/common/content_settings_pattern.h

@@ -80,6 +80,7 @@ class ContentSettingsPattern {
     SCHEME_CHROMESEARCH,
     SCHEME_CHROME,
     SCHEME_CHROMEUNTRUSTED,
+    SCHEME_DEVTOOLS,
     SCHEME_MAX,
   };
 

components/content_settings/core/common/content_settings_pattern_unittest.cc

@@ -88,6 +88,15 @@ TEST(ContentSettingsPatternTest, FromURL) {
   pattern = ContentSettingsPattern::FromURL(GURL("file:///foo/bar.html"));
   EXPECT_TRUE(pattern.IsValid());
   EXPECT_EQ("file:///foo/bar.html", pattern.ToString());
+
+  // WebUI schemes can't be created by FromURL, because they have no port and
+  // can't have domain wildcard.
+  pattern = ContentSettingsPattern::FromURL(GURL("chrome://test"));
+  EXPECT_FALSE(pattern.IsValid());
+  pattern = ContentSettingsPattern::FromURL(GURL("chrome-untrusted://test"));
+  EXPECT_FALSE(pattern.IsValid());
+  pattern = ContentSettingsPattern::FromURL(GURL("devtools://devtools"));
+  EXPECT_FALSE(pattern.IsValid());
 }
 
 TEST(ContentSettingsPatternTest, FilesystemUrls) {
@@ -179,6 +188,20 @@ TEST(ContentSettingsPatternTest, FromURLNoWildcard) {
       GURL("filesystem:https://www.google.com:81/temporary/")));
   EXPECT_FALSE(pattern.Matches(
       GURL("filesystem:https://foo.www.google.com/temporary/")));
+
+  pattern = ContentSettingsPattern::FromURLNoWildcard(GURL("chrome://test"));
+  EXPECT_TRUE(pattern.IsValid());
+  EXPECT_TRUE(pattern.Matches(GURL("chrome://test")));
+
+  pattern = ContentSettingsPattern::FromURLNoWildcard(
+      GURL("chrome-untrusted://test"));
+  EXPECT_TRUE(pattern.IsValid());
+  EXPECT_TRUE(pattern.Matches(GURL("chrome-untrusted://test")));
+
+  pattern =
+      ContentSettingsPattern::FromURLNoWildcard(GURL("devtools://devtools"));
+  EXPECT_TRUE(pattern.IsValid());
+  EXPECT_TRUE(pattern.Matches(GURL("devtools://devtools")));
 }
 
 // The static Wildcard() method goes through a fast path and avoids the Builder
@@ -480,6 +503,18 @@ TEST(ContentSettingsPatternTest, FromString_Canonicalized) {
                Pattern("file:///tmp/bar/../test.html").ToString().c_str());
 }
 
+TEST(ContentSettingsPatternTest, FromString_WebUISchemes) {
+  const char* patterns[] = {"chrome://test/", "chrome-untrusted://test/",
+                            "devtools://devtools/"};
+
+  for (const char* pattern_str : patterns) {
+    ContentSettingsPattern pattern = Pattern(pattern_str);
+    EXPECT_TRUE(pattern.IsValid());
+    EXPECT_EQ(pattern_str, pattern.ToString());
+    EXPECT_TRUE(pattern.Matches(GURL(pattern_str)));
+  }
+}
+
 TEST(ContentSettingsPatternTest, InvalidPatterns) {
   // StubObserver expects an empty pattern top be returned as empty string.
   EXPECT_FALSE(ContentSettingsPattern().IsValid());
@@ -800,6 +835,8 @@ TEST(ContentSettingsPatternTest, Schemes) {
             Pattern("chrome://sample/").GetScheme());
   EXPECT_EQ(ContentSettingsPattern::SCHEME_CHROMEUNTRUSTED,
             Pattern("chrome-untrusted://sample/").GetScheme());
+  EXPECT_EQ(ContentSettingsPattern::SCHEME_DEVTOOLS,
+            Pattern("devtools://devtools/").GetScheme());
 }
 
 TEST(ContentSettingsPatternTest, FileSchemeHasPath) {

components/test/components_test_suite.cc

@@ -43,7 +43,8 @@ namespace {
 // Not using kExtensionScheme and kChromeSearchScheme to avoid the dependency
 // to extensions and chrome/common.
 const char* const kNonWildcardDomainNonPortSchemes[] = {
-    "chrome-extension", "chrome-search", "chrome", "chrome-untrusted"};
+    "chrome-extension", "chrome-search", "chrome", "chrome-untrusted",
+    "devtools"};
 
 class ComponentsTestSuite : public base::TestSuite {
  public:
@@ -75,6 +76,7 @@ class ComponentsTestSuite : public base::TestSuite {
     }
 #else
     url::AddStandardScheme("chrome", url::SCHEME_WITH_HOST);
+    url::AddStandardScheme("chrome-untrusted", url::SCHEME_WITH_HOST);
     url::AddStandardScheme("devtools", url::SCHEME_WITH_HOST);
 
 #endif