Generate GetDefaultFeatureList from feature_policy_features.json5

Previously, adding a new feature to feature policy needs to modify
3 files:
- feature_policy_feature.mojom
- feature_policy_features.json5
- feature_policy.cc

This CL generates |FeaturePolicy::GetDefaultFeatureList| from json5
config, which eliminates the need to modify feature_policy.cc when
adding a new feature.

Change-Id: I368215bc66fa8b568e20203f7e9810ddd13f683f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2212794Reviewed-by: Ian Clelland <iclelland@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Commit-Queue: Charlie Hu <chenleihu@google.com>
Cr-Commit-Position: refs/heads/master@{#789494}

third_party/blink/common/BUILD.gn

@@ -7,6 +7,26 @@ import("//testing/libfuzzer/fuzzer_test.gni")
 import("//testing/test.gni")
 import("//third_party/blink/renderer/build/scripts/scripts.gni")
 
+blink_python_runner("make_generated_feature_policy_features") {
+  script = "../renderer/build/scripts/make_feature_policy_features.py"
+
+  inputs =
+      scripts_for_json5_files + [
+        "../renderer/build/scripts/make_feature_policy_features.py",
+        "../renderer/core/feature_policy/feature_policy_features.json5",
+        "../renderer/build/scripts/templates/feature_policy_features.cc.tmpl",
+      ]
+  outputs = [ "$root_gen_dir/third_party/blink/common/feature_policy/feature_policy_features.cc" ]
+
+  args = [
+    rebase_path("../renderer/core/feature_policy/feature_policy_features.json5",
+                root_build_dir),
+    "--output_dir",
+    rebase_path("$root_gen_dir/third_party/blink/common/feature_policy",
+                root_build_dir),
+  ]
+}
+
 blink_python_runner("make_generated_document_policy_features") {
   script = "../renderer/build/scripts/make_document_policy_features.py"
 
@@ -134,9 +154,11 @@ jumbo_source_set("common") {
   ]
 
   sources += get_target_outputs(":make_generated_document_policy_features")
+  sources += get_target_outputs(":make_generated_feature_policy_features")
 
   public_deps = [
     ":make_generated_document_policy_features",
+    ":make_generated_feature_policy_features",
     "//third_party/blink/common/privacy_budget:privacy_budget",
     "//third_party/blink/public/common:headers",
   ]

third_party/blink/common/feature_policy/README.md

@@ -55,10 +55,6 @@ described in the file's comments.
 2. Append the new feature enum with a brief description as well in
 `third_party/blink/public/mojom/feature_policy/feature_policy_feature.mojom`
 
-3. In `third_party/blink/renderer/platform/feature_policy/feature_policy.cc`,
-add an entry to `FeaturePolicy::GetDefaultFeatureList` with the default value
-to use for the new feature.
-
 ##### Integrate the feature behaviour with feature policy
 1. The most common way to check if features are enabled is `ExecutionContext::IsFeatureEnabled`.
 

third_party/blink/common/feature_policy/feature_policy_unittest.cc

@@ -36,14 +36,14 @@ class FeaturePolicyTest : public testing::Test {
  protected:
   FeaturePolicyTest()
       : feature_list_({{kDefaultOnFeature,
-                        FeaturePolicy::FeatureDefault(
-                            FeaturePolicy::FeatureDefault::EnableForAll)},
+                        FeaturePolicyFeatureDefault(
+                            FeaturePolicyFeatureDefault::EnableForAll)},
                        {kDefaultSelfFeature,
-                        FeaturePolicy::FeatureDefault(
-                            FeaturePolicy::FeatureDefault::EnableForSelf)},
+                        FeaturePolicyFeatureDefault(
+                            FeaturePolicyFeatureDefault::EnableForSelf)},
                        {kDefaultOffFeature,
-                        FeaturePolicy::FeatureDefault(
-                            FeaturePolicy::FeatureDefault::DisableForAll)}}) {}
+                        FeaturePolicyFeatureDefault(
+                            FeaturePolicyFeatureDefault::DisableForAll)}}) {}
 
   ~FeaturePolicyTest() override = default;
 
@@ -81,7 +81,7 @@ class FeaturePolicyTest : public testing::Test {
  private:
   // Contains the list of controlled features, so that we are guaranteed to
   // have at least one of each kind of default behaviour represented.
-  FeaturePolicy::FeatureList feature_list_;
+  FeaturePolicyFeatureList feature_list_;
 };
 
 TEST_F(FeaturePolicyTest, TestInitialPolicy) {

third_party/blink/public/common/BUILD.gn

@@ -76,6 +76,7 @@ source_set("headers") {
     "feature_policy/document_policy.h",
     "feature_policy/document_policy_features.h",
     "feature_policy/feature_policy.h",
+    "feature_policy/feature_policy_features.h",
     "feature_policy/feature_policy_forward.h",
     "feature_policy/policy_value.h",
     "features.h",

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

@@ -11,6 +11,7 @@
 #include "base/macros.h"
 #include "services/network/public/mojom/web_sandbox_flags.mojom-shared.h"
 #include "third_party/blink/public/common/common_export.h"
+#include "third_party/blink/public/common/feature_policy/feature_policy_features.h"
 #include "third_party/blink/public/mojom/feature_policy/feature_policy.mojom-forward.h"
 #include "third_party/blink/public/mojom/feature_policy/feature_policy_feature.mojom-forward.h"
 #include "url/origin.h"
@@ -69,9 +70,10 @@ namespace blink {
 // feature is available when no policy has been declared, ans determines how the
 // feature is inherited across origin boundaries.
 //
-// If the default policy  is in effect for a frame, then it controls how the
+// If the default policy is in effect for a frame, then it controls how the
 // feature is inherited by any cross-origin iframes embedded by the frame. (See
-// the comments below in FeaturePolicy::FeatureDefault for specifics)
+// the comments in |FeaturePolicyFeatureDefault| in feature_policy_features.h
+// for specifics)
 //
 // Policy Inheritance
 // ------------------
@@ -80,7 +82,7 @@ namespace blink {
 // receive the same set of enables features as the parent frame. Whether or not
 // features are inherited by cross-origin iframes without an explicit policy is
 // determined by the feature's default policy. (Again, see the comments in
-// FeaturePolicy::FeatureDefault for details)
+// |FeaturePolicyFeatureDefault| in feature_policy_features.h for details)
 
 // This struct holds feature policy allowlist data that needs to be replicated
 // between a RenderFrame and any of its associated RenderFrameProxies. A list of
@@ -164,30 +166,6 @@ class BLINK_COMMON_EXPORT FeaturePolicy {
     bool matches_opaque_src_{false};
   };
 
-  // The FeaturePolicy::FeatureDefault enum defines the default enable state for
-  // a feature when neither it nor any parent frame have declared an explicit
-  // policy. The three possibilities map directly to Feature Policy Allowlist
-  // semantics.
-  //
-  // The default values for each feature are set in GetDefaultFeatureList.
-  enum class FeatureDefault {
-    // Equivalent to []. If this default policy is in effect for a frame, then
-    // the feature will not be enabled for that frame or any of its children.
-    DisableForAll,
-
-    // Equivalent to ["self"]. If this default policy is in effect for a frame,
-    // then the feature will be enabled for that frame, and any same-origin
-    // child frames, but not for any cross-origin child frames.
-    EnableForSelf,
-
-    // Equivalent to ["*"]. If in effect for a frame, then the feature is
-    // enabled for that frame and all of its children.
-    EnableForAll
-  };
-
-  using FeatureList =
-      std::map<mojom::FeaturePolicyFeature, FeaturePolicy::FeatureDefault>;
-
   ~FeaturePolicy();
 
   static std::unique_ptr<FeaturePolicy> CreateFromParentPolicy(
@@ -228,8 +206,7 @@ class BLINK_COMMON_EXPORT FeaturePolicy {
   const url::Origin& GetOriginForTest() const { return origin_; }
 
   // Returns the list of features which can be controlled by Feature Policy.
-  const FeatureList& GetFeatureList() const;
-  static const FeatureList& GetDefaultFeatureList();
+  const FeaturePolicyFeatureList& GetFeatureList() const;
 
   static mojom::FeaturePolicyFeature FeatureForSandboxFlag(
       network::mojom::WebSandboxFlags flag);
@@ -237,12 +214,13 @@ class BLINK_COMMON_EXPORT FeaturePolicy {
  private:
   friend class FeaturePolicyTest;
 
-  FeaturePolicy(url::Origin origin, const FeatureList& feature_list);
+  FeaturePolicy(url::Origin origin,
+                const FeaturePolicyFeatureList& feature_list);
   static std::unique_ptr<FeaturePolicy> CreateFromParentPolicy(
       const FeaturePolicy* parent_policy,
       const ParsedFeaturePolicy& container_policy,
       const url::Origin& origin,
-      const FeatureList& features);
+      const FeaturePolicyFeatureList& features);
 
   // Updates the inherited policy with the declarations from the iframe allow*
   // attributes.
@@ -265,7 +243,7 @@ class BLINK_COMMON_EXPORT FeaturePolicy {
   // https://crbug.com/937131.
   FeatureState proposed_inherited_policies_;
 
-  const FeatureList& feature_list_;
+  const FeaturePolicyFeatureList& feature_list_;
 
   DISALLOW_COPY_AND_ASSIGN(FeaturePolicy);
 };

third_party/blink/public/common/feature_policy/feature_policy_features.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_PUBLIC_COMMON_FEATURE_POLICY_FEATURE_POLICY_FEATURES_H_
+#define THIRD_PARTY_BLINK_PUBLIC_COMMON_FEATURE_POLICY_FEATURE_POLICY_FEATURES_H_
+
+#include "base/containers/flat_map.h"
+#include "third_party/blink/public/common/common_export.h"
+#include "third_party/blink/public/mojom/feature_policy/feature_policy_feature.mojom-forward.h"
+
+namespace blink {
+
+// The FeaturePolicyFeatureDefault enum defines the default enable state for
+// a feature when neither it nor any parent frame have declared an explicit
+// policy. The three possibilities map directly to Feature Policy Allowlist
+// semantics.
+//
+// The default values for each feature are set in GetDefaultFeatureList.
+enum class FeaturePolicyFeatureDefault {
+  // Equivalent to []. If this default policy is in effect for a frame, then
+  // the feature will not be enabled for that frame or any of its children.
+  DisableForAll,
+
+  // Equivalent to ["self"]. If this default policy is in effect for a frame,
+  // then the feature will be enabled for that frame, and any same-origin
+  // child frames, but not for any cross-origin child frames.
+  EnableForSelf,
+
+  // Equivalent to ["*"]. If in effect for a frame, then the feature is
+  // enabled for that frame and all of its children.
+  EnableForAll
+};
+
+using FeaturePolicyFeatureList =
+    std::map<mojom::FeaturePolicyFeature, FeaturePolicyFeatureDefault>;
+
+BLINK_COMMON_EXPORT const FeaturePolicyFeatureList&
+GetFeaturePolicyFeatureList();
+
+}  // namespace blink
+
+#endif  // THIRD_PARTY_BLINK_PUBLIC_COMMON_FEATURE_POLICY_FEATURE_POLICY_FEATURES_H_

third_party/blink/renderer/build/scripts/make_feature_policy_features.py

+# 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.
+
+import json5_generator
+import template_expander
+
+
+class FeaturePolicyFeatureWriter(json5_generator.Writer):
+    file_basename = 'feature_policy_features'
+
+    def __init__(self, json5_file_path, output_dir):
+        super(FeaturePolicyFeatureWriter,
+              self).__init__(json5_file_path, output_dir)
+
+        @template_expander.use_jinja('templates/' + self.file_basename +
+                                     '.cc.tmpl')
+        def generate_implementation():
+            return {
+                'header_guard':
+                self.make_header_guard(self._relative_output_dir +
+                                       self.file_basename + '.h'),
+                'input_files':
+                self._input_files,
+                'features':
+                self.json5_file.name_dictionaries
+            }
+
+        self._outputs = {
+            self.file_basename + '.cc': generate_implementation,
+        }
+
+
+if __name__ == '__main__':
+    json5_generator.Maker(FeaturePolicyFeatureWriter).main()

third_party/blink/renderer/build/scripts/templates/feature_policy_features.cc.tmpl

+{% from 'templates/macros.tmpl' import license, source_files_for_generated_file %}
+{{license()}}
+
+{{ source_files_for_generated_file(template_file, input_files) }}
+
+#include "third_party/blink/public/common/feature_policy/feature_policy_features.h"
+
+#include "base/no_destructor.h"
+#include "third_party/blink/public/mojom/feature_policy/feature_policy_feature.mojom.h"
+
+namespace blink {
+
+const FeaturePolicyFeatureList& GetFeaturePolicyFeatureList() {
+  static const base::NoDestructor<FeaturePolicyFeatureList> feature_list({
+    {%- for feature in features %}
+      {
+        mojom::FeaturePolicyFeature::k{{feature.name}},
+        FeaturePolicyFeatureDefault::{{feature.feature_default}}
+      },
+    {%- endfor %}
+  });
+  return *feature_list;
+}
+
+} // namespace blink

third_party/blink/renderer/core/feature_policy/feature_policy_features.json5

@@ -23,6 +23,14 @@
     // which gets parsed from the header or the allow attribute.
     feature_policy_name: {
     },
+
+    // feature_default: Default allowlist state for feature. This corresponds
+    // to enum class |FeaturePolicyFeatureDefault| in
+    // public/common/feature_policy/feature_policy_features.h
+    feature_default: {
+      default: "EnableForSelf",
+      valid_values: ["DisableForAll", "EnableForAll", "EnableForSelf"]
+    }
   },
 
   data: [
@@ -76,6 +84,7 @@
     {
       name: "ClientHintUA",
       feature_policy_name: "ch-ua",
+      feature_default: "EnableForAll",
       depends_on: ["FeaturePolicyForClientHints"],
     },
     {
@@ -96,6 +105,7 @@
     {
       name: "ClientHintUAMobile",
       feature_policy_name: "ch-ua-mobile",
+      feature_default: "EnableForAll",
       depends_on: ["FeaturePolicyForClientHints"],
     },
     {
@@ -139,15 +149,18 @@
     {
       name: "DocumentDomain",
       feature_policy_name: "document-domain",
+      feature_default: "EnableForAll",
     },
     {
       name: "DocumentWrite",
       feature_policy_name: "document-write",
+      feature_default: "EnableForAll",
       depends_on: ["ExperimentalProductivityFeatures"],
     },
     {
       name: "Downloads",
       feature_policy_name: "downloads",
+      feature_default: "EnableForAll",
       depends_on: ["FeaturePolicyForSandbox"],
     },
     {
@@ -157,21 +170,25 @@
     {
       name: "ExecutionWhileOutOfViewport",
       feature_policy_name: "execution-while-out-of-viewport",
+      feature_default: "EnableForAll",
       depends_on: ["FreezeFramesOnVisibility"],
     },
     {
       name: "ExecutionWhileNotRendered",
       feature_policy_name: "execution-while-not-rendered",
+      feature_default: "EnableForAll",
       depends_on: ["FreezeFramesOnVisibility"],
     },
     {
       name: "FocusWithoutUserActivation",
       feature_policy_name: "focus-without-user-activation",
+      feature_default: "EnableForAll",
       depends_on: ["BlockingFocusWithoutUserActivation"],
     },
     {
       name: "FormSubmission",
       feature_policy_name: "forms",
+      feature_default: "EnableForAll",
       depends_on: ["FeaturePolicyForSandbox"],
     },
     {
@@ -216,11 +233,13 @@
     {
       name: "Modals",
       feature_policy_name: "modals",
+      feature_default: "EnableForAll",
       depends_on: ["FeaturePolicyForSandbox"],
     },
     {
       name: "OrientationLock",
       feature_policy_name: "orientation-lock",
+      feature_default: "EnableForAll",
       depends_on: ["FeaturePolicyForSandbox"],
     },
     {
@@ -231,21 +250,25 @@
     {
       name: "PictureInPicture",
       feature_policy_name: "picture-in-picture",
+      feature_default: "EnableForAll",
       depends_on: ["PictureInPictureAPI"],
     },
     {
       name: "PointerLock",
       feature_policy_name: "pointer-lock",
+      feature_default: "EnableForAll",
       depends_on: ["FeaturePolicyForSandbox"],
     },
     {
       name: "Popups",
       feature_policy_name: "popups",
+      feature_default: "EnableForAll",
       depends_on: ["FeaturePolicyForSandbox"],
     },
     {
       name: "Presentation",
       feature_policy_name: "presentation",
+      feature_default: "EnableForAll",
       depends_on: ["FeaturePolicyForSandbox"],
     },
     {
@@ -261,6 +284,7 @@
     {
       name: "Script",
       feature_policy_name: "scripts",
+      feature_default: "EnableForAll",
       depends_on: ["FeaturePolicyForSandbox"],
     },
     {
@@ -270,21 +294,25 @@
     },
     {
       name: "StorageAccessAPI",
+      feature_default: "EnableForAll",
       feature_policy_name: "storage-access-api",
       depends_on: ["StorageAccessAPI"],
     },
     {
       name: "SyncScript",
       feature_policy_name: "sync-script",
+      feature_default: "EnableForAll",
       depends_on: ["ExperimentalProductivityFeatures"],
     },
     {
       name: "SyncXHR",
+      feature_default: "EnableForAll",
       feature_policy_name: "sync-xhr",
     },
     {
       name: "TopNavigation",
       feature_policy_name: "top-navigation",
+      feature_default: "EnableForAll",
       depends_on: ["FeaturePolicyForSandbox"],
     },
     {
@@ -300,6 +328,7 @@
     {
       name: "VerticalScroll",
       feature_policy_name: "vertical-scroll",
+      feature_default: "EnableForAll",
       depends_on: ["ExperimentalProductivityFeatures"],
     },
     {