Move initialization of report_only_feature_policy_ to SecurityContextInit

This CL moves initialization of report_only_feature_policy_ from
ExecutionContext::ApplyReportOnlyFeaturePolicyFromHeader to
SecurityContextInit where enforced feature_policy_ is initialized.

Add |pending_console_message_| field to Document in order to correctly
handle console message generated in Document constructor where
frame_->GetDocument returns nullptr which causes FrameConsole to return
early.

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

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

@@ -6885,40 +6885,6 @@ void Document::ApplyPendingFramePolicyHeaders() {
   pending_dp_headers_.clear();
 }
 
-void Document::ApplyReportOnlyFeaturePolicyFromHeader(
-    const String& feature_policy_report_only_header) {
-  if (feature_policy_report_only_header.IsEmpty())
-    return;
-  // TODO(iclelland): Remove this message when reporting is no longer part of an
-  // origin trial.
-  // Note that we do not return here. Instead, the header is parsed and the
-  // report-only policy is stored, in case a valid Origin Trial token is added
-  // later. In that case, any subsequent violations will be correctly reported.
-  if (!RuntimeEnabledFeatures::FeaturePolicyReportingEnabled(this)) {
-    AddConsoleMessage(MakeGarbageCollected<ConsoleMessage>(
-        mojom::ConsoleMessageSource::kSecurity,
-        mojom::ConsoleMessageLevel::kWarning,
-        "Feature-Policy-Report-Only header will have no effect unless Feature "
-        "Policy reporting is enabled with an Origin Trial. Sign up at "
-        "https://developers.chrome.com/origintrials/"));
-  }
-
-  UseCounter::Count(*this, WebFeature::kFeaturePolicyReportOnlyHeader);
-  Vector<String> messages;
-  const ParsedFeaturePolicy& report_only_policy =
-      FeaturePolicyParser::ParseHeader(feature_policy_report_only_header,
-                                       GetSecurityOrigin(), &messages, this);
-  for (auto& message : messages) {
-    AddConsoleMessage(MakeGarbageCollected<ConsoleMessage>(
-        mojom::ConsoleMessageSource::kSecurity,
-        mojom::ConsoleMessageLevel::kError,
-        "Error with Feature-Policy-Report-Only header: " + message));
-  }
-
-  GetSecurityContext().AddReportOnlyFeaturePolicy(
-      report_only_policy, GetOwnerContainerPolicy(), GetParentFeaturePolicy());
-}
-
 bool Document::AllowedToUseDynamicMarkUpInsertion(
     const char* api_name,
     ExceptionState& exception_state) {

third_party/blink/renderer/core/dom/document.h

@@ -1529,11 +1529,6 @@ class CORE_EXPORT Document : public ContainerNode,
   // Apply pending feature policy headers and document policy headers.
   void ApplyPendingFramePolicyHeaders();
 
-  // Set the report-only feature policy on this document in response to an HTTP
-  // Feature-Policy-Report-Only header.
-  void ApplyReportOnlyFeaturePolicyFromHeader(
-      const String& feature_policy_report_only_header);
-
   const AtomicString& bgColor() const;
   void setBgColor(const AtomicString&);
   const AtomicString& fgColor() const;

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

@@ -349,6 +349,13 @@ DocumentInit& DocumentInit::WithFeaturePolicyHeader(const String& header) {
   return *this;
 }
 
+DocumentInit& DocumentInit::WithReportOnlyFeaturePolicyHeader(
+    const String& header) {
+  DCHECK(report_only_feature_policy_header_.IsEmpty());
+  report_only_feature_policy_header_ = header;
+  return *this;
+}
+
 DocumentInit& DocumentInit::WithOriginTrialsHeader(const String& header) {
   DCHECK(origin_trials_header_.IsEmpty());
   origin_trials_header_ = header;

third_party/blink/renderer/core/dom/document_init.h

@@ -154,6 +154,11 @@ class CORE_EXPORT DocumentInit final {
   DocumentInit& WithFeaturePolicyHeader(const String& header);
   const String& FeaturePolicyHeader() const { return feature_policy_header_; }
 
+  DocumentInit& WithReportOnlyFeaturePolicyHeader(const String& header);
+  const String& ReportOnlyFeaturePolicyHeader() const {
+    return report_only_feature_policy_header_;
+  }
+
   DocumentInit& WithOriginTrialsHeader(const String& header);
   const String& OriginTrialsHeader() const { return origin_trials_header_; }
 
@@ -235,6 +240,7 @@ class CORE_EXPORT DocumentInit final {
 
   // The feature policy set via response header.
   String feature_policy_header_;
+  String report_only_feature_policy_header_;
 
   // The origin trial set via response header.
   String origin_trials_header_;

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

@@ -58,6 +58,7 @@ SecurityContext::SecurityContext(const SecurityContextInit& init,
     : sandbox_flags_(init.GetSandboxFlags()),
       security_origin_(init.GetSecurityOrigin()),
       feature_policy_(init.CreateFeaturePolicy()),
+      report_only_feature_policy_(init.CreateReportOnlyFeaturePolicy()),
       document_policy_(init.CreateDocumentPolicy()),
       content_security_policy_(init.GetCSP()),
       address_space_(network::mojom::IPAddressSpace::kUnknown),
@@ -127,16 +128,6 @@ void SecurityContext::SetFeaturePolicy(
   feature_policy_ = std::move(feature_policy);
 }
 
-// Uses the parent enforcing policy as the basis for the report-only policy.
-void SecurityContext::AddReportOnlyFeaturePolicy(
-    const ParsedFeaturePolicy& parsed_report_only_header,
-    const ParsedFeaturePolicy& container_policy,
-    const FeaturePolicy* parent_feature_policy) {
-  report_only_feature_policy_ = FeaturePolicy::CreateFromParentPolicy(
-      parent_feature_policy, container_policy, security_origin_->ToUrlOrigin());
-  report_only_feature_policy_->SetHeaderPolicy(parsed_report_only_header);
-}
-
 void SecurityContext::SetDocumentPolicyForTesting(
     std::unique_ptr<DocumentPolicy> document_policy) {
   document_policy_ = std::move(document_policy);

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

@@ -97,6 +97,14 @@ void SecurityContextInit::ApplyPendingDataToDocument(Document& document) const {
         mojom::ConsoleMessageLevel::kError,
         "Error with Feature-Policy header: " + message));
   }
+  for (const auto& message : report_only_feature_policy_parse_messages_) {
+    document.AddConsoleMessage(MakeGarbageCollected<ConsoleMessage>(
+        mojom::blink::ConsoleMessageSource::kSecurity,
+        mojom::blink::ConsoleMessageLevel::kError,
+        "Error with Feature-Policy-Report-Only header: " + message));
+  }
+  if (!report_only_feature_policy_header_.empty())
+    UseCounter::Count(document, WebFeature::kFeaturePolicyReportOnlyHeader);
 }
 
 void SecurityContextInit::InitializeContentSecurityPolicy(
@@ -293,6 +301,10 @@ void SecurityContextInit::InitializeFeaturePolicy(
       initializer.FeaturePolicyHeader(), security_origin_,
       &feature_policy_parse_messages_, this);
 
+  report_only_feature_policy_header_ = FeaturePolicyParser::ParseHeader(
+      initializer.ReportOnlyFeaturePolicyHeader(), security_origin_,
+      &report_only_feature_policy_parse_messages_, this);
+
   if (sandbox_flags_ != mojom::blink::WebSandboxFlags::kNone &&
       RuntimeEnabledFeatures::FeaturePolicyForSandboxEnabled()) {
     // The sandbox flags might have come from CSP header or the browser; in
@@ -325,8 +337,31 @@ void SecurityContextInit::InitializeFeaturePolicy(
     parent_frame_ = frame->Tree().Parent();
 }
 
+std::unique_ptr<FeaturePolicy>
+SecurityContextInit::CreateReportOnlyFeaturePolicy() const {
+  // For non-Document initialization, returns nullptr directly.
+  if (!initialized_feature_policy_state_)
+    return nullptr;
+  // Report-only feature policy only takes effect when it is stricter than
+  // enforced feature policy, i.e. when enforced feature policy allows a feature
+  // while report-only feature policy do not. In such scenario, a report-only
+  // policy violation report will be generated, but the feature is still allowed
+  // to be used. Since child frames cannot loosen enforced feature policy, there
+  // is no need to inherit parent policy and container policy for report-only
+  // feature policy. For inherited policies, the behavior is dominated by
+  // enforced feature policy.
+  DCHECK(security_origin_);
+  std::unique_ptr<FeaturePolicy> report_only_policy =
+      FeaturePolicy::CreateFromParentPolicy(nullptr /* parent_policy */,
+                                            {} /* container_policy */,
+                                            security_origin_->ToUrlOrigin());
+  report_only_policy->SetHeaderPolicy(report_only_feature_policy_header_);
+  return report_only_policy;
+}
+
 std::unique_ptr<FeaturePolicy> SecurityContextInit::CreateFeaturePolicy()
     const {
+  // For non-Document initialization, returns nullptr directly.
   if (!initialized_feature_policy_state_)
     return nullptr;
 

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

@@ -45,6 +45,7 @@ class CORE_EXPORT SecurityContextInit : public FeaturePolicyParserDelegate {
   ContentSecurityPolicy* GetCSP() const { return csp_; }
 
   std::unique_ptr<FeaturePolicy> CreateFeaturePolicy() const;
+  std::unique_ptr<FeaturePolicy> CreateReportOnlyFeaturePolicy() const;
 
   std::unique_ptr<DocumentPolicy> CreateDocumentPolicy() const;
 
@@ -87,7 +88,9 @@ class CORE_EXPORT SecurityContextInit : public FeaturePolicyParserDelegate {
   DocumentPolicy::FeatureState document_policy_;
   bool initialized_feature_policy_state_ = false;
   Vector<String> feature_policy_parse_messages_;
+  Vector<String> report_only_feature_policy_parse_messages_;
   ParsedFeaturePolicy feature_policy_header_;
+  ParsedFeaturePolicy report_only_feature_policy_header_;
   LocalFrame* frame_for_opener_feature_state_ = nullptr;
   Frame* parent_frame_ = nullptr;
   ParsedFeaturePolicy container_policy_;

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

@@ -1498,6 +1498,9 @@ void DocumentLoader::InstallNewDocument(
           .WithFramePolicy(frame_policy_)
           .WithNewRegistrationContext()
           .WithFeaturePolicyHeader(feature_policy.ToString())
+          // TODO(iclelland): Add Feature-Policy-Report-Only to Origin Policy.
+          .WithReportOnlyFeaturePolicyHeader(
+              response_.HttpHeaderField(http_names::kFeaturePolicyReportOnly))
           .WithDocumentPolicy(document_policy_)
           .WithOriginTrialsHeader(
               response_.HttpHeaderField(http_names::kOriginTrial))
@@ -1712,11 +1715,6 @@ void DocumentLoader::CreateParserPostCommit() {
   // are sent in didCommitNavigation().
   document->ApplyPendingFramePolicyHeaders();
 
-  WTF::String report_only_feature_policy(
-      response_.HttpHeaderField(http_names::kFeaturePolicyReportOnly));
-  // TODO(iclelland): Add Feature-Policy-Report-Only to Origin Policy.
-  document->ApplyReportOnlyFeaturePolicyFromHeader(report_only_feature_policy);
-
   GetFrameLoader().DispatchDidClearDocumentOfWindowObject();
 
   parser_->SetDocumentWasLoadedAsPartOfNavigation();

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

@@ -22,7 +22,7 @@ NullExecutionContext::NullExecutionContext(
     : ExecutionContext(
           v8::Isolate::GetCurrent(),
           SecurityContextInit(
-              nullptr,
+              nullptr /* origin */,
               origin_trial_context,
               MakeGarbageCollected<Agent>(v8::Isolate::GetCurrent(),
                                           base::UnguessableToken::Null()))),

third_party/blink/web_tests/virtual/stable/http/tests/origin_trials/feature-policy-reporting/feature-policy-reporting-no-trial-expected.txt

-CONSOLE WARNING: Feature-Policy-Report-Only header will have no effect unless Feature Policy reporting is enabled with an Origin Trial. Sign up at https://developers.chrome.com/origintrials/
 CONSOLE WARNING: line 21: Synchronous XMLHttpRequest on the main thread is deprecated because of its detrimental effects to the end user's experience. For more help, check https://xhr.spec.whatwg.org/.
 CONSOLE ERROR: line 8: Unable to preventDefault inside passive event listener due to target being treated as passive. See https://www.chromestatus.com/features/5093566007214080
 This is a testharness.js-based test.