Extensions: Simplify extension page CSP parsing.

This CL simplifies the CSP parsing logic for extension pages a bit. It should
introduce no behavior change.

BUG=914224

Change-Id: I5c1c2fbcc48364f0e013b16fa6d545da428193b1
Reviewed-on: https://chromium-review.googlesource.com/c/1391361
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619079}

extensions/common/api/_manifest_features.json

diff --git a/extensions/common/api/_manifest_features.json b/extensions/common/api/_manifest_features.json
index 159cbb5..8563424 100644
--- a/extensions/common/api/_manifest_features.json
+++ b/extensions/common/api/_manifest_features.json
@@ -113,12 +113,6 @@
     // app.content_security_policy whitelist).
     "extension_types": ["extension", "legacy_packaged_app"]
   },
-  "content_security_policy.extension_pages": {
-    // TODO(crbug.com/914224): Keep the channel in sync with the check in
-    // csp_info.cc.
-    "channel": "trunk",
-    "extension_types": ["extension"]
-  },
   "current_locale": {
     "channel": "stable",
     "extension_types": "all"

extensions/common/manifest_handlers/csp_info.cc

diff --git a/extensions/common/manifest_handlers/csp_info.cc b/extensions/common/manifest_handlers/csp_info.cc
index 1c33a84..7e5148b 100644
--- a/extensions/common/manifest_handlers/csp_info.cc
+++ b/extensions/common/manifest_handlers/csp_info.cc
@@ -119,68 +119,60 @@
 
 bool CSPHandler::Parse(Extension* extension, base::string16* error) {
   const std::string key = Keys()[0];
-  if (!extension->manifest()->HasPath(key))
-    return SetDefaultExtensionPagesCSP(extension);
-
   // The "content_security_policy" manifest key can either be a string or a
   // dictionary of the format
   // "content_security_policy" : {
   //     "extension_pages" : ""
   //  }
-  const base::DictionaryValue* csp_dict = nullptr;
-  std::string content_security_policy;
+  const base::Value* csp = nullptr;
+  bool result = extension->manifest()->Get(key, &csp);
+  DCHECK_EQ(result, !!csp);
 
   // TODO(crbug.com/914224): Remove the channel check once the support for the
   // dictionary key is launched to other channels.
   bool csp_dictionary_supported =
       !is_platform_app_ &&
       GetCurrentChannel() == version_info::Channel::UNKNOWN;
-  if (csp_dictionary_supported &&
-      extension->manifest()->GetDictionary(key, &csp_dict))
-    return ParseCSPDictionary(extension, error, *csp_dict);
+  if (csp_dictionary_supported && csp && csp->is_dict())
+    return ParseCSPDictionary(extension, error, *csp);
 
-  if (extension->manifest()->GetString(key, &content_security_policy)) {
-    return ParseExtensionPagesCSP(extension, error, key,
-                                  content_security_policy);
-  }
-
-  *error = GetInvalidManifestKeyError(key);
-  return false;
+  return ParseExtensionPagesCSP(extension, error, key, csp);
 }
 
 bool CSPHandler::ParseCSPDictionary(Extension* extension,
                                     base::string16* error,
                                     const base::Value& csp_dict) {
   DCHECK(csp_dict.is_dict());
-
-  auto* extension_pages_csp = csp_dict.FindKey(kExtensionPagesKey);
-  if (!extension_pages_csp)
-    return SetDefaultExtensionPagesCSP(extension);
-
-  if (!extension_pages_csp->is_string()) {
-    *error = GetInvalidManifestKeyError(kExtensionPagesPath);
-    return false;
-  }
-
   return ParseExtensionPagesCSP(extension, error, kExtensionPagesPath,
-                                extension_pages_csp->GetString());
+                                csp_dict.FindKey(kExtensionPagesKey));
 }
 
 bool CSPHandler::ParseExtensionPagesCSP(
     Extension* extension,
     base::string16* error,
     const std::string& manifest_key,
-    const std::string& content_security_policy) {
-  if (!ContentSecurityPolicyIsLegal(content_security_policy)) {
+    const base::Value* content_security_policy) {
+  if (!content_security_policy)
+    return SetDefaultExtensionPagesCSP(extension);
+
+  if (!content_security_policy->is_string()) {
     *error = GetInvalidManifestKeyError(manifest_key);
     return false;
   }
+
+  const std::string& content_security_policy_str =
+      content_security_policy->GetString();
+  if (!ContentSecurityPolicyIsLegal(content_security_policy_str)) {
+    *error = GetInvalidManifestKeyError(manifest_key);
+    return false;
+  }
+
   std::vector<InstallWarning> warnings;
   // TODO(crbug.com/914224): For manifest V3, instead of sanitizing the
   // extension provided csp value and raising install warnings, see if we want
   // to raise errors and prevent the extension from loading.
   std::string sanitized_content_security_policy = SanitizeContentSecurityPolicy(
-      content_security_policy, GetValidatorOptions(extension), &warnings);
+      content_security_policy_str, GetValidatorOptions(extension), &warnings);
   extension->AddInstallWarnings(std::move(warnings));
 
   extension->SetManifestData(

extensions/common/manifest_handlers/csp_info.h

diff --git a/extensions/common/manifest_handlers/csp_info.h b/extensions/common/manifest_handlers/csp_info.h
index 2aea0af..5f5d0b02 100644
--- a/extensions/common/manifest_handlers/csp_info.h
+++ b/extensions/common/manifest_handlers/csp_info.h
@@ -53,7 +53,7 @@
   bool ParseExtensionPagesCSP(Extension* extension,
                               base::string16* error,
                               const std::string& manifest_key,
-                              const std::string& content_security_policy);
+                              const base::Value* content_security_policy);
 
   // Sets the default CSP value for the extension.
   bool SetDefaultExtensionPagesCSP(Extension* extension);