Bug 190094 – Make crossOriginObject.then undefined for promises

• Component: Bindings
• Status: RESOLVED FIXED

[Chris Dumez]

Make crossOriginObject.then undefined for promises. This allows promises to work better with cross-origin WindowProxy and Location objects.

Specification:
- https://github.com/whatwg/html/pull/3242
- https://github.com/whatwg/dom/issues/536

[Chris Dumez]

Created attachment 351126 [details]
Patch

[Darin Adler]

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.

[Chris Dumez]

Created attachment 351145 [details]
Patch

[EWS Watchlist]

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

[EWS Watchlist]

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

[Chris Dumez]

Created attachment 351156 [details]
Patch

[EWS Watchlist]

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

[EWS Watchlist]

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

[Chris Dumez]

Created attachment 351163 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 351163 [details]
Patch

Clearing flags on attachment: 351163

Committed r236661: <https://trac.webkit.org/changeset/236661>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/44910057>