Commit 1ca5136d authored by Katie Dektar's avatar Katie Dektar Committed by Commit Bot

Revert "[COOP] access reporting [5/N] Plumb report type."

This reverts commit 6e52cf21.

Reason for revert: Speculative revert: A related test has begun
failing since this landed.

Failing test:
external/wpt/html/cross-origin-opener-policy/reporting/access-reporting/property.https.htm

First failing build (with this change in blamelist): 
https://ci.chromium.org/p/chromium/builders/ci/WebKit%20Linux%20Leak/16841

Original change's description:
> [COOP] access reporting [5/N] Plumb report type.
> 
> All of this is put behind a flag disabled by default.
> This is mostly based on the initial prototype:
> https://chromium-review.googlesource.com/c/chromium/src/+/2223934/24
> 
> Define, plumb and use the "report-type" for COOP access reporting.
> 
> COOP access reporting:
> [1/N] https://chromium-review.googlesource.com/c/chromium/src/+/2264294
> [2/N] https://chromium-review.googlesource.com/c/chromium/src/+/2270185
> [3/N] https://chromium-review.googlesource.com/c/chromium/src/+/2270472
> [4/N] https://chromium-review.googlesource.com/c/chromium/src/+/2273120
> [5/N] this patch.
> 
> Bug: chromium:1090273
> Change-Id: I5e51b3da6a20c85c073ea1fd84ea74557760a42f
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2309433
> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
> Reviewed-by: Camille Lamy <clamy@chromium.org>
> Reviewed-by: Pâris Meuleman <pmeuleman@chromium.org>
> Reviewed-by: Mike West <mkwst@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#790420}

TBR=clamy@chromium.org,mkwst@chromium.org,arthursonzogni@chromium.org,pmeuleman@chromium.org

Change-Id: I88a01f1dc601205b2344c4251747cf75ef5a84d0
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:1090273
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2310769Reviewed-by: default avatarKatie Dektar <katie@chromium.org>
Commit-Queue: Katie Dektar <katie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#790466}
parent cfd36094
......@@ -34,7 +34,8 @@ constexpr char kViolationTypeFromDocument[] = "navigation-from-document";
constexpr char kViolationTypeToDocument[] = "navigation-to-document";
constexpr char kViolationType[] = "violation-type";
std::string ToString(network::mojom::CrossOriginOpenerPolicyValue coop_value) {
std::string CoopValueToString(
network::mojom::CrossOriginOpenerPolicyValue coop_value) {
switch (coop_value) {
case network::mojom::CrossOriginOpenerPolicyValue::kUnsafeNone:
return kUnsafeNone;
......@@ -47,15 +48,6 @@ std::string ToString(network::mojom::CrossOriginOpenerPolicyValue coop_value) {
}
}
const char* ToString(network::mojom::CoopAccessReportType report_type) {
switch (report_type) {
case network::mojom::CoopAccessReportType::kReportAccessTo:
return "access-to-coop-page";
case network::mojom::CoopAccessReportType::kReportAccessFrom:
return "access-from-coop-page";
}
}
RenderFrameHostImpl* GetSourceRfhForCoopReporting(
RenderFrameHostImpl* current_rfh) {
CHECK(current_rfh);
......@@ -167,16 +159,15 @@ void CrossOriginOpenerPolicyReporter::QueueOpenerBreakageReport(
body.SetString(kViolationType, is_reported_from_document
? kViolationTypeFromDocument
: kViolationTypeToDocument);
body.SetString(
kEffectivePolicy,
ToString(is_report_only ? coop_.report_only_value : coop_.value));
body.SetString(kEffectivePolicy,
CoopValueToString(is_report_only ? coop_.report_only_value
: coop_.value));
storage_partition_->GetNetworkContext()->QueueReport(
"coop", *endpoint, context_url_, /*user_agent=*/base::nullopt,
std::move(body));
}
void CrossOriginOpenerPolicyReporter::QueueAccessReport(
network::mojom::CoopAccessReportType report_type,
const std::string& property) {
// Cross-Origin-Opener-Policy-Report-Only is not required to provide
// endpoints.
......@@ -189,15 +180,15 @@ void CrossOriginOpenerPolicyReporter::QueueAccessReport(
network::features::kCrossOriginOpenerPolicyAccessReporting));
base::DictionaryValue body;
body.SetStringPath(kViolationType, ToString(report_type));
body.SetStringPath(kDisposition, kDispositionReporting);
body.SetStringPath(kEffectivePolicy,
ToString(coop_.report_only_value));
CoopValueToString(coop_.report_only_value));
body.SetStringPath(kProperty, property);
// TODO(arthursonzogni): Fill "blocked-window-url".
// TODO(arthursonzogni): Fill "source-file".
// TODO(arthursonzogni): Fill "line-no".
// TODO(arthursonzogni): Fill "col-no".
// TODO(arthursonzogni): Fill "violation-type".
storage_partition_->GetNetworkContext()->QueueReport(
"coop", endpoint, context_url_, base::nullopt, std::move(body));
}
......@@ -330,13 +321,8 @@ void CrossOriginOpenerPolicyReporter::MonitorAccesses(
remote_reporter;
Clone(remote_reporter.InitWithNewPipeAndPassReceiver());
network::mojom::CoopAccessReportType report_type =
accessing_node->current_frame_host()->coop_reporter() == this
? network::mojom::CoopAccessReportType::kReportAccessFrom
: network::mojom::CoopAccessReportType::kReportAccessTo;
accessing_rfh->GetAssociatedLocalMainFrame()->InstallCoopAccessMonitor(
report_type, accessed_window_token, std::move(remote_reporter));
accessed_window_token, std::move(remote_reporter));
}
} // namespace content
......@@ -45,8 +45,7 @@ class CONTENT_EXPORT CrossOriginOpenerPolicyReporter final
void QueueOpenerBreakageReport(const GURL& other_url,
bool is_reported_from_document,
bool is_report_only) final;
void QueueAccessReport(network::mojom::CoopAccessReportType report_type,
const std::string& property) final;
void QueueAccessReport(const std::string& property) final;
// Returns the "previous" URL that is safe to expose.
// Reference, "Next document URL for reporting" section:
......
......@@ -7,11 +7,6 @@ module network.mojom;
import "url/mojom/url.mojom";
import "mojo/public/mojom/base/unguessable_token.mojom";
enum CoopAccessReportType {
kReportAccessFrom, // The reports are sent to the 'accessing window'.
kReportAccessTo, // The reports are sent to the 'accessed window'.
};
// Reports potential COOP violations. Implemented in the browser process.
// TODO(ahemery, pmeuleman): Add extra coop breakage cases as listed in
// https://docs.google.com/document/d/1zWqwI8PFrezwQpBSejIMUfdtsIYl9-h8epasdrDXVIM/edit
......@@ -32,7 +27,7 @@ interface CrossOriginOpenerPolicyReporter {
// When two browsing contexts from different virtual browsing context groups
// tries to access each other, a report it sent.
// - |property| is the name of the access property (postMessage, open, ...).
QueueAccessReport(CoopAccessReportType report_type, string property);
QueueAccessReport(string property);
// Connects a new pipe to this instance.
Clone(pending_receiver<CrossOriginOpenerPolicyReporter> receiver);
......
......@@ -881,7 +881,6 @@ interface LocalMainFrame {
// Check accesses made from this window to |accessed_window|. If this happens,
// a report will be sent to |reporter|.
InstallCoopAccessMonitor(
network.mojom.CoopAccessReportType report_type,
mojo_base.mojom.UnguessableToken accessed_window,
pending_remote<network.mojom.CrossOriginOpenerPolicyReporter> reporter);
};
......
......@@ -426,14 +426,12 @@ void DOMWindow::PostMessageForTesting(
}
void DOMWindow::InstallCoopAccessMonitor(
network::mojom::blink::CoopAccessReportType report_type,
LocalFrame* accessing_frame,
mojo::PendingRemote<network::mojom::blink::CrossOriginOpenerPolicyReporter>
pending_reporter) {
CoopAccessMonitor monitor;
DCHECK(accessing_frame->IsMainFrame());
monitor.report_type = report_type;
monitor.accessing_main_frame = accessing_frame->GetFrameToken();
// TODO(arthursonzogni): Clean coop_access_monitor_ when a reporter is gone.
......@@ -467,8 +465,10 @@ void DOMWindow::ReportCoopAccess(v8::Isolate* isolate,
// TODO(arthursonzogni): Capture and send the SourceLocation.
// TODO(arthursonzogni): Send the blocked-window-url.
// TODO(arthursonzogni): Send whether this was access-to-coop or
// access-from-coop.
it->reporter->QueueAccessReport(it->report_type, property_name);
it->reporter->QueueAccessReport(property_name);
// TODO(arthursonzogni): In the access-from-coop case, dispatch a
// reportingObserver event.
......
......@@ -137,7 +137,6 @@ class CORE_EXPORT DOMWindow : public EventTargetWithInlineData {
// Check accesses from |accessing_frame| and every same-origin iframe toward
// this window. A report is sent to |reporter| when this happens.
void InstallCoopAccessMonitor(
network::mojom::blink::CoopAccessReportType report_type,
LocalFrame* accessing_frame,
mojo::PendingRemote<
network::mojom::blink::CrossOriginOpenerPolicyReporter> reporter);
......@@ -184,7 +183,6 @@ class CORE_EXPORT DOMWindow : public EventTargetWithInlineData {
// Check accesses made toward this window from |accessing_main_frame|. If this
// happens a report will sent to |reporter|.
struct CoopAccessMonitor {
network::mojom::blink::CoopAccessReportType report_type;
base::UnguessableToken accessing_main_frame;
mojo::Remote<network::mojom::blink::CrossOriginOpenerPolicyReporter>
reporter;
......
......@@ -2380,7 +2380,6 @@ void LocalFrame::GetStringForRange(const gfx::Range& range,
#endif
void LocalFrame::InstallCoopAccessMonitor(
network::mojom::blink::CoopAccessReportType report_type,
const base::UnguessableToken& accessed_window,
mojo::PendingRemote<network::mojom::blink::CrossOriginOpenerPolicyReporter>
reporter) {
......@@ -2389,7 +2388,7 @@ void LocalFrame::InstallCoopAccessMonitor(
if (!accessed_frame)
return;
accessed_frame->DomWindow()->InstallCoopAccessMonitor(report_type, this,
accessed_frame->DomWindow()->InstallCoopAccessMonitor(this,
std::move(reporter));
}
......
......@@ -620,7 +620,6 @@ class CORE_EXPORT LocalFrame final
GetStringForRangeCallback callback) final;
#endif
void InstallCoopAccessMonitor(
network::mojom::blink::CoopAccessReportType report_type,
const base::UnguessableToken& accessed_window,
mojo::PendingRemote<
network::mojom::blink::CrossOriginOpenerPolicyReporter> reporter)
......
This is a testharness.js-based test.
FAIL Opener accesses openee (COOP-RO+COEP). Report to openee assert_not_equals: Report not received got disallowed value "timeout"
Harness: the test ran to completion.
This is a testharness.js-based test.
FAIL Openee (COOP-RO+COEP) accesses opener. Report to openee promise_test: Unhandled rejection with value: object "TypeError: Cannot read property 'includes' of undefined"
FAIL Openee (COOP-RO+COEP) accesses opener. Report to openee assert_not_equals: Report not received got disallowed value "timeout"
Harness: the test ran to completion.
This is a testharness.js-based test.
PASS cross-origin > w => w.blur()
PASS cross-origin > w => w.close()
PASS cross-origin > w => w.closed
PASS cross-origin > w => w.focus()
PASS cross-origin > w => w.frames
FAIL cross-origin > w => w.blur() assert_not_equals: Report not received got disallowed value "timeout"
FAIL cross-origin > w => w.close() assert_not_equals: Report not received got disallowed value "timeout"
FAIL cross-origin > w => w.closed assert_not_equals: Report not received got disallowed value "timeout"
FAIL cross-origin > w => w.focus() assert_not_equals: Report not received got disallowed value "timeout"
FAIL cross-origin > w => w.frames assert_not_equals: Report not received got disallowed value "timeout"
FAIL cross-origin > w => w[0] assert_not_equals: Report not received got disallowed value "timeout"
FAIL cross-origin > w => w[0] = "" assert_not_equals: Report not received got disallowed value "timeout"
PASS cross-origin > w => w.length
FAIL cross-origin > w => w.length assert_not_equals: Report not received got disallowed value "timeout"
FAIL cross-origin > w => w.location assert_not_equals: Report not received got disallowed value "timeout"
FAIL cross-origin > w => w.location = "#" assert_not_equals: Report not received got disallowed value "timeout"
FAIL cross-origin > w => w["test"] assert_not_equals: Report not received got disallowed value "timeout"
FAIL cross-origin > w => w["test"] = "" assert_not_equals: Report not received got disallowed value "timeout"
PASS cross-origin > w => w.opener
FAIL cross-origin > w => w.opener assert_not_equals: Report not received got disallowed value "timeout"
FAIL cross-origin > w => w.opener = "" assert_not_equals: Report not received got disallowed value "timeout"
PASS cross-origin > w => w.postMessage("")
PASS cross-origin > w => w.postMessage("", "")
PASS cross-origin > w => w.self
PASS cross-origin > w => w.top
PASS cross-origin > w => w.window
FAIL cross-origin > w => w.postMessage("") assert_not_equals: Report not received got disallowed value "timeout"
FAIL cross-origin > w => w.postMessage("", "") assert_not_equals: Report not received got disallowed value "timeout"
FAIL cross-origin > w => w.self assert_not_equals: Report not received got disallowed value "timeout"
FAIL cross-origin > w => w.top assert_not_equals: Report not received got disallowed value "timeout"
FAIL cross-origin > w => w.window assert_not_equals: Report not received got disallowed value "timeout"
FAIL same-site > w => w.blur() assert_not_equals: Report not received got disallowed value "timeout"
FAIL same-site > w => w.close() assert_not_equals: Report not received got disallowed value "timeout"
PASS same-site > w => w.closed
FAIL same-site > w => w.closed assert_not_equals: Report not received got disallowed value "timeout"
FAIL same-site > w => w.focus() assert_not_equals: Report not received got disallowed value "timeout"
PASS same-site > w => w.frames
FAIL same-site > w => w.frames assert_not_equals: Report not received got disallowed value "timeout"
FAIL same-site > w => w[0] assert_not_equals: Report not received got disallowed value "timeout"
FAIL same-site > w => w[0] = "" assert_not_equals: Report not received got disallowed value "timeout"
PASS same-site > w => w.length
FAIL same-site > w => w.length assert_not_equals: Report not received got disallowed value "timeout"
FAIL same-site > w => w.location assert_not_equals: Report not received got disallowed value "timeout"
FAIL same-site > w => w.location = "#" assert_not_equals: Report not received got disallowed value "timeout"
FAIL same-site > w => w["test"] assert_not_equals: Report not received got disallowed value "timeout"
FAIL same-site > w => w["test"] = "" assert_not_equals: Report not received got disallowed value "timeout"
PASS same-site > w => w.opener
FAIL same-site > w => w.opener assert_not_equals: Report not received got disallowed value "timeout"
FAIL same-site > w => w.opener = "" assert_not_equals: Report not received got disallowed value "timeout"
FAIL same-site > w => w.postMessage("") assert_not_equals: Report not received got disallowed value "timeout"
FAIL same-site > w => w.postMessage("", "") assert_not_equals: Report not received got disallowed value "timeout"
PASS same-site > w => w.self
PASS same-site > w => w.top
FAIL same-site > w => w.self assert_not_equals: Report not received got disallowed value "timeout"
FAIL same-site > w => w.top assert_not_equals: Report not received got disallowed value "timeout"
FAIL same-site > w => w.window assert_not_equals: Report not received got disallowed value "timeout"
Harness: the test ran to completion.
......@@ -47,7 +47,7 @@ origin.forEach(([origin_name, origin]) => {
const reportTo = reportToHeaders(report_token);
const openee_url = origin+ executor_path +
reportTo.header + reportTo.coopReportOnlySameOriginHeader + coep_header +
reportTo.header + reportTo.coopSameOriginHeader + coep_header +
`&uuid=${executor_token}`;
const openee = window.open(openee_url);
t.add_cleanup(() => send(executor_token, "window.close()"))
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment