Move DocumentPolicy::Parse to DocumentPolicyParser::Parse

Since parsing of DocumentPolicy only happens in renderer, there is no
need to put Parse as part of DocumentPolicy which gets exposed to
browser. This CL separates Parse from DocumentPolicy and puts it in
renderer/core.

Bug: 993790
Change-Id: I1d7f196c9b47735d947d74c9cd7916f5001ec5b2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2017925Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Ian Clelland <iclelland@chromium.org>
Commit-Queue: Charlie Hu <chenleihu@google.com>
Cr-Commit-Position: refs/heads/master@{#737809}

third_party/blink/common/feature_policy/document_policy.cc

@@ -4,9 +4,8 @@
 
 #include "third_party/blink/public/common/feature_policy/document_policy.h"
 
-#include <map>
-
 #include "base/no_destructor.h"
+#include "third_party/blink/public/common/feature_policy/document_policy_features.h"
 #include "third_party/blink/public/common/http/structured_header.h"
 #include "third_party/blink/public/mojom/feature_policy/feature_policy_feature.mojom.h"
 
@@ -15,7 +14,10 @@ namespace blink {
 // static
 std::unique_ptr<DocumentPolicy> DocumentPolicy::CreateWithHeaderPolicy(
     const FeatureState& header_policy) {
-  return CreateWithHeaderPolicy(header_policy, GetFeatureDefaults());
+  DocumentPolicy::FeatureState feature_defaults;
+  for (const auto& entry : GetDocumentPolicyFeatureInfoMap())
+    feature_defaults.emplace(entry.first, entry.second.default_value);
+  return CreateWithHeaderPolicy(header_policy, feature_defaults);
 }
 
 namespace {
@@ -32,46 +34,6 @@ http_structured_header::Item PolicyValueToItem(const PolicyValue& value) {
   }
 }
 
-base::Optional<PolicyValue> ItemToPolicyValue(
-    const http_structured_header::Item& item) {
-  switch (item.Type()) {
-    case http_structured_header::Item::ItemType::kIntegerType:
-      return PolicyValue(static_cast<double>(item.GetInteger()));
-    case http_structured_header::Item::ItemType::kFloatType:
-      return PolicyValue(item.GetFloat());
-    default:
-      return base::nullopt;
-  }
-}
-
-struct FeatureInfo {
-  std::string feature_name;
-  std::string feature_param_name;
-};
-
-using FeatureInfoMap = std::map<mojom::FeaturePolicyFeature, FeatureInfo>;
-
-const FeatureInfoMap& GetDefaultFeatureInfoMap() {
-  static base::NoDestructor<FeatureInfoMap> feature_info_map(
-      {{mojom::FeaturePolicyFeature::kFontDisplay,
-        {"font-display-late-swap", ""}},
-       {mojom::FeaturePolicyFeature::kUnoptimizedLosslessImages,
-        {"unoptimized-lossless-images", "bpp"}}});
-  return *feature_info_map;
-}
-
-using NameFeatureMap = std::map<std::string, mojom::FeaturePolicyFeature>;
-
-const NameFeatureMap& GetDefaultNameFeatureMap() {
-  static base::NoDestructor<NameFeatureMap> name_feature_map;
-  if (name_feature_map->empty()) {
-    for (const auto& entry : GetDefaultFeatureInfoMap()) {
-      (*name_feature_map)[entry.second.feature_name] = entry.first;
-    }
-  }
-  return *name_feature_map;
-}
-
 }  // namespace
 
 // static
@@ -84,15 +46,15 @@ base::Optional<std::string> DocumentPolicy::Serialize(
       sorted_policy(policy.begin(), policy.end());
   std::sort(sorted_policy.begin(), sorted_policy.end(),
             [](const auto& a, const auto& b) {
-              const auto& m = GetDefaultFeatureInfoMap();
+              const auto& m = GetDocumentPolicyFeatureInfoMap();
               const std::string& feature_a = m.at(a.first).feature_name;
               const std::string& feature_b = m.at(b.first).feature_name;
               return feature_a < feature_b;
             });
 
   for (const auto& policy_entry : sorted_policy) {
-    const FeatureInfo& info =
-        GetDefaultFeatureInfoMap().at(policy_entry.first /* feature */);
+    const auto& info =
+        GetDocumentPolicyFeatureInfoMap().at(policy_entry.first /* feature */);
 
     const PolicyValue& value = policy_entry.second;
     if (value.Type() == mojom::PolicyValueType::kBool) {
@@ -116,74 +78,6 @@ base::Optional<std::string> DocumentPolicy::Serialize(
   return http_structured_header::SerializeList(root);
 }
 
-// static
-base::Optional<DocumentPolicy::FeatureState> DocumentPolicy::Parse(
-    const std::string& policy_string) {
-  const auto& name_feature_map = GetDefaultNameFeatureMap();
-  const auto& default_values_map = GetFeatureDefaults();
-  const auto& feature_info_map = GetDefaultFeatureInfoMap();
-
-  auto root = http_structured_header::ParseList(policy_string);
-  if (!root)
-    return base::nullopt;
-
-  DocumentPolicy::FeatureState policy;
-  for (const http_structured_header::ParameterizedMember& directive :
-       root.value()) {
-    // Each directive is allowed exactly 1 member.
-    if (directive.member.size() != 1)
-      return base::nullopt;
-
-    const http_structured_header::Item& feature_token =
-        directive.member.front();
-    // The item in directive should be token type.
-    if (!feature_token.is_token())
-      return base::nullopt;
-
-    // Feature policy now only support boolean and double PolicyValue
-    // which correspond to 0 and 1 param number.
-    if (directive.params.size() > 1)
-      return base::nullopt;
-
-    base::Optional<PolicyValue> policy_value;
-    std::string feature_name = feature_token.GetString();
-
-    if (directive.params.empty()) {  // boolean value
-      // handle "no-" prefix
-      const std::string& feature_str = feature_token.GetString();
-      const bool bool_val =
-          feature_str.size() < 3 || feature_str.substr(0, 3) != "no-";
-      policy_value = PolicyValue(bool_val);
-      if (!bool_val) {  // drop "no-" prefix
-        feature_name = feature_name.substr(3);
-      }
-    } else {  // double value
-      policy_value =
-          ItemToPolicyValue(directive.params.front().second /* param value */);
-    }
-
-    if (!policy_value)
-      return base::nullopt;
-
-    if (name_feature_map.find(feature_name) ==
-        name_feature_map.end())  // Unrecognized feature name.
-      return base::nullopt;
-    const mojom::FeaturePolicyFeature feature =
-        name_feature_map.at(feature_name);
-
-    if (default_values_map.at(feature).Type() !=
-        policy_value->Type())  // Invalid value type.
-      return base::nullopt;
-
-    if ((*policy_value).Type() != mojom::PolicyValueType::kBool &&
-        feature_info_map.at(feature).feature_param_name !=
-            directive.params.front().first)  // Invalid param key name.
-      return base::nullopt;
-
-    policy.insert({feature, std::move(*policy_value)});
-  }
-  return policy;
-}
 
 // static
 DocumentPolicy::FeatureState DocumentPolicy::MergeFeatureState(
@@ -224,7 +118,8 @@ DocumentPolicy::FeatureState DocumentPolicy::MergeFeatureState(
 
 bool DocumentPolicy::IsFeatureEnabled(
     mojom::FeaturePolicyFeature feature) const {
-  mojom::PolicyValueType feature_type = GetFeatureDefaults().at(feature).Type();
+  mojom::PolicyValueType feature_type =
+      GetDocumentPolicyFeatureInfoMap().at(feature).default_value.Type();
   return IsFeatureEnabled(feature,
                           PolicyValue::CreateMaxPolicyValue(feature_type));
 }
@@ -273,19 +168,6 @@ std::unique_ptr<DocumentPolicy> DocumentPolicy::CreateWithHeaderPolicy(
   return new_policy;
 }
 
-// static
-// TODO(iclelland): This list just contains two sample features for use during
-// development. It should be generated from definitions in a feature
-// configuration json5 file.
-const DocumentPolicy::FeatureState& DocumentPolicy::GetFeatureDefaults() {
-  static base::NoDestructor<FeatureState> default_feature_list(
-      {{mojom::FeaturePolicyFeature::kFontDisplay, PolicyValue(true)},
-       {mojom::FeaturePolicyFeature::kUnoptimizedLosslessImages,
-        PolicyValue::CreateMaxPolicyValue(
-            mojom::PolicyValueType::kDecDouble)}});
-  return *default_feature_list;
-}
-
 // static
 bool DocumentPolicy::IsPolicyCompatible(
     const DocumentPolicy::FeatureState& required_policy,

third_party/blink/common/feature_policy/document_policy_unittest.cc

@@ -12,138 +12,12 @@ namespace {
 
 class DocumentPolicyTest : public ::testing::Test {
  public:
-  DocumentPolicyTest() {}
+  DocumentPolicyTest() = default;
 
  protected:
   std::unique_ptr<DocumentPolicy> document_policy_;
 };
 
-const std::string kValidPolicies[] = {
-    "",   // An empty policy.
-    " ",  // An empty policy.
-    "font-display-late-swap",
-    "no-font-display-late-swap",
-    "unoptimized-lossless-images;bpp=1.0",
-    "unoptimized-lossless-images;bpp=2",
-    "unoptimized-lossless-images;bpp=2.0,no-font-display-late-swap",
-    "no-font-display-late-swap,unoptimized-lossless-images;bpp=2.0"};
-
-const std::string kInvalidPolicies[] = {
-    "bad-feature-name", "no-bad-feature-name",
-    "font-display-late-swap;value=true",   // unnecessary param
-    "unoptimized-lossless-images;bpp=?0",  // wrong type of param
-    "unoptimized-lossless-images;ppb=2",   // wrong param key
-    "\"font-display-late-swap\"",  // policy member should be token instead of
-                                   // string
-    "();bpp=2",                    // empty feature token
-    "(font-display-late-swap, unoptimized-lossless-images);bpp=2"  // too many
-                                                                   // feature
-                                                                   // tokens
-};
-
-// TODO(chenleihu): find a FeaturePolicyFeature name start with 'f' < c < 'n'
-// to further strengthen the test on proving "no-" prefix is not counted as part
-// of feature name for ordering.
-const std::pair<DocumentPolicy::FeatureState, std::string>
-    kPolicySerializationTestCases[] = {
-        {{{blink::mojom::FeaturePolicyFeature::kFontDisplay,
-           PolicyValue(false)},
-          {blink::mojom::FeaturePolicyFeature::kUnoptimizedLosslessImages,
-           PolicyValue(1.0)}},
-         "no-font-display-late-swap, unoptimized-lossless-images;bpp=1.0"},
-        // Changing ordering of FeatureState element should not affect
-        // serialization result.
-        {{{blink::mojom::FeaturePolicyFeature::kUnoptimizedLosslessImages,
-           PolicyValue(1.0)},
-          {blink::mojom::FeaturePolicyFeature::kFontDisplay,
-           PolicyValue(false)}},
-         "no-font-display-late-swap, unoptimized-lossless-images;bpp=1.0"},
-        // Flipping boolean-valued policy from false to true should not affect
-        // result ordering of feature.
-        {{{blink::mojom::FeaturePolicyFeature::kFontDisplay, PolicyValue(true)},
-          {blink::mojom::FeaturePolicyFeature::kUnoptimizedLosslessImages,
-           PolicyValue(1.0)}},
-         "font-display-late-swap, unoptimized-lossless-images;bpp=1.0"}};
-
-const std::pair<std::string, DocumentPolicy::FeatureState>
-    kPolicyParseTestCases[] = {
-        {"no-font-display-late-swap,unoptimized-lossless-images;bpp=1",
-         {{blink::mojom::FeaturePolicyFeature::kFontDisplay,
-           PolicyValue(false)},
-          {blink::mojom::FeaturePolicyFeature::kUnoptimizedLosslessImages,
-           PolicyValue(1.0)}}},
-        // White-space is allowed in some positions in structured-header.
-        {"no-font-display-late-swap,   unoptimized-lossless-images;bpp=1",
-         {{blink::mojom::FeaturePolicyFeature::kFontDisplay,
-           PolicyValue(false)},
-          {blink::mojom::FeaturePolicyFeature::kUnoptimizedLosslessImages,
-           PolicyValue(1.0)}}}};
-
-const DocumentPolicy::FeatureState kParsedPolicies[] = {
-    {},  // An empty policy
-    {{mojom::FeaturePolicyFeature::kFontDisplay, PolicyValue(false)}},
-    {{mojom::FeaturePolicyFeature::kFontDisplay, PolicyValue(true)}},
-    {{mojom::FeaturePolicyFeature::kUnoptimizedLosslessImages,
-      PolicyValue(1.0)}},
-    {{mojom::FeaturePolicyFeature::kFontDisplay, PolicyValue(true)},
-     {mojom::FeaturePolicyFeature::kUnoptimizedLosslessImages,
-      PolicyValue(1.0)}}};
-
-// Serialize and then Parse the result of serialization should cancel each
-// other out, i.e. d == Parse(Serialize(d)).
-// The other way s == Serialize(Parse(s)) is not always true because structured
-// header allows some optional white spaces in its parsing targets and floating
-// point numbers will be rounded, e.g. bpp=1 will be parsed to PolicyValue(1.0)
-// and get serialized to bpp=1.0.
-TEST_F(DocumentPolicyTest, SerializeAndParse) {
-  for (const auto& policy : kParsedPolicies) {
-    const base::Optional<std::string> policy_string =
-        DocumentPolicy::Serialize(policy);
-    ASSERT_TRUE(policy_string.has_value());
-    const base::Optional<DocumentPolicy::FeatureState> reparsed_policy =
-        DocumentPolicy::Parse(policy_string.value());
-
-    ASSERT_TRUE(reparsed_policy.has_value());
-    EXPECT_EQ(reparsed_policy.value(), policy);
-  }
-}
-
-TEST_F(DocumentPolicyTest, ParseValidPolicy) {
-  for (const std::string& policy : kValidPolicies) {
-    EXPECT_NE(DocumentPolicy::Parse(policy), base::nullopt)
-        << "Should parse " << policy;
-  }
-}
-
-TEST_F(DocumentPolicyTest, ParseInvalidPolicy) {
-  for (const std::string& policy : kInvalidPolicies) {
-    EXPECT_EQ(DocumentPolicy::Parse(policy), base::nullopt)
-        << "Should fail to parse " << policy;
-  }
-}
-
-TEST_F(DocumentPolicyTest, SerializeResultShouldMatch) {
-  for (const auto& test_case : kPolicySerializationTestCases) {
-    const DocumentPolicy::FeatureState& policy = test_case.first;
-    const std::string& expected = test_case.second;
-    const auto result = DocumentPolicy::Serialize(policy);
-
-    ASSERT_TRUE(result.has_value());
-    EXPECT_EQ(result.value(), expected);
-  }
-}
-
-TEST_F(DocumentPolicyTest, ParseResultShouldMatch) {
-  for (const auto& test_case : kPolicyParseTestCases) {
-    const std::string& input = test_case.first;
-    const DocumentPolicy::FeatureState& expected = test_case.second;
-    const auto result = DocumentPolicy::Parse(input);
-
-    ASSERT_TRUE(result.has_value());
-    EXPECT_EQ(result.value(), expected);
-  }
-}
-
 // Helper function to convert literal to FeatureState.
 template <class T>
 DocumentPolicy::FeatureState FeatureState(

third_party/blink/public/common/feature_policy/document_policy.h

@@ -92,17 +92,11 @@ class BLINK_COMMON_EXPORT DocumentPolicy {
   static bool IsPolicyCompatible(const FeatureState& required_policy,
                                  const FeatureState& incoming_policy);
 
-  // Returns the list of features which can be controlled by Document Policy,
-  // and their default values.
-  static const FeatureState& GetFeatureDefaults();
-
   // Serialize document policy according to http_structured_header.
   // returns base::nullopt when http structured header serializer encounters
   // problems, e.g. double value out of the range supported.
   static base::Optional<std::string> Serialize(const FeatureState& policy);
 
-  // Parse document policy header to FeatureState
-  static base::Optional<FeatureState> Parse(const std::string& header);
 
   // Merge two FeatureState map. Take stricter value when there is conflict.
   static FeatureState MergeFeatureState(const FeatureState& policy1,

third_party/blink/renderer/core/BUILD.gn

@@ -1149,6 +1149,7 @@ jumbo_source_set("unit_tests") {
     "exported/web_searchable_form_data_test.cc",
     "exported/web_selector_test.cc",
     "exported/web_view_test.cc",
+    "feature_policy/document_policy_parser_test.cc",
     "feature_policy/feature_policy_test.cc",
     "feature_policy/policy_test.cc",
     "fetch/blob_bytes_consumer_test.cc",

third_party/blink/renderer/core/dom/document_test.cc

@@ -38,6 +38,7 @@
 #include "services/network/public/mojom/referrer_policy.mojom-blink.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/blink/public/common/feature_policy/document_policy_features.h"
 #include "third_party/blink/public/mojom/feature_policy/feature_policy_feature.mojom-blink.h"
 #include "third_party/blink/renderer/bindings/core/v8/isolated_world_csp.h"
 #include "third_party/blink/renderer/bindings/core/v8/v8_binding_for_core.h"
@@ -1216,7 +1217,7 @@ TEST_F(DocumentTest, DocumentPolicyFeaturePolicyCoexist) {
   GetDocument().GetSecurityContext().SetDocumentPolicyForTesting(
       DocumentPolicy::CreateWithHeaderPolicy({}));
   EXPECT_EQ(
-      DocumentPolicy::GetFeatureDefaults().at(test_feature),
+      GetDocumentPolicyFeatureInfoMap().at(test_feature).default_value,
       GetDocument().GetSecurityContext().GetDocumentPolicy()->GetFeatureValue(
           test_feature));
 

third_party/blink/renderer/core/feature_policy/BUILD.gn

@@ -6,6 +6,8 @@ import("//third_party/blink/renderer/core/core.gni")
 
 blink_core_sources("feature_policy") {
   sources = [
+    "document_policy_parser.cc",
+    "document_policy_parser.h",
     "dom_document_policy.h",
     "dom_feature_policy.cc",
     "dom_feature_policy.h",

third_party/blink/renderer/core/feature_policy/document_policy_parser.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "third_party/blink/renderer/core/feature_policy/document_policy_parser.h"
+
+#include "third_party/blink/public/common/http/structured_header.h"
+#include "third_party/blink/public/mojom/feature_policy/policy_value.mojom-blink-forward.h"
+
+namespace blink {
+
+base::Optional<PolicyValue> ItemToPolicyValue(
+    const ::blink::http_structured_header::Item& item) {
+  switch (item.Type()) {
+    case ::blink::http_structured_header::Item::ItemType::kIntegerType:
+      return PolicyValue(static_cast<double>(item.GetInteger()));
+    case ::blink::http_structured_header::Item::ItemType::kFloatType:
+      return PolicyValue(item.GetFloat());
+    default:
+      return base::nullopt;
+  }
+}
+
+// static
+base::Optional<DocumentPolicy::FeatureState> DocumentPolicyParser::Parse(
+    const String& policy_string) {
+  return ParseInternal(policy_string, GetDocumentPolicyNameFeatureMap(),
+                       GetDocumentPolicyFeatureInfoMap(),
+                       GetAvailableDocumentPolicyFeatures());
+}
+
+// static
+base::Optional<DocumentPolicy::FeatureState>
+DocumentPolicyParser::ParseInternal(
+    const String& policy_string,
+    const DocumentPolicyNameFeatureMap& name_feature_map,
+    const DocumentPolicyFeatureInfoMap& feature_info_map,
+    const FeatureSet& available_features) {
+  auto root = ::blink::http_structured_header::ParseList(policy_string.Ascii());
+  if (!root)
+    return base::nullopt;
+
+  DocumentPolicy::FeatureState policy;
+  for (const ::blink::http_structured_header::ParameterizedMember& directive :
+       root.value()) {
+    // Each directive is allowed exactly 1 member.
+    if (directive.member.size() != 1)
+      return base::nullopt;
+
+    const ::blink::http_structured_header::Item& feature_token =
+        directive.member.front();
+    // The item in directive should be token type.
+    if (!feature_token.is_token())
+      return base::nullopt;
+
+    // Feature policy now only support boolean and double PolicyValue
+    // which correspond to 0 and 1 param number.
+    if (directive.params.size() > 1)
+      return base::nullopt;
+
+    base::Optional<PolicyValue> policy_value;
+    std::string feature_name = feature_token.GetString();
+
+    if (directive.params.empty()) {  // boolean value
+      // handle "no-" prefix
+      const std::string& feature_str = feature_token.GetString();
+      const bool bool_val =
+          feature_str.size() < 3 || feature_str.substr(0, 3) != "no-";
+      policy_value = PolicyValue(bool_val);
+      if (!bool_val) {  // drop "no-" prefix
+        feature_name = feature_name.substr(3);
+      }
+    } else {  // double value
+      policy_value =
+          ItemToPolicyValue(directive.params.front().second /* param value */);
+    }
+
+    if (!policy_value)
+      return base::nullopt;
+
+    if (name_feature_map.find(feature_name) ==
+        name_feature_map.end())  // Unrecognized feature name.
+      return base::nullopt;
+
+    const mojom::blink::FeaturePolicyFeature feature =
+        name_feature_map.at(feature_name);
+
+    // If feature is not available, i.e. not enabled, ignore the entry.
+    if (available_features.find(feature) == available_features.end())
+      continue;
+
+    if (feature_info_map.at(feature).default_value.Type() !=
+        policy_value->Type())  // Invalid value type.
+      return base::nullopt;
+
+    if ((*policy_value).Type() != mojom::blink::PolicyValueType::kBool &&
+        feature_info_map.at(feature).feature_param_name !=
+            directive.params.front().first)  // Invalid param key name.
+      return base::nullopt;
+
+    policy.emplace(feature, std::move(*policy_value));
+  }
+  return policy;
+}
+
+}  // namespace blink

third_party/blink/renderer/core/feature_policy/document_policy_parser.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef THIRD_PARTY_BLINK_RENDERER_CORE_FEATURE_POLICY_DOCUMENT_POLICY_PARSER_H_
+#define THIRD_PARTY_BLINK_RENDERER_CORE_FEATURE_POLICY_DOCUMENT_POLICY_PARSER_H_
+
+#include "third_party/blink/public/common/feature_policy/document_policy.h"
+#include "third_party/blink/public/common/feature_policy/document_policy_features.h"
+#include "third_party/blink/renderer/core/core_export.h"
+#include "third_party/blink/renderer/core/feature_policy/feature_policy_helper.h"
+#include "third_party/blink/renderer/platform/wtf/allocator/allocator.h"
+
+namespace blink {
+class CORE_EXPORT DocumentPolicyParser {
+  STATIC_ONLY(DocumentPolicyParser);
+
+ public:
+  // Parse document policy header and 'policy' attribute on iframe to
+  // DocumentPolicy::FeatureState.
+  static base::Optional<DocumentPolicy::FeatureState> Parse(
+      const String& policy_string);
+
+  // Internal parsing method for testing.
+  static base::Optional<DocumentPolicy::FeatureState> ParseInternal(
+      const String& policy_string,
+      const DocumentPolicyNameFeatureMap& name_feature_map,
+      const DocumentPolicyFeatureInfoMap& feature_info_map,
+      const FeatureSet& available_features);
+};
+}  // namespace blink
+
+#endif  // THIRD_PARTY_BLINK_RENDERER_CORE_FEATURE_POLICY_DOCUMENT_POLICY_PARSER_H_

third_party/blink/renderer/core/feature_policy/document_policy_parser_test.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "third_party/blink/renderer/core/feature_policy/document_policy_parser.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/blink/public/common/feature_policy/document_policy.h"
+#include "third_party/blink/public/mojom/feature_policy/feature_policy_feature.mojom-blink.h"
+
+namespace blink {
+namespace {
+
+using DocumentPolicyParserTest = ::testing::Test;
+
+const char* const kValidPolicies[] = {
+    "",   // An empty policy.
+    " ",  // An empty policy.
+    "font-display-late-swap",
+    "no-font-display-late-swap",
+    "unoptimized-lossless-images;bpp=1.0",
+    "unoptimized-lossless-images;bpp=2",
+    "unoptimized-lossless-images;bpp=2.0,no-font-display-late-swap",
+    "no-font-display-late-swap,unoptimized-lossless-images;bpp=2.0"};
+
+const char* const kInvalidPolicies[] = {
+    "bad-feature-name", "no-bad-feature-name",
+    "font-display-late-swap;value=true",   // unnecessary param
+    "unoptimized-lossless-images;bpp=?0",  // wrong type of param
+    "unoptimized-lossless-images;ppb=2",   // wrong param key
+    "\"font-display-late-swap\"",  // policy member should be token instead of
+                                   // string
+    "();bpp=2",                    // empty feature token
+    "(font-display-late-swap, unoptimized-lossless-images);bpp=2"  // too many
+                                                                   // feature
+                                                                   // tokens
+};
+
+// TODO(chenleihu): find a FeaturePolicyFeature name start with 'f' < c < 'n'
+// to further strengthen the test on proving "no-" prefix is not counted as part
+// of feature name for ordering.
+const std::pair<DocumentPolicy::FeatureState, std::string>
+    kPolicySerializationTestCases[] = {
+        {{{blink::mojom::FeaturePolicyFeature::kFontDisplay,
+           PolicyValue(false)},
+          {blink::mojom::FeaturePolicyFeature::kUnoptimizedLosslessImages,
+           PolicyValue(1.0)}},
+         "no-font-display-late-swap, unoptimized-lossless-images;bpp=1.0"},
+        // Changing ordering of FeatureState element should not affect
+        // serialization result.
+        {{{blink::mojom::FeaturePolicyFeature::kUnoptimizedLosslessImages,
+           PolicyValue(1.0)},
+          {blink::mojom::FeaturePolicyFeature::kFontDisplay,
+           PolicyValue(false)}},
+         "no-font-display-late-swap, unoptimized-lossless-images;bpp=1.0"},
+        // Flipping boolean-valued policy from false to true should not affect
+        // result ordering of feature.
+        {{{blink::mojom::FeaturePolicyFeature::kFontDisplay, PolicyValue(true)},
+          {blink::mojom::FeaturePolicyFeature::kUnoptimizedLosslessImages,
+           PolicyValue(1.0)}},
+         "font-display-late-swap, unoptimized-lossless-images;bpp=1.0"}};
+
+const std::pair<const char*, DocumentPolicy::FeatureState>
+    kPolicyParseTestCases[] = {
+        {"no-font-display-late-swap,unoptimized-lossless-images;bpp=1",
+         {{blink::mojom::FeaturePolicyFeature::kFontDisplay,
+           PolicyValue(false)},
+          {blink::mojom::FeaturePolicyFeature::kUnoptimizedLosslessImages,
+           PolicyValue(1.0)}}},
+        // White-space is allowed in some positions in structured-header.
+        {"no-font-display-late-swap,   unoptimized-lossless-images;bpp=1",
+         {{blink::mojom::FeaturePolicyFeature::kFontDisplay,
+           PolicyValue(false)},
+          {blink::mojom::FeaturePolicyFeature::kUnoptimizedLosslessImages,
+           PolicyValue(1.0)}}}};
+
+const DocumentPolicy::FeatureState kParsedPolicies[] = {
+    {},  // An empty policy
+    {{mojom::FeaturePolicyFeature::kFontDisplay, PolicyValue(false)}},
+    {{mojom::FeaturePolicyFeature::kFontDisplay, PolicyValue(true)}},
+    {{mojom::FeaturePolicyFeature::kUnoptimizedLosslessImages,
+      PolicyValue(1.0)}},
+    {{mojom::FeaturePolicyFeature::kFontDisplay, PolicyValue(true)},
+     {mojom::FeaturePolicyFeature::kUnoptimizedLosslessImages,
+      PolicyValue(1.0)}}};
+
+// Serialize and then Parse the result of serialization should cancel each
+// other out, i.e. d == Parse(Serialize(d)).
+// The other way s == Serialize(Parse(s)) is not always true because structured
+// header allows some optional white spaces in its parsing targets and floating
+// point numbers will be rounded, e.g. bpp=1 will be parsed to PolicyValue(1.0)
+// and get serialized to bpp=1.0.
+TEST_F(DocumentPolicyParserTest, SerializeAndParse) {
+  for (const auto& policy : kParsedPolicies) {
+    const base::Optional<std::string> policy_string =
+        DocumentPolicy::Serialize(policy);
+    ASSERT_TRUE(policy_string.has_value());
+    const base::Optional<DocumentPolicy::FeatureState> reparsed_policy =
+        DocumentPolicyParser::Parse(policy_string.value().c_str());
+
+    ASSERT_TRUE(reparsed_policy.has_value());
+    EXPECT_EQ(reparsed_policy.value(), policy);
+  }
+}
+
+TEST_F(DocumentPolicyParserTest, ParseValidPolicy) {
+  for (const char* policy : kValidPolicies) {
+    EXPECT_NE(DocumentPolicyParser::Parse(policy), base::nullopt)
+        << "Should parse " << policy;
+  }
+}
+
+TEST_F(DocumentPolicyParserTest, ParseInvalidPolicy) {
+  for (const char* policy : kInvalidPolicies) {
+    EXPECT_EQ(DocumentPolicyParser::Parse(policy), base::nullopt)
+        << "Should fail to parse " << policy;
+  }
+}
+
+TEST_F(DocumentPolicyParserTest, SerializeResultShouldMatch) {
+  for (const auto& test_case : kPolicySerializationTestCases) {
+    const DocumentPolicy::FeatureState& policy = test_case.first;
+    const std::string& expected = test_case.second;
+    const auto result = DocumentPolicy::Serialize(policy);
+
+    ASSERT_TRUE(result.has_value());
+    EXPECT_EQ(result.value(), expected);
+  }
+}
+
+TEST_F(DocumentPolicyParserTest, ParseResultShouldMatch) {
+  for (const auto& test_case : kPolicyParseTestCases) {
+    const char* input = test_case.first;
+    const DocumentPolicy::FeatureState& expected = test_case.second;
+    const auto result = DocumentPolicyParser::Parse(input);
+
+    ASSERT_TRUE(result.has_value());
+    EXPECT_EQ(result.value(), expected);
+  }
+}
+
+}  // namespace
+}  // namespace blink

third_party/blink/renderer/core/html/html_iframe_element.cc

@@ -30,6 +30,7 @@
 #include "third_party/blink/renderer/core/css/css_property_names.h"
 #include "third_party/blink/renderer/core/css/style_change_reason.h"
 #include "third_party/blink/renderer/core/dom/element.h"
+#include "third_party/blink/renderer/core/feature_policy/document_policy_parser.h"
 #include "third_party/blink/renderer/core/feature_policy/feature_policy_parser.h"
 #include "third_party/blink/renderer/core/feature_policy/iframe_policy.h"
 #include "third_party/blink/renderer/core/frame/csp/content_security_policy.h"
@@ -284,7 +285,7 @@ void HTMLIFrameElement::ParseAttribute(
 
 DocumentPolicy::FeatureState HTMLIFrameElement::ConstructRequiredPolicy()
     const {
-  return DocumentPolicy::Parse(required_policy_.Ascii())
+  return DocumentPolicyParser::Parse(required_policy_)
       .value_or(DocumentPolicy::FeatureState{});
 }
 

third_party/blink/renderer/core/loader/document_loader.cc

@@ -45,6 +45,7 @@
 #include "third_party/blink/renderer/core/dom/events/event.h"
 #include "third_party/blink/renderer/core/dom/scriptable_document_parser.h"
 #include "third_party/blink/renderer/core/dom/weak_identifier_map.h"
+#include "third_party/blink/renderer/core/feature_policy/document_policy_parser.h"
 #include "third_party/blink/renderer/core/frame/csp/content_security_policy.h"
 #include "third_party/blink/renderer/core/frame/deprecation.h"
 #include "third_party/blink/renderer/core/frame/frame_console.h"
@@ -185,9 +186,8 @@ DocumentLoader::DocumentLoader(
     // Require-Document-Policy header only affects subtree of current document,
     // but not the current document.
     const DocumentPolicy::FeatureState header_required_policy =
-        DocumentPolicy::Parse(
-            response_.HttpHeaderField(http_names::kRequireDocumentPolicy)
-                .Ascii())
+        DocumentPolicyParser::Parse(
+            response_.HttpHeaderField(http_names::kRequireDocumentPolicy))
             .value_or(DocumentPolicy::FeatureState{});
     frame_->SetRequiredDocumentPolicy(DocumentPolicy::MergeFeatureState(
         frame_policy_.required_document_policy, header_required_policy));
@@ -818,9 +818,9 @@ DocumentPolicy::FeatureState DocumentLoader::CreateDocumentPolicy() {
   if (!RuntimeEnabledFeatures::DocumentPolicyEnabled())
     return DocumentPolicy::FeatureState{};
 
-  DocumentPolicy::FeatureState header_policy =
-      DocumentPolicy::Parse(
-          response_.HttpHeaderField(http_names::kDocumentPolicy).Ascii())
+  const DocumentPolicy::FeatureState header_policy =
+      DocumentPolicyParser::Parse(
+          response_.HttpHeaderField(http_names::kDocumentPolicy))
           .value_or(DocumentPolicy::FeatureState{});
 
   if (!DocumentPolicy::IsPolicyCompatible(