[webkit-reviews] review granted: [Bug 197600] Implement Promise.allSettled : [Attachment 370789] Patch

[bugzilla-daemon at webkit.org]

Keith Miller <keith_miller at apple.com> has granted Dean Jackson
<dino at apple.com>'s request for review:
Bug 197600: Implement Promise.allSettled
https://bugs.webkit.org/show_bug.cgi?id=197600

Attachment 370789: Patch

https://bugs.webkit.org/attachment.cgi?id=370789&action=review




--- Comment #18 from Keith Miller <keith_miller at apple.com> ---
Comment on attachment 370789
  --> https://bugs.webkit.org/attachment.cgi?id=370789
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=370789&action=review

r=me.

> Source/JavaScriptCore/builtins/PromiseConstructor.js:150
> +	       promiseCapability. at resolve.@call(@undefined, values);

Why does this need to be passed undefined as the this value? Isn't @resolve an
internal method? Seems like the this value shouldn't matter.

> Source/JavaScriptCore/builtins/PromiseConstructor.js:152
> +	   promiseCapability. at reject.@call(@undefined, error);

Ditto.

> JSTests/test262/expectations.yaml:1789
> +test/intl402/Collator/ignore-invalid-unicode-ext-values.js:
> +  default: 'Test262Error: Locale en is affected by key co; value standard.
Expected SameValue(ëenû, ëen-AUû) to be true'
> +  strict mode: 'Test262Error: Locale en is affected by key co; value
standard. Expected SameValue(ëenû, ëen-AUû) to be true'

What? Does this test use promise.allSettled?

Was this already failing?

> JSTests/test262/expectations.yaml:1819
> +test/intl402/fallback-locales-are-supported.js:
> +  default: "Test262Error: Locale zh-Hans-CN is supported, but fallback
zh-Hans isn't. Expected SameValue(ë-1û, ë-1û) to be false (Testing with
Collator.)"
> +  strict mode: "Test262Error: Locale zh-Hans-CN is supported, but fallback
zh-Hans isn't. Expected SameValue(ë-1û, ë-1û) to be false (Testing with
Collator.)"

Ditto...