Bug 234127 – Implement AbortSignal.throwIfAborted

• Component: DOM
• Status: RESOLVED FIXED

[Anne van Kesteren]

Convenience method for web developers writing signal-accepting operations.

Discussion: https://github.com/whatwg/dom/issues/927
Standard change: https://github.com/whatwg/dom/pull/1034
Tests: https://github.com/web-platform-tests/wpt/pull/31947

[Chris Dumez]

Created attachment 446731 [details]
Patch

[EWS Watchlist]

This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess

[Darin Adler]

Comment on attachment 446731 [details]
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.

[Chris Dumez]

Created attachment 446829 [details]
Patch

[Chris Dumez]

(In reply to Darin Adler from comment #3)
> Comment on attachment 446731 [details]
> 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.

You're right, I didn't need the JSC::.
 
> 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.

I actually didn't need the static_cast<> at all it seems.

[EWS]

Committed r286904 (245130@main): <https://commits.webkit.org/245130@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 446829 [details].

[Radar WebKit Bug Importer]

<rdar://problem/86353898>