Attention is currently required from: Brandon Heenan, Charlie Reis, Julian Pastarmov, Kentaro Hara.
Patch set 13:Commit-Queue +1
1 comment:
Commit Message:
Patch Set #12, Line 7: thrrottling
typo
Thanks ... I saw that earlier then forgot to fix it when I uploaded the latest patchset. :-)
To view, visit change 3966873. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Brandon Heenan, Charlie Reis, James Maclean, Julian Pastarmov.
Patch set 13:Code-Review +1
1 comment:
Patchset:
blink LGTM
To view, visit change 3966873. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Charlie Reis, James Maclean.
Patch set 13:Code-Review +1
3 comments:
Patchset:
lgtm with a rather important nit though.
File components/policy/resources/templates/policy_definitions/Miscellaneous/ThrottleNonVisibleCrossOriginIframesAllowed.yaml:
Patch Set #13, Line 8: experiments.
please replace experiments with "Chrome variations" and remove one of the two dots here.
File components/policy/resources/templates/policy_definitions/Miscellaneous/ThrottleNonVisibleCrossOriginIframesEnabled.yaml:
Patch Set #10, Line 6: https://crbug.com/958475
If it's ok with you, I went back to your earlier suggestion of using a chromestatus.com link. […]
That's fine thanks!
To view, visit change 3966873. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Charlie Reis.
2 comments:
Patchset:
Thanks for the reviews!
File components/policy/resources/templates/policy_definitions/Miscellaneous/ThrottleNonVisibleCrossOriginIframesAllowed.yaml:
Patch Set #13, Line 8: experiments.
please replace experiments with "Chrome variations" and remove one of the two dots here.
Done
To view, visit change 3966873. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Charlie Reis.
Patch set 14:Commit-Queue +2
Attention is currently required from: Charlie Reis.
1 comment:
Patchset:
pastarmovj@ - It seems that registering the pref as a profile pref is necessary for it to pass ChunkedPolicyPrefsTest.PolicyToPrefsMapping/6, so I've restored it.
Let me know if that's ok, or if I've missed something. I'll hold off submitting it until I hear back from you.
To view, visit change 3966873. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Charlie Reis, James Maclean.
Patch set 15:Code-Review +1
2 comments:
Patchset:
I commented in the pref test file what needs to be done to get the test passing without the pref being in the profile pref store.
File chrome/test/data/policy/policy_test_cases.json:
You need to specify "location": "local_state" here as well to tell the test that the pref is in local state. Same for all other tests.
To view, visit change 3966873. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Charlie Reis.
3 comments:
Patchset:
Thanks again!
File chrome/test/data/policy/policy_test_cases.json:
You need to specify "location": "local_state" here as well to tell the test that the pref is in loca […]
Got it, thanks!
File components/policy/resources/templates/policy_definitions/Miscellaneous/ThrottleNonVisibleCrossOriginIframesEnabled.yaml:
Patch Set #6, Line 6: When true, this policy allows users to enable the throttling of cross-origin
I'll defer this until we decide on the previous comment.
Done
To view, visit change 3966873. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Charlie Reis.
Patch set 16:Commit-Queue +2
Attention is currently required from: Charlie Reis, James Maclean.
Patch set 16:Commit-Queue +2
To view, visit change 3966873. To unsubscribe, or for help writing mail filters, visit settings.
Chromium LUCI CQ submitted this change.
15 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: chrome/browser/chrome_content_browser_client.cc
Insertions: 0, Deletions: 3.
@@ -1582,9 +1582,6 @@
prefs::kAccessControlAllowMethodsInCORSPreflightSpecConformant, true);
registry->RegisterBooleanPref(
- prefs::kThrottleNonVisibleCrossOriginIframesAllowed, true);
-
- registry->RegisterBooleanPref(
policy::policy_prefs::kOffsetParentNewSpecBehaviorEnabled, true);
registry->RegisterBooleanPref(
policy::policy_prefs::kSendMouseEventsDisabledFormControlsEnabled, true);
```
```
The name of the file: chrome/test/data/policy/policy_test_cases.json
Insertions: 3, Deletions: 0.
@@ -8331,6 +8331,7 @@
},
"prefs": {
"throttle_non_visible_cross_origin_iframes_allowed": {
+ "location": "local_state",
"value": true
}
}
@@ -8341,6 +8342,7 @@
},
"prefs": {
"throttle_non_visible_cross_origin_iframes_allowed": {
+ "location": "local_state",
"value": false
}
}
@@ -8349,6 +8351,7 @@
"policies": {},
"prefs": {
"throttle_non_visible_cross_origin_iframes_allowed": {
+ "location": "local_state",
"default_value": true
}
}
```
Add enterprise policy to disable cross-origin non-visible iframe throttling.
This CL implements an enterprise policy to allow disabling of the
ThrottleDisplayNoneAndVisibilityHiddenCrossOriginIframes feature in
Blink.
Since the override requires providing a helper function directly in
third_party/blink/common, it seems simpler to not use
RuntimeEnabledFeatures for this feature, and instead just directly
declare it in blink::features. This requires small changes in the
tests, but overall makes the code easier to understand.
Bug: 1330592
Change-Id: I75eda216805864a266716d16d06fdb9d3d581cbc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3966873
Reviewed-by: Avi Drissman <a...@chromium.org>
Commit-Queue: James Maclean <wjma...@chromium.org>
Reviewed-by: Julian Pastarmov <pasta...@chromium.org>
Reviewed-by: Kentaro Hara <har...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1075923}
---
M chrome/browser/chrome_browser_main.cc
M chrome/browser/chrome_content_browser_client.cc
M chrome/browser/policy/configuration_policy_handler_list_factory.cc
M chrome/common/pref_names.cc
M chrome/common/pref_names.h
M chrome/test/data/policy/policy_test_cases.json
M components/policy/resources/templates/policies.yaml
A components/policy/resources/templates/policy_definitions/Miscellaneous/ThrottleNonVisibleCrossOriginIframesAllowed.yaml
M content/public/common/content_switch_dependent_feature_overrides.cc
M third_party/blink/common/features.cc
M third_party/blink/common/switches.cc
M third_party/blink/public/common/features.h
M third_party/blink/public/common/switches.h
M third_party/blink/renderer/core/frame/frame_view.cc
M third_party/blink/renderer/core/scheduler_integration_tests/frame_throttling_test.cc
M third_party/blink/renderer/platform/runtime_enabled_features.json5
M tools/metrics/histograms/enums.xml
17 files changed, 193 insertions(+), 21 deletions(-)