Rename expect-no-embedded-resources Document-Policy to the new name [chromium/src : main]
Alex N. Jose (Gerrit)
Alex N. Jose added 1 comment![]( "Open in Gerrit")
Could I get your review of this CL? This is essentially a rename of the existing policy name to reflect the new name.
Related details
- Chromium IPC Reviews
- David Baron
- Domenic Denicola
- Ian Clelland
- Kouhei Ueno
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
gwsq (Gerrit)
Message from gwsq![]( "Open in Gerrit")
From googleclient/chrome/chromium_gwsq/ipc/config.gwsq:
IPC: dch...@chromium.org
📎 It looks like you’re making a possibly security-sensitive change! 📎 IPC security review isn’t a rubberstamp, so your friendly security reviewer will need a fair amount of context to review your CL effectively. Please review your CL description and code comments to make sure they provide context for someone unfamiliar with your project/area. Pay special attention to where data comes from and which processes it flows between (and their privilege levels). Feel free to point your security reviewer at design docs, bugs, or other links if you can’t reasonably make a self-contained CL description. (Also see https://cbea.ms/git-commit/).
IPC reviewer(s): dch...@chromium.org
Reviewer source(s):
dch...@chromium.org is from context(googleclient/chrome/chromium_gwsq/ipc/config.gwsq)
Related details
- Daniel Cheng
- David Baron
- Domenic Denicola
- Ian Clelland
- Kouhei Ueno
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Clelland (Gerrit)
Ian Clelland voted Code-Review+1![]( "Open in Gerrit")
- Alex N. Jose
- Daniel Cheng
- David Baron
- Domenic Denicola
- Kouhei Ueno
Code-Coverage
Code-Review
David Baron (Gerrit)
David Baron voted and added 1 comment![]( "Open in Gerrit")
Votes added by David Baron
| Code-Review | +1 |
1 comment
LGTM, although I wonder if you should also be renaming the RuntimeEnabledFeature.
(Also, that's a lot of reviewers; it might be good to specify who should be reviewing what. I suspect I was included for `VirtualTestSuites`, and I think you probably should wait for a review from someone in `third_party/blink/common/permissions_policy/OWNERS`.)
Related details
- Alex N. Jose
- Daniel Cheng
- Domenic Denicola
- Kouhei Ueno
David Baron (Gerrit)
David Baron added 1 comment![]( "Open in Gerrit")
LGTM, although I wonder if you should also be renaming the RuntimeEnabledFeature.
(Also, that's a lot of reviewers; it might be good to specify who should be reviewing what. I suspect I was included for `VirtualTestSuites`, and I think you probably should wait for a review from someone in `third_party/blink/common/permissions_policy/OWNERS`.)
(ah, you already have that now)
Alex N. Jose (Gerrit)
Alex N. Jose added 1 comment![]( "Open in Gerrit")
David BaronLGTM, although I wonder if you should also be renaming the RuntimeEnabledFeature.
(Also, that's a lot of reviewers; it might be good to specify who should be reviewing what. I suspect I was included for `VirtualTestSuites`, and I think you probably should wait for a review from someone in `third_party/blink/common/permissions_policy/OWNERS`.)
(ah, you already have that now)
Thanks for the review, dbaron@. The RuntimeEnabledFeature is also being renamed in the CL, to `DocumentPolicyExpectNoLinkedResources`. Let me know if you were pointing at another rename I overlooked. Thanks so much!
Related details
David Baron (Gerrit)
David Baron added 1 comment![]( "Open in Gerrit")
David BaronLGTM, although I wonder if you should also be renaming the RuntimeEnabledFeature.
(Also, that's a lot of reviewers; it might be good to specify who should be reviewing what. I suspect I was included for `VirtualTestSuites`, and I think you probably should wait for a review from someone in `third_party/blink/common/permissions_policy/OWNERS`.)
Alex N. Jose(ah, you already have that now)
Thanks for the review, dbaron@. The RuntimeEnabledFeature is also being renamed in the CL, to `DocumentPolicyExpectNoLinkedResources`. Let me know if you were pointing at another rename I overlooked. Thanks so much!
- Alex N. Jose
Daniel Cheng (Gerrit)
Daniel Cheng voted and added 2 comments![]( "Open in Gerrit")
Votes added by Daniel Cheng
| Code-Review | +1 |
2 comments
LGTM w/nit
// Hints if the document is not expecting linked resources.Can we add a spec reference to where DocumentPolicy is defined, maybe at line 7? So people can go find it and figure out what "linked resources" means, for example.
Related details
- Alex N. Jose
- Domenic Denicola
- Kouhei Ueno
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex N. Jose (Gerrit)
Alex N. Jose added 1 comment![]( "Open in Gerrit")
// Hints if the document is not expecting linked resources.Can we add a spec reference to where DocumentPolicy is defined, maybe at line 7? So people can go find it and figure out what "linked resources" means, for example.
Added a reference to the policy explainer. Marking as resolved.
Related details
- Domenic Denicola
- Kouhei Ueno
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex N. Jose (Gerrit)
Alex N. Jose voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
4 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: third_party/blink/public/mojom/permissions_policy/document_policy_feature.mojom
Insertions: 1, Deletions: 0.
@@ -41,6 +41,7 @@
// from the Reporting API.
kIncludeJSCallStacksInCrashReports = 14,
// Hints if the document is not expecting linked resources.
+ // https://github.com/explainers-by-googlers/expect-no-linked-resources
kExpectNoLinkedResources = 15,
// Don't change assigned numbers of any item, and don't reuse removed slots.
```
Change information
Rename expect-no-embedded-resources Document-Policy to the new name
The implementation of this policy lags behind the proposal name change.
This CL renames the Document-Policy: expect-no-embedded-resources
to expect-no-linked-resources to reflect the revised policy name.
https://chromestatus.com/feature/5202800863346688
- M third_party/blink/public/mojom/permissions_policy/document_policy_feature.mojom
- M third_party/blink/public/mojom/use_counter/metrics/web_feature.mojom
- M third_party/blink/renderer/core/html/parser/html_document_parser.cc
- M third_party/blink/renderer/core/permissions_policy/document_policy_features.json5
- M third_party/blink/renderer/platform/runtime_enabled_features.json5
- M third_party/blink/web_tests/VirtualTestSuites
- D third_party/blink/web_tests/http/tests/expect-no-embedded-resources/no-preloadscanning-when-no-resources.php
- A third_party/blink/web_tests/http/tests/expect-no-linked-resources/no-preloadscanning-when-no-resources.php
- D third_party/blink/web_tests/virtual/expect-no-embedded-resources/README.md
- A third_party/blink/web_tests/virtual/expect-no-linked-resources/README.md
- M tools/metrics/histograms/enums.xml
Code-Review: +1 by David Baron, +1 by Daniel Cheng, +1 by Ian Clelland
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |