Refactor DocumentPolicyViolation reporting to use pure dictionary approach [chromium/src : main]

[Yoav Weiss (@Shopify) (Gerrit)]

Attention needed from Helmut Januschka

Yoav Weiss (@Shopify) added 2 comments

Patchset-level comments

File-level comment, Patchset 10 (Latest):

Yoav Weiss (@Shopify) . resolved

Thanks for taking this on!!

Is this WPT-testable?

File third_party/blink/renderer/core/frame/reporting_context.cc

Line 203, Patchset 10 (Latest): // DocumentPolicyViolation is now dictionary-based

Yoav Weiss (@Shopify) . unresolved

I wonder if it makes sense to create a dictionary-based LocationReportBody. WDYT?

Open in Gerrit

Related details

Attention is currently required from:

• Helmut Januschka

Submit Requirements:

• Code-Coverage
• Code-Owners
• Code-Review
• No-Unresolved-Comments

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: I3d44bf82a35e46217a3d7d51e76a39dc2de9446e

Gerrit-Change-Number: 6987161

Gerrit-PatchSet: 10

Gerrit-Owner: Helmut Januschka <hel...@januschka.com>

Gerrit-Reviewer: Helmut Januschka <hel...@januschka.com>

Gerrit-CC: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-CC: Kentaro Hara <har...@chromium.org>

Gerrit-CC: Raphael Kubo da Costa <ku...@igalia.com>

Gerrit-CC: Yoav Weiss (@Shopify) <yoav...@chromium.org>

Gerrit-CC: chromium...@chromium.org

Gerrit-Attention: Helmut Januschka <hel...@januschka.com>

Gerrit-Comment-Date: Tue, 30 Sep 2025 05:29:57 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[Dominic Farolino (Gerrit)]

Attention needed from Helmut Januschka

Dominic Farolino added 3 comments

File third_party/blink/renderer/core/frame/report.idl

Line 12, Patchset 10 (Latest): [CallWith=ScriptState] readonly attribute any body;

Dominic Farolino . unresolved

Can't this return a `ReportBodyDict` instead of `any`?

File third_party/blink/renderer/core/frame/report_body.idl

Line 7, Patchset 10 (Latest):[Exposed=(Window, Worker)] interface ReportBody {

Dominic Farolino . unresolved

Let's not reformat any of this if we're not materially changing it.

Line 15, Patchset 10 (Latest): DOMString? sourceFile;

Dominic Farolino . unresolved

Hmm, how does this match the spec PR? The spec PR shows that `ReportBody` is a completely empty dictionary. Surely not every single "derived" instance of `ReportBodyDict` should get these members.

Open in Gerrit

Related details

Attention is currently required from:

• Helmut Januschka

Submit Requirements:

• Code-Coverage
• Code-Owners
• Code-Review
• No-Unresolved-Comments

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: I3d44bf82a35e46217a3d7d51e76a39dc2de9446e

Gerrit-Change-Number: 6987161

Gerrit-PatchSet: 10

Gerrit-Owner: Helmut Januschka <hel...@januschka.com>

Gerrit-Reviewer: Helmut Januschka <hel...@januschka.com>

Gerrit-CC: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-CC: Dominic Farolino <d...@chromium.org>

Gerrit-CC: Kentaro Hara <har...@chromium.org>

Gerrit-CC: Raphael Kubo da Costa <ku...@igalia.com>

Gerrit-CC: Yoav Weiss (@Shopify) <yoav...@chromium.org>

Gerrit-CC: chromium...@chromium.org

Gerrit-Attention: Helmut Januschka <hel...@januschka.com>

Gerrit-Comment-Date: Tue, 30 Sep 2025 17:37:13 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[Dominic Farolino (Gerrit)]

Attention needed from Helmut Januschka

Dominic Farolino added 2 comments

Commit Message

Line 10, Patchset 10 (Latest):implementation to align with W3C Reporting API specification changes.

Dominic Farolino . unresolved

Please link to https://github.com/w3c/reporting/pull/284/files from here if possible.

File third_party/blink/renderer/core/frame/report.idl

Line 12, Patchset 10 (Latest): [CallWith=ScriptState] readonly attribute any body;

Dominic Farolino . unresolved

Also, this is definitely a breaking change, right? That is, the `body` returned from this getter will no longer have a `toJSON()` method of its own, which seems like it could cause compatibility issues for sites currently relying on it.

@Yoav: do you know what the story is here? I'm a little surprised the spec PR landed so smoothly with this breaking web-observable change, unless I'm misunderstanding how this is wired up.

Either way, this does seem web-observable for any callers of the "new" `Report` constructor that pass in a dictionary instead of a `ReportBody` interface object. And there is at least one new user of that constructor introduced in this CL (in `third_party/blink/renderer/core/frame/local_dom_window.cc`), so we should flag guard this. Author: could you add a new runtime enabled feature for this, so that we can flag-guard all of the web-observable changes?

Gerrit-Comment-Date: Tue, 30 Sep 2025 17:47:45 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No