Extensions: Always use CSP dictionary on Manifest V3.

Currently the "content_security_policy" manifest key is restricted to trunk.
However it is mandatory for Mv3 extensions. Hence, always allow it if the
manifest version is 3.

Also, make some other minor cosmetic changes.

BUG=993530

Change-Id: Ifca0c588d923a36644002e865f853d74533d7356
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1815277
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#698986}

extensions/common/manifest_handlers/csp_info.cc

@@ -125,8 +125,8 @@ const std::string& CSPInfo::GetExtensionPagesCSP(const Extension* extension) {
 
 // static
 const std::string* CSPInfo::GetIsolatedWorldCSP(const Extension& extension) {
-  // TODO(crbug.com/914224): This should be only called for extensions which can
-  // have isolated worlds. Figure out the case of TYPE_USER_SCRIPT and add
+  // TODO(crbug.com/1005978): This should be only called for extensions which
+  // can have isolated worlds. Figure out the case of TYPE_USER_SCRIPT and add
   // DCHECK(csp_info).
   CSPInfo* csp_info = static_cast<CSPInfo*>(
       extension.GetManifestData(keys::kContentSecurityPolicy));
@@ -163,19 +163,24 @@ bool CSPHandler::Parse(Extension* extension, base::string16* error) {
   // The "content_security_policy" manifest key can either be a string or a
   // dictionary of the format
   // "content_security_policy" : {
-  //     "extension_pages" : ""
+  //     "extension_pages": "",
+  //     "sandbox": "",
+  //     "isolated_world": ""
   //  }
   const base::Value* csp = GetManifestPath(extension, key);
+  const int kManifestVersion3 = 3;
 
-  // TODO(crbug.com/914224): Remove the channel check once the support for the
-  // dictionary key is launched to other channels.
+  // TODO(crbug.com/914224): Remove the channel check once support for isolated
+  // world CSP is implemenented.
   bool csp_dictionary_supported =
       extension->GetType() == Manifest::TYPE_EXTENSION &&
-      GetCurrentChannel() == version_info::Channel::UNKNOWN;
+      (extension->manifest_version() >= kManifestVersion3 ||
+       GetCurrentChannel() == version_info::Channel::UNKNOWN);
 
   if (csp_dictionary_supported) {
-    // CSP key as dictionary is mandatory for manifest v3 extensions.
-    if (extension->manifest_version() == 3) {
+    // CSP key as dictionary is mandatory for manifest v3 (and above)
+    // extensions.
+    if (extension->manifest_version() >= kManifestVersion3) {
       if (csp && !csp->is_dict()) {
         *error = GetInvalidManifestKeyError(key);
         return false;
@@ -204,14 +209,6 @@ bool CSPHandler::Parse(Extension* extension, base::string16* error) {
 
 bool CSPHandler::ParseCSPDictionary(Extension* extension,
                                     base::string16* error) {
-  if (!ParseExtensionPagesCSP(
-          extension, error, keys::kContentSecurityPolicy_ExtensionPagesPath,
-          true /* secure_only */,
-          GetManifestPath(extension,
-                          keys::kContentSecurityPolicy_ExtensionPagesPath))) {
-    return false;
-  }
-
   // keys::kSandboxedPagesCSP shouldn't be used when using
   // keys::kContentSecurityPolicy as a dictionary.
   if (extension->manifest()->HasPath(keys::kSandboxedPagesCSP)) {
@@ -219,7 +216,12 @@ bool CSPHandler::ParseCSPDictionary(Extension* extension,
     return false;
   }
 
-  return ParseSandboxCSP(
+  return ParseExtensionPagesCSP(
+             extension, error, keys::kContentSecurityPolicy_ExtensionPagesPath,
+             true /* secure_only */,
+             GetManifestPath(
+                 extension, keys::kContentSecurityPolicy_ExtensionPagesPath)) &&
+         ParseSandboxCSP(
              extension, error, keys::kContentSecurityPolicy_SandboxedPagesPath,
              GetManifestPath(
                  extension, keys::kContentSecurityPolicy_SandboxedPagesPath)) &&
@@ -376,7 +378,8 @@ void CSPHandler::SetSandboxCSP(Extension* extension, std::string sandbox_csp) {
 }
 
 bool CSPHandler::AlwaysParseForType(Manifest::Type type) const {
-  // TODO(karandeepb): Check if TYPE_USER_SCRIPT needs to be included here.
+  // TODO(crbug.com/1005978): Check if TYPE_USER_SCRIPT needs to be included
+  // here.
   return type == Manifest::TYPE_PLATFORM_APP ||
          type == Manifest::TYPE_EXTENSION ||
          type == Manifest::TYPE_LEGACY_PACKAGED_APP;

extensions/common/manifest_handlers/csp_info.h

@@ -64,8 +64,8 @@ class CSPHandler : public ManifestHandler {
   CSPHandler();
   ~CSPHandler() override;
 
+  // ManifestHandler override:
   bool Parse(Extension* extension, base::string16* error) override;
-  bool AlwaysParseForType(Manifest::Type type) const override;
 
  private:
   // Parses the "content_security_policy" dictionary in the manifest.
@@ -103,6 +103,8 @@ class CSPHandler : public ManifestHandler {
   // Helper to set the sandbox content security policy manifest data.
   void SetSandboxCSP(Extension* extension, std::string sandbox_csp);
 
+  // ManifestHandler overrides:
+  bool AlwaysParseForType(Manifest::Type type) const override;
   base::span<const char* const> Keys() const override;
 
   DISALLOW_COPY_AND_ASSIGN(CSPHandler);

extensions/common/manifest_handlers/csp_info_unittest.cc

@@ -266,8 +266,6 @@ TEST_F(CSPInfoUnitTest, CSPDictionary_IsolatedWorlds) {
 // key is mandatory for manifest v3 extensions and that defaults are applied
 // correctly.
 TEST_F(CSPInfoUnitTest, CSPDictionaryMandatoryForV3) {
-  ScopedCurrentChannel channel(version_info::Channel::UNKNOWN);
-
   LoadAndExpectError("csp_invalid_type_v3.json",
                      GetInvalidManifestKeyError(keys::kContentSecurityPolicy));