Simplify OriginTrialContext initialization and its interaction with FeaturePolicyParser

* Move OriginTrialContext from SecurityContext to ExecutionContext,
  since it is unused for RemoteSecurityContexts.
* Construct the OriginTrialContext directly in the ExecutionContext
  constructor, rather than requiring subclasses to do later. This
  requires removing one of the constructor variants that is only used
  for testing, so add SetTrialTokenValidatorForTesting() to
  OriginTrialContext for that test.
* SecurityContextInit is a FeaturePolicyParserDelegate. This is
  because it needs to provide access to the OriginTrialContext
   during FeaturePolicy calculation, because the ExecutionContext's
  OriginTrialContext was not set up when the FeaturePolicy was
  calculated. Now it is. Remove all of the plumbing related to
  SecurityContextInit being a FeaturePolicyParserDelegate, and pass
  SecurityContextInit::execution_context_ where needed instead.
* This means ExecutionContext is the sole FeaturePolicyParserDelegate.
  Have FeaturePolicyParser reference the ExecutionContext directly and
  remove the FeaturePolicyParserDelegate layer.

Change-Id: If57297f1bfefc72a5378dd55e6adc52d80ce8461
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2293475
Commit-Queue: Nate Chapin <japhet@chromium.org>
Auto-Submit: Nate Chapin <japhet@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#788665}

third_party/blink/renderer/core/execution_context/execution_context.cc

@@ -67,18 +67,13 @@ ExecutionContext::ExecutionContext(v8::Isolate* isolate, Agent* agent)
       csp_delegate_(MakeGarbageCollected<ExecutionContextCSPDelegate>(*this)),
       window_interaction_tokens_(0),
       referrer_policy_(network::mojom::ReferrerPolicy::kDefault),
-      address_space_(network::mojom::blink::IPAddressSpace::kUnknown) {
+      address_space_(network::mojom::blink::IPAddressSpace::kUnknown),
+      origin_trial_context_(MakeGarbageCollected<OriginTrialContext>(this)) {
   DCHECK(agent_);
 }
 
 ExecutionContext::~ExecutionContext() = default;
 
-void ExecutionContext::Initialize(const SecurityContextInit& init) {
-  security_context_.Initialize(init);
-  if (GetOriginTrialContext())
-    GetOriginTrialContext()->BindExecutionContext(this);
-}
-
 // static
 ExecutionContext* ExecutionContext::From(const ScriptState* script_state) {
   v8::HandleScope scope(script_state->GetIsolate());
@@ -379,6 +374,7 @@ void ExecutionContext::Trace(Visitor* visitor) const {
   visitor->Trace(csp_delegate_);
   visitor->Trace(timers_);
   visitor->Trace(context_lifecycle_observer_list_);
+  visitor->Trace(origin_trial_context_);
   ContextLifecycleNotifier::Trace(visitor);
   ConsoleLogger::Trace(visitor);
   Supplementable<ExecutionContext>::Trace(visitor);
@@ -401,25 +397,7 @@ v8::MicrotaskQueue* ExecutionContext::GetMicrotaskQueue() const {
 }
 
 bool ExecutionContext::FeatureEnabled(OriginTrialFeature feature) const {
-  return GetOriginTrialContext() &&
-         GetOriginTrialContext()->IsFeatureEnabled(feature);
-}
-
-void ExecutionContext::CountFeaturePolicyUsage(mojom::WebFeature feature) {
-  UseCounter::Count(*this, feature);
-}
-
-bool ExecutionContext::FeaturePolicyFeatureObserved(
-    mojom::blink::FeaturePolicyFeature feature) {
-  size_t feature_index = static_cast<size_t>(feature);
-  if (parsed_feature_policies_.size() == 0) {
-    parsed_feature_policies_.resize(
-        static_cast<size_t>(mojom::blink::FeaturePolicyFeature::kMaxValue) + 1);
-  } else if (parsed_feature_policies_[feature_index]) {
-    return true;
-  }
-  parsed_feature_policies_[feature_index] = true;
-  return false;
+  return origin_trial_context_->IsFeatureEnabled(feature);
 }
 
 void ExecutionContext::FeaturePolicyPotentialBehaviourChangeObserved(

third_party/blink/renderer/core/execution_context/execution_context.h

@@ -44,7 +44,6 @@
 #include "third_party/blink/renderer/core/core_export.h"
 #include "third_party/blink/renderer/core/execution_context/execution_context_lifecycle_observer.h"
 #include "third_party/blink/renderer/core/execution_context/security_context.h"
-#include "third_party/blink/renderer/core/feature_policy/feature_policy_parser_delegate.h"
 #include "third_party/blink/renderer/core/frame/dom_timer_coordinator.h"
 #include "third_party/blink/renderer/platform/context_lifecycle_notifier.h"
 #include "third_party/blink/renderer/platform/heap/handle.h"
@@ -121,8 +120,7 @@ class CORE_EXPORT ExecutionContext : public Supplementable<ExecutionContext>,
                                      public ContextLifecycleNotifier,
                                      public ConsoleLogger,
                                      public UseCounter,
-                                     public FeaturePolicyParserDelegate {
-
+                                     public FeatureContext {
  public:
   void Trace(Visitor*) const override;
 
@@ -136,8 +134,6 @@ class CORE_EXPORT ExecutionContext : public Supplementable<ExecutionContext>,
   static ExecutionContext* ForRelevantRealm(
       const v8::FunctionCallbackInfo<v8::Value>&);
 
-  void Initialize(const SecurityContextInit&);
-
   virtual bool IsWindow() const { return false; }
   virtual bool IsWorkerOrWorkletGlobalScope() const { return false; }
   virtual bool IsWorkerGlobalScope() const { return false; }
@@ -312,17 +308,14 @@ class CORE_EXPORT ExecutionContext : public Supplementable<ExecutionContext>,
   v8::MicrotaskQueue* GetMicrotaskQueue() const;
 
   OriginTrialContext* GetOriginTrialContext() const {
-    return security_context_.GetOriginTrialContext();
+    return origin_trial_context_;
   }
 
   virtual TrustedTypePolicyFactory* GetTrustedTypes() const { return nullptr; }
   virtual bool RequireTrustedTypes() const;
 
-  // FeaturePolicyParserDelegate override
+  // FeatureContext override
   bool FeatureEnabled(OriginTrialFeature) const override;
-  void CountFeaturePolicyUsage(mojom::WebFeature feature) override;
-  bool FeaturePolicyFeatureObserved(
-      mojom::blink::FeaturePolicyFeature feature) override;
 
   // Tests whether the policy-controlled feature is enabled in this frame.
   // Optionally sends a report to any registered reporting observers or
@@ -440,10 +433,7 @@ class CORE_EXPORT ExecutionContext : public Supplementable<ExecutionContext>,
 
   network::mojom::blink::IPAddressSpace address_space_;
 
-  // Tracks which feature policies have already been parsed, so as not to count
-  // them multiple times.
-  // The size of this vector is 0 until FeaturePolicyFeatureObserved is called.
-  Vector<bool> parsed_feature_policies_;
+  Member<OriginTrialContext> origin_trial_context_;
 
   // Tracks which feature policy features have been logged in this execution
   // context as to the FeaturePolicyProposalWouldChangeBehaviour

third_party/blink/renderer/core/execution_context/security_context.cc

@@ -35,9 +35,8 @@
 #include "third_party/blink/public/mojom/feature_policy/policy_value.mojom-blink.h"
 #include "third_party/blink/public/mojom/security_context/insecure_request_policy.mojom-blink.h"
 #include "third_party/blink/public/platform/platform.h"
-#include "third_party/blink/renderer/core/execution_context/security_context_init.h"
+#include "third_party/blink/renderer/core/execution_context/execution_context.h"
 #include "third_party/blink/renderer/core/frame/csp/content_security_policy.h"
-#include "third_party/blink/renderer/core/origin_trials/origin_trial_context.h"
 #include "third_party/blink/renderer/platform/runtime_enabled_features.h"
 #include "third_party/blink/renderer/platform/weborigin/security_origin.h"
 
@@ -64,14 +63,9 @@ SecurityContext::SecurityContext(ExecutionContext* execution_context)
 
 SecurityContext::~SecurityContext() = default;
 
-void SecurityContext::Initialize(const SecurityContextInit& init) {
-  origin_trial_context_ = init.GetOriginTrialContext();
-}
-
 void SecurityContext::Trace(Visitor* visitor) const {
   visitor->Trace(execution_context_);
   visitor->Trace(content_security_policy_);
-  visitor->Trace(origin_trial_context_);
 }
 
 void SecurityContext::SetSecurityOrigin(

third_party/blink/renderer/core/execution_context/security_context.h

@@ -50,8 +50,6 @@ class DocumentPolicy;
 class ExecutionContext;
 class FeaturePolicy;
 class PolicyValue;
-class OriginTrialContext;
-class SecurityContextInit;
 class SecurityOrigin;
 struct ParsedFeaturePolicyDeclaration;
 
@@ -78,8 +76,6 @@ class CORE_EXPORT SecurityContext {
   explicit SecurityContext(ExecutionContext*);
   virtual ~SecurityContext();
 
-  void Initialize(const SecurityContextInit&);
-
   void Trace(Visitor*) const;
 
   using InsecureNavigationsSet = HashSet<unsigned, WTF::AlreadyHashed>;
@@ -168,10 +164,6 @@ class CORE_EXPORT SecurityContext {
   FeatureStatus IsFeatureEnabled(mojom::blink::DocumentPolicyFeature,
                                  PolicyValue threshold_value) const;
 
-  OriginTrialContext* GetOriginTrialContext() const {
-    return origin_trial_context_;
-  }
-
   SecureContextMode GetSecureContextMode() const {
     return secure_context_mode_;
   }
@@ -192,7 +184,6 @@ class CORE_EXPORT SecurityContext {
   InsecureNavigationsSet insecure_navigations_to_upgrade_;
   bool require_safe_types_ = false;
   SecureContextMode secure_context_mode_ = SecureContextMode::kInsecureContext;
-  Member<OriginTrialContext> origin_trial_context_;
   DISALLOW_COPY_AND_ASSIGN(SecurityContext);
 };
 

third_party/blink/renderer/core/execution_context/security_context_init.cc

@@ -32,19 +32,19 @@ namespace {
 // ParsedDocumentPolicy.
 DocumentPolicy::ParsedDocumentPolicy FilterByOriginTrial(
     const DocumentPolicy::ParsedDocumentPolicy& parsed_policy,
-    SecurityContextInit* init) {
+    ExecutionContext* context) {
   DocumentPolicy::ParsedDocumentPolicy filtered_policy;
   for (auto i = parsed_policy.feature_state.begin(),
             last = parsed_policy.feature_state.end();
        i != last;) {
-    if (!DisabledByOriginTrial(i->first, init))
+    if (!DisabledByOriginTrial(i->first, context))
       filtered_policy.feature_state.insert(*i);
     ++i;
   }
   for (auto i = parsed_policy.endpoint_map.begin(),
             last = parsed_policy.endpoint_map.end();
        i != last;) {
-    if (!DisabledByOriginTrial(i->first, init))
+    if (!DisabledByOriginTrial(i->first, context))
       filtered_policy.endpoint_map.insert(*i);
     ++i;
   }
@@ -71,42 +71,22 @@ void MergeFeaturesFromOriginPolicy(WTF::StringBuilder& feature_policy,
 
 }  // namespace
 
-SecurityContextInit::SecurityContextInit(OriginTrialContext* origin_trials)
-    : origin_trials_(origin_trials) {}
-
 // A helper class that allows the security context be initialized in the
 // process of constructing the document.
 SecurityContextInit::SecurityContextInit(ExecutionContext* context)
     : execution_context_(context) {}
 
-void SecurityContextInit::CountFeaturePolicyUsage(
-    mojom::blink::WebFeature feature) {
-  if (execution_context_)
-    execution_context_->CountFeaturePolicyUsage(feature);
-}
-
-bool SecurityContextInit::FeaturePolicyFeatureObserved(
-    mojom::blink::FeaturePolicyFeature feature) {
-  return execution_context_ &&
-         execution_context_->FeaturePolicyFeatureObserved(feature);
-}
-
-bool SecurityContextInit::FeatureEnabled(OriginTrialFeature feature) const {
-  return origin_trials_->IsFeatureEnabled(feature);
-}
-
 void SecurityContextInit::ApplyDocumentPolicy(
     DocumentPolicy::ParsedDocumentPolicy& document_policy,
     const String& report_only_document_policy_header) {
-  DCHECK(origin_trials_);
-  if (!RuntimeEnabledFeatures::DocumentPolicyEnabled(this))
+  if (!RuntimeEnabledFeatures::DocumentPolicyEnabled(execution_context_))
     return;
 
   // Because Document-Policy http header is parsed in DocumentLoader,
   // when origin trial context is not initialized yet.
   // Needs to filter out features that are not in origin trial after
   // we have origin trial information available.
-  document_policy = FilterByOriginTrial(document_policy, this);
+  document_policy = FilterByOriginTrial(document_policy, execution_context_);
   if (!document_policy.feature_state.empty()) {
     UseCounter::Count(execution_context_, WebFeature::kDocumentPolicyHeader);
     for (const auto& policy_entry : document_policy.feature_state) {
@@ -131,7 +111,7 @@ void SecurityContextInit::ApplyDocumentPolicy(
           report_only_document_policy_header, logger);
   if (report_only_parsed_policy) {
     report_only_document_policy =
-        FilterByOriginTrial(*report_only_parsed_policy, this);
+        FilterByOriginTrial(*report_only_parsed_policy, execution_context_);
     if (!report_only_document_policy.feature_state.empty()) {
       UseCounter::Count(execution_context_,
                         WebFeature::kDocumentPolicyReportOnlyHeader);
@@ -146,8 +126,6 @@ void SecurityContextInit::ApplyFeaturePolicy(
     const ResourceResponse& response,
     const base::Optional<WebOriginPolicy>& origin_policy,
     const FramePolicy& frame_policy) {
-  DCHECK(origin_trials_);
-
   // If we are a HTMLViewSourceDocument we use container, header or
   // inherited policies. https://crbug.com/898688.
   if (frame->InViewSourceMode()) {
@@ -182,14 +160,15 @@ void SecurityContextInit::ApplyFeaturePolicy(
 
   feature_policy_header_ = FeaturePolicyParser::ParseHeader(
       feature_policy_header, permissions_policy_header,
-      execution_context_->GetSecurityOrigin(), feature_policy_logger, this);
+      execution_context_->GetSecurityOrigin(), feature_policy_logger,
+      execution_context_);
 
   ParsedFeaturePolicy report_only_feature_policy_header =
       FeaturePolicyParser::ParseHeader(
           response.HttpHeaderField(http_names::kFeaturePolicyReportOnly),
           report_only_permissions_policy_header,
           execution_context_->GetSecurityOrigin(),
-          report_only_feature_policy_logger, this);
+          report_only_feature_policy_logger, execution_context_);
 
   if (!report_only_feature_policy_header.empty()) {
     UseCounter::Count(execution_context_,
@@ -280,18 +259,4 @@ void SecurityContextInit::ApplyFeaturePolicy(
   }
 }
 
-void SecurityContextInit::InitializeOriginTrials(
-    const String& origin_trials_header) {
-  origin_trials_ = MakeGarbageCollected<OriginTrialContext>();
-  if (origin_trials_header.IsEmpty())
-    return;
-  std::unique_ptr<Vector<String>> tokens(
-      OriginTrialContext::ParseHeaderValue(origin_trials_header));
-  if (!tokens)
-    return;
-  origin_trials_->AddTokens(*tokens, execution_context_->GetSecurityOrigin(),
-                            execution_context_->GetSecureContextMode() ==
-                                SecureContextMode::kSecureContext);
-}
-
 }  // namespace blink

third_party/blink/renderer/core/execution_context/security_context_init.h

@@ -20,19 +20,14 @@
 
 namespace blink {
 class LocalFrame;
-class OriginTrialContext;
 class ResourceResponse;
 
-class CORE_EXPORT SecurityContextInit : public FeaturePolicyParserDelegate {
+class CORE_EXPORT SecurityContextInit {
   STACK_ALLOCATED();
 
  public:
-  // The first constructor is for workers and tests. The second is for windows.
-  // TODO(japhet): Merge these.
-  explicit SecurityContextInit(OriginTrialContext*);
   explicit SecurityContextInit(ExecutionContext*);
 
-  void InitializeOriginTrials(const String& origin_trials_header);
   void ApplyFeaturePolicy(LocalFrame* frame,
                           const ResourceResponse& response,
                           const base::Optional<WebOriginPolicy>& origin_policy,
@@ -45,17 +40,9 @@ class CORE_EXPORT SecurityContextInit : public FeaturePolicyParserDelegate {
     return feature_policy_header_;
   }
 
-  OriginTrialContext* GetOriginTrialContext() const { return origin_trials_; }
-
-  void CountFeaturePolicyUsage(mojom::blink::WebFeature feature) override;
-  bool FeaturePolicyFeatureObserved(
-      mojom::blink::FeaturePolicyFeature) override;
-  bool FeatureEnabled(OriginTrialFeature feature) const override;
-
  private:
   ExecutionContext* execution_context_ = nullptr;
   ParsedFeaturePolicy feature_policy_header_;
-  OriginTrialContext* origin_trials_ = nullptr;
 };
 
 }  // namespace blink

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

@@ -12,7 +12,6 @@ blink_core_sources("feature_policy") {
     "dom_feature_policy.h",
     "feature_policy_parser.cc",
     "feature_policy_parser.h",
-    "feature_policy_parser_delegate.h",
     "iframe_policy.h",
     "layout_animations_policy.cc",
     "layout_animations_policy.h",

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

@@ -36,18 +36,60 @@ struct FeaturePolicyDeclarationNode {
 using FeaturePolicyNode = Vector<FeaturePolicyDeclarationNode>;
 }  // namespace internal
 
+class ParsedFeaturePolicies final
+    : public GarbageCollected<ParsedFeaturePolicies>,
+      public Supplement<ExecutionContext> {
+  USING_GARBAGE_COLLECTED_MIXIN(ParsedFeaturePolicies);
+
+ public:
+  static const char kSupplementName[];
+
+  static ParsedFeaturePolicies& From(ExecutionContext& context) {
+    ParsedFeaturePolicies* policies =
+        Supplement<ExecutionContext>::From<ParsedFeaturePolicies>(context);
+    if (!policies) {
+      policies = MakeGarbageCollected<ParsedFeaturePolicies>(context);
+      Supplement<ExecutionContext>::ProvideTo(context, policies);
+    }
+    return *policies;
+  }
+
+  explicit ParsedFeaturePolicies(ExecutionContext& context)
+      : Supplement<ExecutionContext>(context),
+        policies_(
+            static_cast<size_t>(mojom::blink::FeaturePolicyFeature::kMaxValue) +
+            1) {}
+
+  bool Observed(mojom::blink::FeaturePolicyFeature feature) {
+    size_t feature_index = static_cast<size_t>(feature);
+    if (policies_[feature_index])
+      return true;
+    policies_[feature_index] = true;
+    return false;
+  }
+
+ private:
+  // Tracks which feature policies have already been parsed, so as not to count
+  // them multiple times.
+  Vector<bool> policies_;
+};
+
+const char ParsedFeaturePolicies::kSupplementName[] = "ParsedFeaturePolicies";
+
 class ParsingContext {
+  STACK_ALLOCATED();
+
  public:
   ParsingContext(PolicyParserMessageBuffer& logger,
                  scoped_refptr<const SecurityOrigin> self_origin,
                  scoped_refptr<const SecurityOrigin> src_origin,
                  const FeatureNameMap& feature_names,
-                 FeaturePolicyParserDelegate* delegate)
+                 ExecutionContext* execution_context)
       : logger_(logger),
         self_origin_(self_origin),
         src_origin_(src_origin),
         feature_names_(feature_names),
-        delegate_(delegate) {}
+        execution_context_(execution_context) {}
 
   ~ParsingContext() = default;
 
@@ -79,8 +121,6 @@ class ParsingContext {
 
   bool FeatureObserved(mojom::blink::FeaturePolicyFeature feature);
 
-  void ReportFeaturePolicyWebFeatureUsage(mojom::blink::WebFeature feature);
-
   void ReportFeatureUsage(mojom::blink::FeaturePolicyFeature feature);
 
   // This function should be called after Allowlist Histograms related flags
@@ -98,7 +138,7 @@ class ParsingContext {
   scoped_refptr<const SecurityOrigin> self_origin_;
   scoped_refptr<const SecurityOrigin> src_origin_;
   const FeatureNameMap& feature_names_;
-  FeaturePolicyParserDelegate* delegate_;
+  ExecutionContext* execution_context_;
 
   // Flags for the types of items which can be used in allowlists.
   bool allowlist_includes_star_ = false;
@@ -123,16 +163,11 @@ bool ParsingContext::FeatureObserved(
   }
 }
 
-void ParsingContext::ReportFeaturePolicyWebFeatureUsage(
-    mojom::blink::WebFeature feature) {
-  if (delegate_)
-    delegate_->CountFeaturePolicyUsage(feature);
-}
-
 void ParsingContext::ReportFeatureUsage(
     mojom::blink::FeaturePolicyFeature feature) {
   if (src_origin_) {
-    if (!delegate_ || !delegate_->FeaturePolicyFeatureObserved(feature)) {
+    if (!execution_context_ ||
+        !ParsedFeaturePolicies::From(*execution_context_).Observed(feature)) {
       UMA_HISTOGRAM_ENUMERATION("Blink.UseCounter.FeaturePolicy.Allow",
                                 feature);
     }
@@ -196,7 +231,7 @@ ParsingContext::ParseFeatureName(const String& feature_name) {
     logger_.Warn("Unrecognized feature: '" + feature_name + "'.");
     return base::nullopt;
   }
-  if (DisabledByOriginTrial(feature_name, delegate_)) {
+  if (DisabledByOriginTrial(feature_name, execution_context_)) {
     logger_.Warn("Origin trial controlled feature not enabled: '" +
                  feature_name + "'.");
     return base::nullopt;
@@ -362,7 +397,8 @@ internal::FeaturePolicyNode ParsingContext::ParseFeaturePolicyToIR(
   policy.Split(',', policy_items);
 
   if (policy_items.size() > 1) {
-    ReportFeaturePolicyWebFeatureUsage(
+    UseCounter::Count(
+        execution_context_,
         mojom::blink::WebFeature::kFeaturePolicyCommaSeparatedDeclarations);
   }
 
@@ -372,9 +408,9 @@ internal::FeaturePolicyNode ParsingContext::ParseFeaturePolicyToIR(
     item.Split(';', feature_entries);
 
     if (feature_entries.size() > 1) {
-      ReportFeaturePolicyWebFeatureUsage(
-          mojom::blink::WebFeature::
-              kFeaturePolicySemicolonSeparatedDeclarations);
+      UseCounter::Count(execution_context_,
+                        mojom::blink::WebFeature::
+                            kFeaturePolicySemicolonSeparatedDeclarations);
     }
 
     for (const String& feature_entry : feature_entries) {
@@ -475,9 +511,9 @@ ParsedFeaturePolicy FeaturePolicyParser::ParseHeader(
     const String& permissions_policy_header,
     scoped_refptr<const SecurityOrigin> origin,
     PolicyParserMessageBuffer& logger,
-    FeaturePolicyParserDelegate* delegate) {
+    ExecutionContext* execution_context) {
   ParsingContext context(logger, origin, nullptr, GetDefaultFeatureNameMap(),
-                         delegate);
+                         execution_context);
   auto policy_ir =
       context.ParsePermissionsPolicyToIR(permissions_policy_header);
   policy_ir.AppendVector(context.ParseFeaturePolicyToIR(feature_policy_header));
@@ -489,9 +525,9 @@ ParsedFeaturePolicy FeaturePolicyParser::ParseAttribute(
     scoped_refptr<const SecurityOrigin> self_origin,
     scoped_refptr<const SecurityOrigin> src_origin,
     PolicyParserMessageBuffer& logger,
-    FeaturePolicyParserDelegate* delegate) {
+    ExecutionContext* execution_context) {
   ParsingContext context(logger, self_origin, src_origin,
-                         GetDefaultFeatureNameMap(), delegate);
+                         GetDefaultFeatureNameMap(), execution_context);
   return context.ParseIR(context.ParseFeaturePolicyToIR(policy));
 }
 
@@ -501,9 +537,9 @@ ParsedFeaturePolicy FeaturePolicyParser::ParseFeaturePolicyForTest(
     scoped_refptr<const SecurityOrigin> src_origin,
     PolicyParserMessageBuffer& logger,
     const FeatureNameMap& feature_names,
-    FeaturePolicyParserDelegate* delegate) {
+    ExecutionContext* execution_context) {
   ParsingContext context(logger, self_origin, src_origin, feature_names,
-                         delegate);
+                         execution_context);
   return context.ParseIR(context.ParseFeaturePolicyToIR(policy));
 }
 
@@ -513,9 +549,9 @@ ParsedFeaturePolicy FeaturePolicyParser::ParsePermissionsPolicyForTest(
     scoped_refptr<const SecurityOrigin> src_origin,
     PolicyParserMessageBuffer& logger,
     const FeatureNameMap& feature_names,
-    FeaturePolicyParserDelegate* delegate) {
+    ExecutionContext* execution_context) {
   ParsingContext context(logger, self_origin, src_origin, feature_names,
-                         delegate);
+                         execution_context);
   return context.ParseIR(context.ParsePermissionsPolicyToIR(policy));
 }
 

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

@@ -19,7 +19,6 @@
 namespace blink {
 
 class ExecutionContext;
-class FeaturePolicyParserDelegate;
 
 // These values match the "FeaturePolicyAllowlistType" enum in
 // tools/metrics/histograms/enums.xml. Entries should not be renumbered and
@@ -57,12 +56,11 @@ class CORE_EXPORT FeaturePolicyParser {
   // ExecutionContext is used to determine if any origin trials affect the
   // parsing. Example of a feature policy string:
   //     "vibrate a.com b.com; fullscreen 'none'; payment 'self', payment *".
-  static ParsedFeaturePolicy ParseHeader(
-      const String& feature_policy_header,
-      const String& permission_policy_header,
-      scoped_refptr<const SecurityOrigin>,
-      PolicyParserMessageBuffer& logger,
-      FeaturePolicyParserDelegate* delegate = nullptr);
+  static ParsedFeaturePolicy ParseHeader(const String& feature_policy_header,
+                                         const String& permission_policy_header,
+                                         scoped_refptr<const SecurityOrigin>,
+                                         PolicyParserMessageBuffer& logger,
+                                         ExecutionContext* = nullptr);
 
   // Converts a container policy string into a vector of allowlists, given self
   // and src origins provided, one for each feature specified. Unrecognized
@@ -74,7 +72,7 @@ class CORE_EXPORT FeaturePolicyParser {
       scoped_refptr<const SecurityOrigin> self_origin,
       scoped_refptr<const SecurityOrigin> src_origin,
       PolicyParserMessageBuffer& logger,
-      FeaturePolicyParserDelegate* delegate = nullptr);
+      ExecutionContext* = nullptr);
 
   static ParsedFeaturePolicy ParseFeaturePolicyForTest(
       const String& policy,
@@ -82,7 +80,7 @@ class CORE_EXPORT FeaturePolicyParser {
       scoped_refptr<const SecurityOrigin> src_origin,
       PolicyParserMessageBuffer& logger,
       const FeatureNameMap& feature_names,
-      FeaturePolicyParserDelegate* delegate = nullptr);
+      ExecutionContext* = nullptr);
 
   static ParsedFeaturePolicy ParsePermissionsPolicyForTest(
       const String& policy,
@@ -90,7 +88,7 @@ class CORE_EXPORT FeaturePolicyParser {
       scoped_refptr<const SecurityOrigin> src_origin,
       PolicyParserMessageBuffer& logger,
       const FeatureNameMap& feature_names,
-      FeaturePolicyParserDelegate* delegate = nullptr);
+      ExecutionContext* = nullptr);
 };
 
 // Returns true iff any declaration in the policy is for the given feature.

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

-// Copyright 2019 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_FEATURE_POLICY_PARSER_DELEGATE_H_
-#define THIRD_PARTY_BLINK_RENDERER_CORE_FEATURE_POLICY_FEATURE_POLICY_PARSER_DELEGATE_H_
-
-#include "third_party/blink/public/mojom/feature_policy/feature_policy_feature.mojom-blink-forward.h"
-#include "third_party/blink/renderer/core/frame/web_feature_forward.h"
-#include "third_party/blink/renderer/platform/runtime_enabled_features.h"
-
-namespace blink {
-
-class FeaturePolicyParserDelegate : public FeatureContext {
- public:
-  virtual void CountFeaturePolicyUsage(mojom::WebFeature feature) = 0;
-  virtual bool FeaturePolicyFeatureObserved(
-      mojom::blink::FeaturePolicyFeature feature) = 0;
-};
-
-}  // namespace blink
-
-#endif  // THIRD_PARTY_BLINK_RENDERER_CORE_FEATURE_POLICY_FEATURE_POLICY_PARSER_DELEGATE_H_

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

@@ -104,9 +104,9 @@ class FeaturePolicyParserTest : public ::testing::Test {
       const String& feature_policy_header,
       scoped_refptr<const SecurityOrigin> origin,
       PolicyParserMessageBuffer& logger,
-      FeaturePolicyParserDelegate* delegate = nullptr) {
+      ExecutionContext* context = nullptr) {
     return FeaturePolicyParser::ParseHeader(
-        feature_policy_header, g_empty_string, origin, logger, delegate);
+        feature_policy_header, g_empty_string, origin, logger, context);
   }
 };
 
@@ -148,16 +148,15 @@ class FeaturePolicyParserParsingTest
   }
 
  protected:
-  ParsedFeaturePolicy ParseFeaturePolicy(
-      const char* policy_string,
-      const char* self_origin_string,
-      const char* src_origin_string,
-      PolicyParserMessageBuffer& logger,
-      const FeatureNameMap& feature_names,
-      FeaturePolicyParserDelegate* delegate = nullptr) {
+  ParsedFeaturePolicy ParseFeaturePolicy(const char* policy_string,
+                                         const char* self_origin_string,
+                                         const char* src_origin_string,
+                                         PolicyParserMessageBuffer& logger,
+                                         const FeatureNameMap& feature_names,
+                                         ExecutionContext* context = nullptr) {
     return FeaturePolicyParser::ParseFeaturePolicyForTest(
         policy_string, SecurityOrigin::CreateFromString(self_origin_string),
-        GetSrcOrigin(src_origin_string), logger, feature_names, delegate);
+        GetSrcOrigin(src_origin_string), logger, feature_names, context);
   }
 
   ParsedFeaturePolicy ParsePermissionsPolicy(
@@ -166,10 +165,10 @@ class FeaturePolicyParserParsingTest
       const char* src_origin_string,
       PolicyParserMessageBuffer& logger,
       const FeatureNameMap& feature_names,
-      FeaturePolicyParserDelegate* delegate = nullptr) {
+      ExecutionContext* context = nullptr) {
     return FeaturePolicyParser::ParsePermissionsPolicyForTest(
         policy_string, SecurityOrigin::CreateFromString(self_origin_string),
-        GetSrcOrigin(src_origin_string), logger, feature_names, delegate);
+        GetSrcOrigin(src_origin_string), logger, feature_names, context);
   }
 
   void CheckParsedPolicy(const ParsedFeaturePolicy& actual,
@@ -558,7 +557,7 @@ TEST_F(FeaturePolicyParserParsingTest,
   CheckParsedPolicy(FeaturePolicyParser::ParseHeader(
                         "geolocation 'none', fullscreen 'self'",
                         "geolocation=self, payment=*", origin_a_.get(), logger,
-                        nullptr /* delegate */),
+                        nullptr /* context */),
                     {
                         {
                             mojom::blink::FeaturePolicyFeature::kGeolocation,

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

@@ -1698,15 +1698,18 @@ void DocumentLoader::CommitNavigation() {
   frame_->DomWindow()->GetSecurityContext().SetContentSecurityPolicy(
       content_security_policy_.Get());
   frame_->DomWindow()->GetSecurityContext().ApplySandboxFlags(sandbox_flags);
+  // Conceptually, SecurityOrigin doesn't have to be initialized after sandbox
+  // flags are applied, but there's a UseCounter in SetSecurityOrigin() that
+  // wants to inspect sandbox flags.
   frame_->DomWindow()->GetSecurityContext().SetSecurityOrigin(
       std::move(security_origin));
+  // Requires SecurityOrigin to be initialized.
+  OriginTrialContext::AddTokensFromHeader(
+      frame_->DomWindow(), response_.HttpHeaderField(http_names::kOriginTrial));
 
   SecurityContextInit security_init(frame_->DomWindow());
-  security_init.InitializeOriginTrials(
-      response_.HttpHeaderField(http_names::kOriginTrial));
-
-  frame_->DomWindow()->Initialize(security_init);
-
+  // FeaturePolicy and DocumentPolicy require SecurityOrigin and origin trials
+  // to be initialized.
   // TODO(iclelland): Add Feature-Policy-Report-Only to Origin Policy.
   security_init.ApplyFeaturePolicy(frame_.Get(), response_, origin_policy_,
                                    frame_policy_);

third_party/blink/renderer/core/origin_trials/origin_trial_context.cc

@@ -131,17 +131,13 @@ std::ostream& operator<<(std::ostream& stream, OriginTrialTokenStatus status) {
 
 }  // namespace
 
-OriginTrialContext::OriginTrialContext()
-    : OriginTrialContext(TrialTokenValidator::Policy()
-                             ? std::make_unique<TrialTokenValidator>()
-                             : nullptr) {}
+OriginTrialContext::OriginTrialContext(ExecutionContext* context)
+    : trial_token_validator_(std::make_unique<TrialTokenValidator>()),
+      context_(context) {}
 
-OriginTrialContext::OriginTrialContext(
-    std::unique_ptr<TrialTokenValidator> validator)
-    : trial_token_validator_(std::move(validator)) {}
-
-void OriginTrialContext::BindExecutionContext(ExecutionContext* context) {
-  context_ = context;
+void OriginTrialContext::SetTrialTokenValidatorForTesting(
+    std::unique_ptr<TrialTokenValidator> validator) {
+  trial_token_validator_ = std::move(validator);
 }
 
 // static
@@ -267,19 +263,13 @@ void OriginTrialContext::AddTokenInternal(const String& token,
 }
 
 void OriginTrialContext::AddTokens(const Vector<String>& tokens) {
-  AddTokens(tokens, GetSecurityOrigin(), IsSecureContext());
-}
-
-void OriginTrialContext::AddTokens(const Vector<String>& tokens,
-                                   const SecurityOrigin* origin,
-                                   bool is_secure) {
   if (tokens.IsEmpty())
     return;
   bool found_valid = false;
   for (const String& token : tokens) {
     if (!token.IsEmpty()) {
       tokens_.push_back(token);
-      if (EnableTrialFromToken(origin, is_secure, token))
+      if (EnableTrialFromToken(GetSecurityOrigin(), IsSecureContext(), token))
         found_valid = true;
     }
   }
@@ -304,7 +294,11 @@ void OriginTrialContext::InitializePendingFeatures() {
   if (!enabled_features_.size() && !navigation_activated_features_.size())
     return;
   auto* window = DynamicTo<LocalDOMWindow>(context_.Get());
-  if (!window)
+  // Normally, LocalDOMWindow::document() doesn't need to be null-checked.
+  // However, this is a rare function that can get called between when the
+  // LocalDOMWindow is constructed and the Document is installed. We are not
+  // ready for script in that case, so bail out.
+  if (!window || !window->document())
     return;
   LocalFrame* frame = window->GetFrame();
   if (!frame)

third_party/blink/renderer/core/origin_trials/origin_trial_context.h

@@ -36,10 +36,9 @@ class ScriptState;
 class CORE_EXPORT OriginTrialContext final
     : public GarbageCollected<OriginTrialContext> {
  public:
-  OriginTrialContext();
-  explicit OriginTrialContext(std::unique_ptr<TrialTokenValidator> validator);
+  explicit OriginTrialContext(ExecutionContext*);
 
-  void BindExecutionContext(ExecutionContext*);
+  void SetTrialTokenValidatorForTesting(std::unique_ptr<TrialTokenValidator>);
 
   // Parses an Origin-Trial header as specified in
   // https://jpchase.github.io/OriginTrials/#header into individual tokens.
@@ -79,9 +78,6 @@ class CORE_EXPORT OriginTrialContext final
   void AddTokenFromExternalScript(const String& token,
                                   const SecurityOrigin* origin);
   void AddTokens(const Vector<String>& tokens);
-  void AddTokens(const Vector<String>& tokens,
-                 const SecurityOrigin* origin,
-                 bool is_secure);
 
   void ActivateNavigationFeaturesFromInitiator(
       const Vector<OriginTrialFeature>& features);

third_party/blink/renderer/core/origin_trials/origin_trial_context_test.cc

@@ -87,10 +87,12 @@ class OriginTrialContextTest : public testing::Test {
  protected:
   OriginTrialContextTest()
       : token_validator_(new MockTokenValidator),
-        execution_context_(MakeGarbageCollected<NullExecutionContext>(
-            MakeGarbageCollected<OriginTrialContext>(
-                std::unique_ptr<MockTokenValidator>(token_validator_)))),
-        histogram_tester_(new HistogramTester()) {}
+        execution_context_(MakeGarbageCollected<NullExecutionContext>()),
+        histogram_tester_(new HistogramTester()) {
+    execution_context_->GetOriginTrialContext()
+        ->SetTrialTokenValidatorForTesting(
+            std::unique_ptr<MockTokenValidator>(token_validator_));
+  }
 
   MockTokenValidator* TokenValidator() { return token_validator_; }
 

third_party/blink/renderer/core/testing/null_execution_context.cc

@@ -11,22 +11,18 @@
 #include "third_party/blink/renderer/core/execution_context/security_context_init.h"
 #include "third_party/blink/renderer/core/frame/csp/content_security_policy.h"
 #include "third_party/blink/renderer/core/frame/dom_timer.h"
-#include "third_party/blink/renderer/core/origin_trials/origin_trial_context.h"
 #include "third_party/blink/renderer/platform/scheduler/public/dummy_schedulers.h"
 #include "third_party/blink/renderer/platform/scheduler/public/frame_scheduler.h"
 #include "third_party/blink/renderer/platform/scheduler/public/thread.h"
 
 namespace blink {
 
-NullExecutionContext::NullExecutionContext(
-    OriginTrialContext* origin_trial_context)
+NullExecutionContext::NullExecutionContext()
     : ExecutionContext(
           v8::Isolate::GetCurrent(),
           MakeGarbageCollected<Agent>(v8::Isolate::GetCurrent(),
                                       base::UnguessableToken::Null())),
-      scheduler_(scheduler::CreateDummyFrameScheduler()) {
-  Initialize(SecurityContextInit(origin_trial_context));
-}
+      scheduler_(scheduler::CreateDummyFrameScheduler()) {}
 
 NullExecutionContext::~NullExecutionContext() {}
 

third_party/blink/renderer/core/testing/null_execution_context.h

@@ -21,7 +21,7 @@ class NullExecutionContext : public GarbageCollected<NullExecutionContext>,
   USING_GARBAGE_COLLECTED_MIXIN(NullExecutionContext);
 
  public:
-  NullExecutionContext(OriginTrialContext* origin_trial_context = nullptr);
+  NullExecutionContext();
   ~NullExecutionContext() override;
 
   void SetURL(const KURL& url) { url_ = url; }

third_party/blink/renderer/core/workers/worker_or_worklet_global_scope.cc

@@ -14,7 +14,6 @@
 #include "third_party/blink/renderer/bindings/core/v8/worker_or_worklet_script_controller.h"
 #include "third_party/blink/renderer/core/dom/events/event_queue.h"
 #include "third_party/blink/renderer/core/execution_context/agent.h"
-#include "third_party/blink/renderer/core/execution_context/security_context_init.h"
 #include "third_party/blink/renderer/core/frame/deprecation.h"
 #include "third_party/blink/renderer/core/inspector/console_message.h"
 #include "third_party/blink/renderer/core/loader/loader_factory_for_worker.h"
@@ -200,7 +199,6 @@ WorkerOrWorkletGlobalScope::WorkerOrWorkletGlobalScope(
       v8_cache_options_(v8_cache_options),
       reporting_proxy_(reporting_proxy) {
   GetSecurityContext().SetSecurityOrigin(std::move(origin));
-  Initialize(SecurityContextInit(MakeGarbageCollected<OriginTrialContext>()));
   if (worker_clients_)
     worker_clients_->ReattachThread();
 }