Summary: | Make crossOriginObject.then undefined for promises | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||||||||||
Component: | Bindings | Assignee: | Chris Dumez <cdumez> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | annevk, cdumez, commit-queue, darin, ews-watchlist, ggaren, rniwa, sam, webkit-bug-importer, youennf | ||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
URL: | https://github.com/whatwg/html/pull/3242 | ||||||||||||||||
Attachments: |
|
Description
Chris Dumez
2018-09-28 15:14:32 PDT
Created attachment 351126 [details]
Patch
Comment on attachment 351126 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=351126&action=review > Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:177 > + if (propertyName == vm.propertyNames->builtinNames().thenPublicName() || propertyName == vm.propertyNames->toStringTagSymbol || propertyName == vm.propertyNames->hasInstanceSymbol || propertyName == vm.propertyNames->isConcatSpreadableSymbol) { I would like it if we had a named function for this check. It would have two benefits: 1) Could put *vm.propertyNames into a local variable to make the code more readable, although we could also do that here. 2) The name of the function could help make it clear what these names have in common. > Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:408 > + propertyNames.add(vm.propertyNames->builtinNames().thenPublicName()); > propertyNames.add(vm.propertyNames->toStringTagSymbol); > propertyNames.add(vm.propertyNames->hasInstanceSymbol); > propertyNames.add(vm.propertyNames->isConcatSpreadableSymbol); Would be even better if somehow the list of property names existed only once, and could be used both for the if statement above and this adding here. > Source/WebCore/bindings/js/JSLocationCustom.cpp:58 > + if (propertyName == vm.propertyNames->builtinNames().thenPublicName() || propertyName == vm.propertyNames->toStringTagSymbol || propertyName == vm.propertyNames->hasInstanceSymbol || propertyName == vm.propertyNames->isConcatSpreadableSymbol) { Annoying that this repeats the same list but does not share code with DOMWindow. Would be nice to fix that. > Source/WebCore/bindings/js/JSLocationCustom.cpp:185 > + propertyNames.add(vm.propertyNames->builtinNames().thenPublicName()); > propertyNames.add(vm.propertyNames->toStringTagSymbol); > propertyNames.add(vm.propertyNames->hasInstanceSymbol); > propertyNames.add(vm.propertyNames->isConcatSpreadableSymbol); Ditto. Created attachment 351145 [details]
Patch
Comment on attachment 351145 [details] Patch Attachment 351145 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/9388732 New failing tests: http/tests/security/cross-frame-access-enumeration.html Created attachment 351155 [details]
Archive of layout-test-results from ews102 for mac-sierra
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102 Port: mac-sierra Platform: Mac OS X 10.12.6
Created attachment 351156 [details]
Patch
Comment on attachment 351156 [details] Patch Attachment 351156 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/9389377 New failing tests: http/tests/security/cross-frame-access-enumeration.html Created attachment 351161 [details]
Archive of layout-test-results from ews102 for mac-sierra
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102 Port: mac-sierra Platform: Mac OS X 10.12.6
Created attachment 351163 [details]
Patch
Comment on attachment 351163 [details] Patch Clearing flags on attachment: 351163 Committed r236661: <https://trac.webkit.org/changeset/236661> All reviewed patches have been landed. Closing bug. |