[webkit-reviews] review granted: [Bug 234127] Implement AbortSignal.throwIfAborted : [Attachment 446731] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 234127: Implement AbortSignal.throwIfAborted
https://bugs.webkit.org/show_bug.cgi?id=234127

Attachment 446731: Patch

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




--- Comment #3 from Darin Adler <darin at apple.com> ---
Comment on attachment 446731
  --> https://bugs.webkit.org/attachment.cgi?id=446731
Patch

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

> Source/WebCore/dom/AbortSignal.cpp:124
> +    JSC::throwException(&lexicalGlobalObject, scope,
static_cast<JSC::JSValue>(m_reason));

Don’t need to write JSC::throwException; argument-dependent lookup should work
unless AbortSignal has a member function named throwException.

The cast to JSC::JSValue seems really sad. I would probably add one more local
variable:

    JSC::JSValue reason = m_reason;

Just because assignment is less forceful than a static_cast. Another solution
would be to add a member function to JSValueInWrappedObject that returns the
value. Could be named value() or get(). And use that instead of the
static_cast.