Bug 234127

Summary: Implement AbortSignal.throwIfAborted
Product: WebKit Reporter: Anne van Kesteren <annevk>
Component: DOMAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, cdumez, clopez, darin, esprehn+autocc, ews-watchlist, ggaren, kangil.han, kondapallykalyan, sam, webkit-bug-importer, youennf, ysuzuki
Priority: P2 Keywords: InRadar
Version: Safari Technology Preview   
Hardware: Unspecified   
OS: Unspecified   
URL: https://dom.spec.whatwg.org/#dom-abortsignal-throwifaborted
Attachments:
Description Flags
Patch
none
Patch none

Description Anne van Kesteren 2021-12-10 00:36:17 PST
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
Comment 1 Chris Dumez 2021-12-10 08:02:22 PST
Created attachment 446731 [details]
Patch
Comment 2 EWS Watchlist 2021-12-10 08:03:05 PST
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
Comment 3 Darin Adler 2021-12-10 15:15:47 PST
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.
Comment 4 Chris Dumez 2021-12-10 15:23:31 PST
Created attachment 446829 [details]
Patch
Comment 5 Chris Dumez 2021-12-10 15:24:09 PST
(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.
Comment 6 EWS 2021-12-10 23:01:24 PST
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].
Comment 7 Radar WebKit Bug Importer 2021-12-10 23:02:24 PST
<rdar://problem/86353898>