[bindings] Fix names on anonymous cross origin functions [chromium/src : main]
0 views
Skip to first unread message
Dave Tapuska (Gerrit)
Jun 21, 2024, 2:28:22 PM (8 days ago) Jun 21
to Michael Lippautz, Yuki Shiino, Chromium LUCI CQ, chromium...@chromium.org, Kentaro Hara, blink-revie...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org
Attention needed from Michael Lippautz and Yuki Shiino
Dave Tapuska added 1 comment![]( "Open in Gerrit")
File third_party/blink/renderer/platform/bindings/v8_cross_origin_property_support.cc
Line 58, Patchset 3 (Latest):
builder.Append(func_length > 0 ? "set " : "get ");Dave Tapuska . unresolved
I don't know if there is a better way to do this...
Related details
Attention is currently required from:
- Michael Lippautz
- Yuki Shiino
Submit Requirements:
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dave Tapuska (Gerrit)
Jun 21, 2024, 2:29:31 PM (8 days ago) Jun 21
to Michael Lippautz, Yuki Shiino, Chromium LUCI CQ, chromium...@chromium.org, Kentaro Hara, blink-revie...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org
Attention needed from Michael Lippautz and Yuki Shiino
Dave Tapuska added 1 comment![]( "Open in Gerrit")
File third_party/blink/web_tests/http/tests/security/cross-frame-access-get-override.html
Line 19, Patchset 3 (Parent):
shouldBe("targetWindow.focus.name", "''");Dave Tapuska . unresolved
This changes the test back to what matches Webkit's version (which I think captures the original intent of the test case).
Yuki Shiino (Gerrit)
Jun 23, 2024, 9:10:10 PM (6 days ago) Jun 23
to Dave Tapuska, Yuki Shiino, Michael Lippautz, Chromium LUCI CQ, chromium...@chromium.org, Kentaro Hara, blink-revie...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org
Attention needed from Dave Tapuska and Michael Lippautz
Yuki Shiino voted and added 5 comments![]( "Open in Gerrit")
Votes added by Yuki Shiino
| Code-Review | +1 |
5 comments
Patchset-level comments
File third_party/blink/renderer/bindings/scripts/bind_gen/interface.py
Line 4063, Patchset 3 (Latest):
${info}.GetIsolate(), attribute.name, attribute.get_callback, 0,Yuki Shiino . unresolved
As an alternative (if you like), I think we can pass `"get {}".format(attribute.name)` here. Then we don't need a runtime string calculation.
Keep 79-column rule in Python if applied.
Line 4084, Patchset 3 (Latest):
${info}.GetIsolate(), operation.name, operation.callback, operation.func_length,Yuki Shiino . unresolved
style nit: 79-column rule in Python.
File third_party/blink/renderer/platform/bindings/v8_cross_origin_property_support.cc
Line 58, Patchset 3 (Latest):
builder.Append(func_length > 0 ? "set " : "get ");Dave Tapuska . resolved
I don't know if there is a better way to do this...
Yuki Shiino
Given this is an IDL attribute, I think this is fine.
File third_party/blink/web_tests/http/tests/security/cross-frame-access-get-override.html
Line 19, Patchset 3 (Parent):
shouldBe("targetWindow.focus.name", "''");Dave Tapuska . resolved
This changes the test back to what matches Webkit's version (which I think captures the original intent of the test case).
Attention is currently required from:
- Dave Tapuska
- Michael Lippautz
Submit Requirements:
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Lippautz (Gerrit)
Jun 24, 2024, 3:17:27 AM (6 days ago) Jun 24
to Dave Tapuska, Yuki Shiino, Chromium LUCI CQ, chromium...@chromium.org, Kentaro Hara, blink-revie...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org
Attention needed from Dave Tapuska
Michael Lippautz voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
Attention is currently required from:
- Dave Tapuska
Submit Requirements:
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dave Tapuska (Gerrit)
Jun 24, 2024, 12:52:30 PM (5 days ago) Jun 24
to Michael Lippautz, Yuki Shiino, Chromium LUCI CQ, chromium...@chromium.org, Kentaro Hara, blink-revie...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org
Dave Tapuska voted and added 2 comments![]( "Open in Gerrit")
Votes added by Dave Tapuska
| Commit-Queue | +2 |
2 comments
File third_party/blink/renderer/bindings/scripts/bind_gen/interface.py
Line 4063, Patchset 3:
${info}.GetIsolate(), attribute.name, attribute.get_callback, 0,Yuki Shiino . resolved
As an alternative (if you like), I think we can pass `"get {}".format(attribute.name)` here. Then we don't need a runtime string calculation.
Keep 79-column rule in Python if applied.
Dave Tapuska
We can't python format the attribute.name because it is a variable from kCrossOriginAttributeTable. So left it as it is. We could add multiple varaibles to that table but I think this is ok.
Line 4084, Patchset 3:
${info}.GetIsolate(), operation.name, operation.callback, operation.func_length,Yuki Shiino . resolved
style nit: 79-column rule in Python.
Submit Requirements:
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Jun 24, 2024, 1:48:46 PM (5 days ago) Jun 24
to Dave Tapuska, Michael Lippautz, Yuki Shiino, chromium...@chromium.org, Kentaro Hara, blink-revie...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org
Chromium LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
3 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/renderer/bindings/scripts/bind_gen/interface.py
Insertions: 2, Deletions: 2.
@@ -4081,8 +4081,8 @@
continue;
v8::Local<v8::Function> function;
if (!bindings::GetCrossOriginFunction(
- ${info}.GetIsolate(), operation.name, operation.callback, operation.func_length,
- ${class_name}::GetWrapperTypeInfo())
+ ${info}.GetIsolate(), operation.name, operation.callback,
+ operation.func_length, ${class_name}::GetWrapperTypeInfo())
.ToLocal(&function)) {
// Exception was thrown which means that the request was intercepted.
return v8::Intercepted::kYes;
```
Change information
Commit message:
[bindings] Fix names on anonymous cross origin functions
The name on the functions was the empty string. This fixes 4
WPT tests.
Bug: 40767616
Change-Id: I2a38c48cd274e1f2e16633fca38069a3d9cd8ec3
Reviewed-by: Yuki Shiino <yukis...@chromium.org>
Commit-Queue: Dave Tapuska <dtap...@chromium.org>
Reviewed-by: Michael Lippautz <mlip...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1318661}
Files:
- M third_party/blink/renderer/bindings/scripts/bind_gen/interface.py
- M third_party/blink/renderer/platform/bindings/v8_cross_origin_property_support.cc
- M third_party/blink/renderer/platform/bindings/v8_cross_origin_property_support.h
- D third_party/blink/web_tests/external/wpt/html/browsers/origin/cross-origin-objects/cross-origin-objects-function-name-expected.txt
- M third_party/blink/web_tests/http/tests/security/cross-frame-access-get-override-expected.txt
- M third_party/blink/web_tests/http/tests/security/cross-frame-access-get-override.html
- D third_party/blink/web_tests/http/tests/security/cross-frame-access-location-get-override-expected.txt
- D third_party/blink/web_tests/http/tests/security/cross-frame-access-location-prototype-expected.txt
- D third_party/blink/web_tests/http/tests/security/cross-frame-access-location-put-expected.txt
Change size: M
Delta: 9 files changed, 28 insertions(+), 55 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Michael Lippautz, +1 by Yuki Shiino
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages