Revert "PlzDedicatedWorker: Send COEP report for worker initializatio…

…n failure"

This reverts commit 15a3b50.

Reason for revert: crashes in RenderFrameHostImpl::CreateDedicatedWorkerHostFactory https://crbug.com/1180505

Original change's description:
> PlzDedicatedWorker: Send COEP report for worker initialization failure
>
> This CL sends COEP violation reports during the worker initialization
> process.
>
> The corresponded spec is here:
> https://html.spec.whatwg.org/C/#check-a-global-object's-embedder-policy
> > 4. If ownerPolicy's report-only value is "require-corp" and policy's
> value is "unsafe-none", then queue a cross-origin embedder policy
> inheritance violation with response, "worker initialization", owner's
> policy's report only reporting endpoint, "reporting", and owner.
> > 6. Queue a cross-origin embedder policy inheritance violation with
> response, "worker initialization", owner's policy's reporting endpoint,
> "enforce", and owner.
>
> Also adds WPT to check if the owner iframe can observe reports via
> ReportingObserver API. Tests pass with PlzDedicatedWorker and fail with
> non-PlzDedicatedWorker.
>
> Bug: 1171094, 1060837
> Change-Id: Ida5eab351879a6793c412dccc8b0cff38c38e906
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2650015
> Commit-Queue: Asami Doi <asamidoi@chromium.org>
> Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
> Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
> Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#850107}

Bug: 1171094
Bug: 1060837
Bug: 1180505
Change-Id: I2803227fa27b308cb27a6999b2d0f72cad19cf1a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2717526
Reviewed-by: Srinivas Sista <srinivassista@chromium.org>
Commit-Queue: Srinivas Sista <srinivassista@chromium.org>
Cr-Commit-Position: refs/branch-heads/4427@{#5}
Cr-Branched-From: ce035c6-refs/heads/master@{#856944}

content/browser/net/cross_origin_embedder_policy_reporter.cc

@@ -68,15 +68,6 @@ void CrossOriginEmbedderPolicyReporter::QueueNavigationReport(
                  report_only);
 }
 
-void CrossOriginEmbedderPolicyReporter::QueueWorkerInitializationReport(
-    const GURL& blocked_url,
-    bool report_only) {
-  GURL url_to_pass = StripUsernameAndPassword(blocked_url);
-  QueueAndNotify({std::make_pair("type", "worker initialization"),
-                  std::make_pair("blockedURL", url_to_pass.spec())},
-                 report_only);
-}
-
 void CrossOriginEmbedderPolicyReporter::Clone(
     mojo::PendingReceiver<network::mojom::CrossOriginEmbedderPolicyReporter>
         receiver) {

content/browser/net/cross_origin_embedder_policy_reporter.h

@@ -59,16 +59,10 @@ class CONTENT_EXPORT CrossOriginEmbedderPolicyReporter final
   void BindObserver(
       mojo::PendingRemote<blink::mojom::ReportingObserver> observer);
 
-  // https://html.spec.whatwg.org/C/#check-a-navigation-response's-adherence-to-its-embedder-policy
-  // Queues a violation report for COEP mismatch for nested frame navigation.
+  // https://mikewest.github.io/corpp/#abstract-opdef-queue-coep-navigation-violation
+  // Queue a violation report for COEP mismatch for nested frame navigation.
   void QueueNavigationReport(const GURL& blocked_url, bool report_only);
 
-  // https://html.spec.whatwg.org/C/#check-a-global-object's-embedder-policy
-  // Queues a violation report for COEP mismatch during the worker
-  // initialization.
-  void QueueWorkerInitializationReport(const GURL& blocked_url,
-                                       bool report_only);
-
  private:
   void QueueAndNotify(std::initializer_list<
                           std::pair<base::StringPiece, base::StringPiece>> body,

content/browser/net/cross_origin_embedder_policy_reporter_unittest.cc

@@ -106,17 +106,7 @@ class CrossOriginEmbedderPolicyReporterTest : public testing::Test {
                                       base::StringPiece disposition) {
     base::Value dict(base::Value::Type::DICTIONARY);
     for (const auto& pair :
-         CreateBodyInternal("navigation", blocked_url, disposition)) {
-      dict.SetKey(std::move(pair.first), base::Value(std::move(pair.second)));
-    }
-    return dict;
-  }
-
-  base::Value CreateBodyForWorkerInitialization(base::StringPiece blocked_url,
-                                                base::StringPiece disposition) {
-    base::Value dict(base::Value::Type::DICTIONARY);
-    for (const auto& pair : CreateBodyInternal("worker initialization",
-                                               blocked_url, disposition)) {
+         CreateBodyForNavigationInternal(blocked_url, disposition)) {
       dict.SetKey(std::move(pair.first), base::Value(std::move(pair.second)));
     }
     return dict;
@@ -140,7 +130,7 @@ class CrossOriginEmbedderPolicyReporterTest : public testing::Test {
       base::StringPiece disposition) {
     auto body = blink::mojom::ReportBody::New();
     for (const auto& pair :
-         CreateBodyInternal("navigation", blocked_url, disposition)) {
+         CreateBodyForNavigationInternal(blocked_url, disposition)) {
       body->body.push_back(blink::mojom::ReportBodyElement::New(
           std::move(pair.first), std::move(pair.second)));
     }
@@ -159,11 +149,10 @@ class CrossOriginEmbedderPolicyReporterTest : public testing::Test {
             std::make_pair("disposition", disposition.as_string())};
   }
 
-  std::vector<std::pair<std::string, std::string>> CreateBodyInternal(
-      base::StringPiece type,
-      base::StringPiece blocked_url,
-      base::StringPiece disposition) const {
-    return {std::make_pair("type", type.as_string()),
+  std::vector<std::pair<std::string, std::string>>
+  CreateBodyForNavigationInternal(base::StringPiece blocked_url,
+                                  base::StringPiece disposition) const {
+    return {std::make_pair("type", "navigation"),
             std::make_pair("blockedURL", blocked_url.as_string()),
             std::make_pair("disposition", disposition.as_string())};
   }
@@ -337,6 +326,9 @@ TEST_F(CrossOriginEmbedderPolicyReporterTest, BasicNavigation) {
   const GURL kContextUrl("https://example.com/path");
   CrossOriginEmbedderPolicyReporter reporter(
       storage_partition(), kContextUrl, "e1", "e2", net::NetworkIsolationKey());
+  CrossOriginEmbedderPolicy child_coep;
+  child_coep.report_only_value =
+      network::mojom::CrossOriginEmbedderPolicyValue::kRequireCorp;
 
   reporter.QueueNavigationReport(GURL("https://www1.example.com/x#foo?bar=baz"),
                                  /*report_only=*/false);
@@ -415,33 +407,5 @@ TEST_F(CrossOriginEmbedderPolicyReporterTest, UserAndPassForNavigation) {
             CreateBodyForNavigation("https://www2.example.com/y", "reporting"));
 }
 
-TEST_F(CrossOriginEmbedderPolicyReporterTest, BasicWorkerInitialization) {
-  const GURL kContextUrl("https://example.com/path");
-  CrossOriginEmbedderPolicyReporter reporter(
-      storage_partition(), kContextUrl, "e1", "e2", net::NetworkIsolationKey());
-
-  reporter.QueueWorkerInitializationReport(
-      GURL("https://www1.example.com/worker.js"),
-      /*report_only=*/false);
-  reporter.QueueWorkerInitializationReport(
-      GURL("http://www2.example.com:41/worker.js"),
-      /*report_only=*/true);
-
-  ASSERT_EQ(2u, network_context().reports().size());
-  const Report& r1 = network_context().reports()[0];
-  const Report& r2 = network_context().reports()[1];
-
-  EXPECT_EQ(r1.type, "coep");
-  EXPECT_EQ(r1.group, "e1");
-  EXPECT_EQ(r1.url, kContextUrl);
-  EXPECT_EQ(r1.body, CreateBodyForWorkerInitialization(
-                         "https://www1.example.com/worker.js", "enforce"));
-  EXPECT_EQ(r2.type, "coep");
-  EXPECT_EQ(r2.group, "e2");
-  EXPECT_EQ(r2.url, kContextUrl);
-  EXPECT_EQ(r2.body, CreateBodyForWorkerInitialization(
-                         "http://www2.example.com:41/worker.js", "reporting"));
-}
-
 }  // namespace
 }  // namespace content

content/browser/renderer_host/render_frame_host_impl.cc

@@ -7970,7 +7970,7 @@ void RenderFrameHostImpl::CreateDedicatedWorkerHostFactory(
           /*creator_worker_token=*/base::nullopt,
           /*ancestor_render_frame_host_id=*/GetGlobalFrameRoutingId(),
           last_committed_origin_, isolation_info_,
-          cross_origin_embedder_policy_, coep_reporter_.get()),
+          cross_origin_embedder_policy_, std::move(coep_reporter)),
       std::move(receiver));
 }
 

content/browser/worker_host/dedicated_worker_host.cc

@@ -49,7 +49,8 @@ DedicatedWorkerHost::DedicatedWorkerHost(
     const url::Origin& creator_origin,
     const net::IsolationInfo& isolation_info,
     const network::CrossOriginEmbedderPolicy& cross_origin_embedder_policy,
-    CrossOriginEmbedderPolicyReporter* coep_reporter,
+    mojo::PendingRemote<network::mojom::CrossOriginEmbedderPolicyReporter>
+        coep_reporter,
     mojo::PendingReceiver<blink::mojom::DedicatedWorkerHost> host)
     : service_(service),
       token_(token),
@@ -64,7 +65,7 @@ DedicatedWorkerHost::DedicatedWorkerHost(
       isolation_info_(isolation_info),
       creator_cross_origin_embedder_policy_(cross_origin_embedder_policy),
       host_receiver_(this, std::move(host)),
-      coep_reporter_(coep_reporter) {
+      coep_reporter_(std::move(coep_reporter)) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(worker_process_host_);
   DCHECK(worker_process_host_->IsInitializedAndNotDead());
@@ -417,13 +418,8 @@ bool DedicatedWorkerHost::CheckCrossOriginEmbedderPolicy(
   // value is "unsafe-none", then queue a cross-origin embedder policy
   // inheritance violation with response, "worker initialization", owner's
   // policy's report only reporting endpoint, "reporting", and owner.
-  if (creator_cross_origin_embedder_policy.report_only_value ==
-          network::mojom::CrossOriginEmbedderPolicyValue::kRequireCorp &&
-      worker_cross_origin_embedder_policy.value ==
-          network::mojom::CrossOriginEmbedderPolicyValue::kNone) {
-    coep_reporter_->QueueWorkerInitializationReport(final_response_url_.value(),
-                                                    /*report_only=*/true);
-  }
+  // TODO(crbug.com/1060837): Queue a report if the report-only value is not
+  // valid.
 
   // > 5. If ownerPolicy's value is "unsafe-none" or policy's value is
   // "require-corp", then return true.
@@ -437,8 +433,7 @@ bool DedicatedWorkerHost::CheckCrossOriginEmbedderPolicy(
   // > 6. Queue a cross-origin embedder policy inheritance violation with
   // response, "worker initialization", owner's policy's reporting endpoint,
   // "enforce", and owner.
-  coep_reporter_->QueueWorkerInitializationReport(final_response_url_.value(),
-                                                  /*report_only=*/false);
+  // TODO(crbug.com/1060837): Queue this report.
 
   // > 7. Return false.
   return false;
@@ -510,15 +505,19 @@ void DedicatedWorkerHost::BindCacheStorage(
 void DedicatedWorkerHost::CreateNestedDedicatedWorker(
     mojo::PendingReceiver<blink::mojom::DedicatedWorkerHostFactory> receiver) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  mojo::PendingRemote<network::mojom::CrossOriginEmbedderPolicyReporter>
+      coep_reporter;
+  coep_reporter_->Clone(coep_reporter.InitWithNewPipeAndPassReceiver());
   // Set this worker as the creator of the new worker and inherit the ancestor
   // render frame.
+
   mojo::MakeSelfOwnedReceiver(
       std::make_unique<DedicatedWorkerHostFactoryImpl>(
           worker_process_host_->GetID(),
           /*creator_render_frame_host_id_=*/base::nullopt,
           /*creator_worker_token=*/token_, ancestor_render_frame_host_id_,
           worker_origin_, isolation_info_, cross_origin_embedder_policy(),
-          coep_reporter_),
+          std::move(coep_reporter)),
       std::move(receiver));
 }
 

content/browser/worker_host/dedicated_worker_host.h

@@ -11,7 +11,6 @@
 #include "base/scoped_observation.h"
 #include "build/build_config.h"
 #include "content/browser/browser_interface_broker_impl.h"
-#include "content/browser/net/cross_origin_embedder_policy_reporter.h"
 #include "content/public/browser/global_routing_id.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_process_host_observer.h"
@@ -61,7 +60,8 @@ class DedicatedWorkerHost final : public blink::mojom::DedicatedWorkerHost,
       const url::Origin& creator_origin,
       const net::IsolationInfo& isolation_info,
       const network::CrossOriginEmbedderPolicy& cross_origin_embedder_policy,
-      CrossOriginEmbedderPolicyReporter* coep_reporter,
+      mojo::PendingRemote<network::mojom::CrossOriginEmbedderPolicyReporter>
+          coep_reporter,
       mojo::PendingReceiver<blink::mojom::DedicatedWorkerHost> host);
   ~DedicatedWorkerHost() final;
 
@@ -255,11 +255,11 @@ class DedicatedWorkerHost final : public blink::mojom::DedicatedWorkerHost,
   // The endpoint of this mojo interface is the RenderFrameHostImpl's COEP
   // reporter. The COEP endpoint is correct, but the context_url is the
   // Document's URL.
-  // TODO(crbug.com/1060837): After landing PlzDedicatedWorker, make the
+  // TODO(arthursonzogni): After landing PlzDedicatedWorker, make the
   // DedicatedWorkerHost to have its own COEP reporter using the right
   // context_url.
-  CrossOriginEmbedderPolicyReporter* const coep_reporter_;  // Never null.
-
+  mojo::Remote<network::mojom::CrossOriginEmbedderPolicyReporter>
+      coep_reporter_;  // Never null.
   // Will be set once the worker script started loading.
   base::Optional<GURL> final_response_url_;
 

content/browser/worker_host/dedicated_worker_host_factory_impl.cc

@@ -43,15 +43,16 @@ DedicatedWorkerHostFactoryImpl::DedicatedWorkerHostFactoryImpl(
     const url::Origin& creator_origin,
     const net::IsolationInfo& isolation_info,
     const network::CrossOriginEmbedderPolicy& cross_origin_embedder_policy,
-    CrossOriginEmbedderPolicyReporter* coep_reporter)
+    mojo::PendingRemote<network::mojom::CrossOriginEmbedderPolicyReporter>
+        coep_reporter)
     : worker_process_id_(worker_process_id),
       creator_render_frame_host_id_(creator_render_frame_host_id),
       creator_worker_token_(creator_worker_token),
       ancestor_render_frame_host_id_(ancestor_render_frame_host_id),
       creator_origin_(creator_origin),
       isolation_info_(isolation_info),
       cross_origin_embedder_policy_(cross_origin_embedder_policy),
-      coep_reporter_(coep_reporter) {
+      coep_reporter_(std::move(coep_reporter)) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK((creator_render_frame_host_id_ && !creator_worker_token_) ||
          (!creator_render_frame_host_id_ && creator_worker_token_));
@@ -90,10 +91,14 @@ void DedicatedWorkerHostFactoryImpl::CreateWorkerHost(
     return;
   }
 
+  mojo::PendingRemote<network::mojom::CrossOriginEmbedderPolicyReporter>
+      coep_reporter;
+  coep_reporter_->Clone(coep_reporter.InitWithNewPipeAndPassReceiver());
+
   auto* host = new DedicatedWorkerHost(
       service, token, worker_process_host, creator_render_frame_host_id_,
       creator_worker_token_, ancestor_render_frame_host_id_, creator_origin_,
-      isolation_info_, cross_origin_embedder_policy_, coep_reporter_,
+      isolation_info_, cross_origin_embedder_policy_, std::move(coep_reporter),
       std::move(host_receiver));
   host->BindBrowserInterfaceBrokerReceiver(std::move(broker_receiver));
 }
@@ -128,11 +133,16 @@ void DedicatedWorkerHostFactoryImpl::CreateWorkerHostAndStartScriptLoad(
 
   // TODO(https://crbug.com/1058759): Compare |creator_origin_| to
   // |script_url|, and report as bad message if that fails.
+
+  mojo::PendingRemote<network::mojom::CrossOriginEmbedderPolicyReporter>
+      coep_reporter;
+  coep_reporter_->Clone(coep_reporter.InitWithNewPipeAndPassReceiver());
+
   mojo::PendingRemote<blink::mojom::DedicatedWorkerHost> pending_remote_host;
   auto* host = new DedicatedWorkerHost(
       service, token, worker_process_host, creator_render_frame_host_id_,
       creator_worker_token_, ancestor_render_frame_host_id_, creator_origin_,
-      isolation_info_, cross_origin_embedder_policy_, coep_reporter_,
+      isolation_info_, cross_origin_embedder_policy_, std::move(coep_reporter),
       pending_remote_host.InitWithNewPipeAndPassReceiver());
   mojo::PendingRemote<blink::mojom::BrowserInterfaceBroker> broker;
   host->BindBrowserInterfaceBrokerReceiver(

content/browser/worker_host/dedicated_worker_host_factory_impl.h

@@ -6,7 +6,6 @@
 #define CONTENT_BROWSER_WORKER_HOST_DEDICATED_WORKER_HOST_FACTORY_IMPL_H_
 
 #include "base/optional.h"
-#include "content/browser/net/cross_origin_embedder_policy_reporter.h"
 #include "content/common/content_export.h"
 #include "content/public/browser/global_routing_id.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
@@ -32,7 +31,8 @@ class CONTENT_EXPORT DedicatedWorkerHostFactoryImpl
       const url::Origin& creator_origin,
       const net::IsolationInfo& isolation_info,
       const network::CrossOriginEmbedderPolicy& cross_origin_embedder_policy,
-      CrossOriginEmbedderPolicyReporter* coep_reporter);
+      mojo::PendingRemote<network::mojom::CrossOriginEmbedderPolicyReporter>
+          coep_reporter);
   ~DedicatedWorkerHostFactoryImpl() override;
 
   // blink::mojom::DedicatedWorkerHostFactory:
@@ -67,7 +67,8 @@ class CONTENT_EXPORT DedicatedWorkerHostFactoryImpl
   const url::Origin creator_origin_;
   const net::IsolationInfo isolation_info_;
   const network::CrossOriginEmbedderPolicy cross_origin_embedder_policy_;
-  CrossOriginEmbedderPolicyReporter* const coep_reporter_;
+  mojo::Remote<network::mojom::CrossOriginEmbedderPolicyReporter>
+      coep_reporter_;
 
   DISALLOW_COPY_AND_ASSIGN(DedicatedWorkerHostFactoryImpl);
 };

content/browser/worker_host/dedicated_worker_service_impl_unittest.cc

@@ -35,17 +35,18 @@ class MockDedicatedWorker
   MockDedicatedWorker(int worker_process_id,
                       GlobalFrameRoutingId render_frame_host_id) {
     // The COEP reporter is replaced by a dummy connection. Reports are ignored.
-    CrossOriginEmbedderPolicyReporter coep_reporter(
-        RenderFrameHostImpl::FromID(render_frame_host_id)
-            ->GetStoragePartition(),
-        GURL(), base::nullopt, base::nullopt, net::NetworkIsolationKey());
+    mojo::PendingRemote<network::mojom::CrossOriginEmbedderPolicyReporter>
+        coep_reporter_remote;
+    auto dummy_coep_reporter =
+        coep_reporter_remote.InitWithNewPipeAndPassReceiver();
 
     mojo::MakeSelfOwnedReceiver(
         std::make_unique<DedicatedWorkerHostFactoryImpl>(
             worker_process_id, render_frame_host_id,
             /*creator_worker_token=*/base::nullopt, render_frame_host_id,
             url::Origin(), net::IsolationInfo::CreateTransient(),
-            network::CrossOriginEmbedderPolicy(), &coep_reporter),
+            network::CrossOriginEmbedderPolicy(),
+            std::move(coep_reporter_remote)),
         factory_.BindNewPipeAndPassReceiver());
 
     if (base::FeatureList::IsEnabled(blink::features::kPlzDedicatedWorker)) {

third_party/blink/web_tests/TestExpectations

@@ -5661,10 +5661,6 @@ crbug.com/1168785 [ Mac ] virtual/threaded-prefer-compositing/fast/scrolling/aut
 # Sheriff 2021-01-29
 crbug.com/1172437 svg/hixie/perf/004.xml [ Timeout Pass ]
 
-# COEP reporting for PlzDedicatedWorker
-crbug.com/1060837 external/wpt/html/cross-origin-embedder-policy/reporting-to-owner.https.html [ Timeout ]
-crbug.com/1060837 virtual/plz-dedicated-worker/external/wpt/html/cross-origin-embedder-policy/reporting-to-owner.https.html [ Pass ]
-
 # flaky test
 crbug.com/1173956 http/tests/xsl/xslt-transform-with-javascript-disabled.html [ Failure Pass ]