PWA: Send manifest policy to PermissionsPolicy before serializing.

This change takes the manifest parsed policy and sends it to the
PermissionsPolicyParser, which better handles input validation
and other concepts specific to Permissions Policy. The returned
value is then stored in the Web App Provider system.

 * Remove blink::Manifest::PermissionsPolicyDeclaration in favor of
   blink::ParsedPermissionsPolicyDeclaration.
 * Refactor ManifestParser to take an ExecutionContext instead of
   FeatureContext from ManifestManager, which is required in order to
   to use the PermissionsPolicyParser method.

Bug: 1238341
Change-Id: Ic874bf4f89beb72f781645c32ab0d8c202ad494a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3415495
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Jarryd Goodman <jarrydg@chromium.org>
Auto-Submit: Jarryd Goodman <jarrydg@chromium.org>
Cr-Commit-Position: refs/heads/main@{#980643}
NOKEYCHECK=True
GitOrigin-RevId: 496743cd784ad266e16b3dbc6f80f54db67f317f

blink/common/manifest/manifest.cc

@@ -85,19 +85,6 @@ bool Manifest::LaunchHandler::operator!=(const LaunchHandler& other) const {
   return !(*this == other);
 }
 
-bool Manifest::PermissionsPolicyDeclaration::operator==(
-    const PermissionsPolicyDeclaration& other) const {
-  auto AsTuple = [](const auto& item) {
-    return std::tie(item.feature, item.allowlist);
-  };
-  return AsTuple(*this) == AsTuple(other);
-}
-
-bool Manifest::PermissionsPolicyDeclaration::operator!=(
-    const PermissionsPolicyDeclaration& other) const {
-  return !(*this == other);
-}
-
 Manifest::TranslationItem::TranslationItem() = default;
 
 Manifest::TranslationItem::~TranslationItem() = default;

blink/common/manifest/manifest_util.cc

@@ -7,6 +7,7 @@
 #include "base/no_destructor.h"
 #include "base/strings/string_util.h"
 #include "third_party/blink/public/common/manifest/manifest.h"
+#include "third_party/blink/public/common/permissions_policy/permissions_policy.h"
 #include "third_party/blink/public/mojom/manifest/capture_links.mojom.h"
 #include "third_party/blink/public/mojom/manifest/display_mode.mojom.h"
 #include "third_party/blink/public/mojom/manifest/manifest.mojom.h"

blink/public/common/manifest/manifest.h

@@ -146,19 +146,6 @@ class BLINK_COMMON_EXPORT Manifest {
         NavigateExistingClient::kAlways;
   };
 
-  // This struct replicates ManifestPermissionsPolicy with an added copy
-  // constructor, this enables containing classes to have a default copy
-  // constructor.
-  // TODO(crbug.com/): Use mojom::blink::ManifestPermissionsPolicyDeclaration
-  // directly when it can support copy/move.
-  struct BLINK_COMMON_EXPORT PermissionsPolicyDeclaration {
-    bool operator==(const PermissionsPolicyDeclaration& other) const;
-    bool operator!=(const PermissionsPolicyDeclaration& other) const;
-
-    std::string feature;
-    std::vector<std::string> allowlist;
-  };
-
   // Structure containing translations for the translatable manifest fields.
   struct BLINK_COMMON_EXPORT TranslationItem {
     TranslationItem();

blink/public/mojom/manifest/manifest.mojom

@@ -9,6 +9,7 @@ import "mojo/public/mojom/base/string16.mojom";
 import "third_party/blink/public/mojom/manifest/display_mode.mojom";
 import "third_party/blink/public/mojom/manifest/capture_links.mojom";
 import "third_party/blink/public/mojom/manifest/handle_links.mojom";
+import "third_party/blink/public/mojom/permissions_policy/permissions_policy.mojom";
 import "ui/gfx/geometry/mojom/geometry.mojom";
 import "url/mojom/url.mojom";
 import "url/mojom/origin.mojom";
@@ -98,7 +99,7 @@ struct Manifest {
   // https://github.com/robbiemc/pwa-isolated-storage/blob/main/explainer.md
   bool isolated_storage;
 
-  array<ManifestPermissionsPolicyDeclaration> permissions_policy;
+  array<ParsedPermissionsPolicyDeclaration> permissions_policy;
 
   // TODO(crbug.com/1231886): This field is non-standard and part of a Chrome
   // experiment. See:
@@ -316,14 +317,6 @@ struct ManifestUserPreferenceOverrides {
   uint32 background_color;
 };
 
-// Used by Isolated Apps to declare a permissions policy.
-// TODO(crbug.com/1275708): Add a link to the explainer when it becomes
-// available.
-struct ManifestPermissionsPolicyDeclaration {
-  string feature;
-  array<string> allowlist;
-};
-
 // Debug information for a parsed manifest.
 struct ManifestDebugInfo {
   array<ManifestError> errors;

blink/renderer/modules/manifest/manifest_parser.cc

@@ -11,12 +11,18 @@
 #include "net/base/mime_util.h"
 #include "net/base/registry_controlled_domains/registry_controlled_domain.h"
 #include "third_party/blink/public/common/features.h"
+#include "third_party/blink/public/common/manifest/manifest.h"
 #include "third_party/blink/public/common/manifest/manifest_util.h"
 #include "third_party/blink/public/common/mime_util/mime_util.h"
+#include "third_party/blink/public/common/permissions_policy/permissions_policy.h"
 #include "third_party/blink/public/common/security/protocol_handler_security_level.h"
+#include "third_party/blink/public/mojom/manifest/manifest.mojom-blink-forward.h"
+#include "third_party/blink/public/mojom/permissions_policy/permissions_policy.mojom-blink.h"
+#include "third_party/blink/public/platform/url_conversion.h"
 #include "third_party/blink/public/platform/web_icon_sizes_parser.h"
 #include "third_party/blink/public/platform/web_string.h"
 #include "third_party/blink/renderer/core/css/parser/css_parser.h"
+#include "third_party/blink/renderer/core/permissions_policy/permissions_policy_parser.h"
 #include "third_party/blink/renderer/modules/manifest/manifest_uma_util.h"
 #include "third_party/blink/renderer/modules/navigatorcontentutils/navigator_content_utils.h"
 #include "third_party/blink/renderer/platform/json/json_parser.h"
@@ -1495,19 +1501,19 @@ bool ManifestParser::ParseIsolatedStorage(const JSONObject* object) {
   return is_storage_isolated;
 }
 
-Vector<mojom::blink::ManifestPermissionsPolicyDeclarationPtr>
+Vector<blink::ParsedPermissionsPolicyDeclaration>
 ManifestParser::ParseIsolatedAppPermissions(const JSONObject* object) {
-  Vector<mojom::blink::ManifestPermissionsPolicyDeclarationPtr> out;
+  PermissionsPolicyParser::Node policy;
 
   JSONValue* json_value = object->Get("permissions_policy");
   if (!json_value)
-    return out;
+    return Vector<blink::ParsedPermissionsPolicyDeclaration>();
 
   JSONObject* permissions_dict = object->GetJSONObject("permissions_policy");
   if (!permissions_dict) {
     AddErrorInfo(
         "property 'permissions_policy' ignored, type object expected.");
-    return out;
+    return Vector<blink::ParsedPermissionsPolicyDeclaration>();
   }
 
   for (wtf_size_t i = 0; i < permissions_dict->size(); ++i) {
@@ -1524,8 +1530,27 @@ ManifestParser::ParseIsolatedAppPermissions(const JSONObject* object) {
     Vector<String> allowlist = ParseOriginAllowlist(origin_allowlist, feature);
     if (!allowlist.size())
       continue;
-    out.push_back(mojom::blink::ManifestPermissionsPolicyDeclaration::New(
-        feature, allowlist));
+    PermissionsPolicyParser::Declaration new_policy;
+    new_policy.feature_name = feature;
+    for (const auto& origin : allowlist) {
+      // PermissionsPolicyParser expects origin strings to be wrapped in single
+      // quotes, as they would be in the header's permissions policy string.
+      String wrapped_origin = "'" + origin + "'";
+      new_policy.allowlist.push_back(wrapped_origin);
+    }
+    policy.push_back(new_policy);
+  }
+
+  PolicyParserMessageBuffer logger(
+      "Error with permissions_policy manifest field: ");
+  blink::ParsedPermissionsPolicy parsed_policy =
+      PermissionsPolicyParser::ParsePolicyFromNode(
+          policy, SecurityOrigin::Create(manifest_url_), logger,
+          execution_context_);
+
+  Vector<blink::ParsedPermissionsPolicyDeclaration> out;
+  for (const auto& decl : parsed_policy) {
+    out.push_back(std::move(decl));
   }
   return out;
 }

blink/renderer/modules/manifest/manifest_parser.h

@@ -9,7 +9,9 @@
 
 #include "base/types/strong_alias.h"
 #include "third_party/abseil-cpp/absl/types/optional.h"
+#include "third_party/blink/public/common/permissions_policy/permissions_policy.h"
 #include "third_party/blink/public/mojom/manifest/manifest.mojom-blink.h"
+#include "third_party/blink/public/mojom/permissions_policy/permissions_policy.mojom-blink.h"
 #include "third_party/blink/renderer/core/execution_context/execution_context.h"
 #include "third_party/blink/renderer/modules/modules_export.h"
 #include "third_party/blink/renderer/platform/graphics/color.h"
@@ -433,8 +435,10 @@ class MODULES_EXPORT ManifestParser {
   bool ParseIsolatedStorage(const JSONObject* object);
 
   // Parses the 'permissions_policy' field of the manifest.
-  Vector<mojom::blink::ManifestPermissionsPolicyDeclarationPtr>
-  ParseIsolatedAppPermissions(const JSONObject* object);
+  // This outsources semantic parsing of the policy to the
+  // PermissionsPolicyParser.
+  Vector<blink::ParsedPermissionsPolicyDeclaration> ParseIsolatedAppPermissions(
+      const JSONObject* object);
   Vector<String> ParseOriginAllowlist(const JSONArray* allowlist,
                                       const String& feature);
 

blink/renderer/modules/manifest/manifest_parser_unittest.cc

@@ -12,6 +12,7 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/abseil-cpp/absl/types/optional.h"
 #include "third_party/blink/public/common/features.h"
+#include "third_party/blink/public/common/permissions_policy/permissions_policy.h"
 #include "third_party/blink/renderer/platform/testing/runtime_enabled_features_test_helpers.h"
 #include "third_party/blink/renderer/platform/weborigin/kurl.h"
 #include "third_party/blink/renderer/platform/weborigin/security_origin.h"