Bug 227673

Summary: Use AbortSignal's [PrivateIdentifier] whenSignalAborted() static method
Product: WebKit Reporter: Alexey Shvayka <ashvayka>
Component: DOMAssignee: Alexey Shvayka <ashvayka>
Status: RESOLVED CONFIGURATION CHANGED    
Severity: Enhancement CC: ahmad.saleem792, ap, benjamin, bfulgham, calvaris, cdumez, commit-queue, ews-watchlist, joepeck, rniwa, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on: 227275, 227732    
Bug Blocks:    
Attachments:
Description Flags
Patch
ews-feeder: commit-queue-
Patch
none
Patch none

Alexey Shvayka
Reported 2021-07-04 07:40:17 PDT
[WebIDL] Use AbortSignal's [PrivateIdentifier] whenSignalAborted() static method
Attachments
Patch (4.97 KB, patch)
2021-07-04 07:44 PDT, Alexey Shvayka 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (4.41 KB, patch)
2021-07-04 08:02 PDT, Alexey Shvayka 		no flags
DetailsFormatted DiffDiff
Patch (10.01 KB, patch)
2021-07-05 02:39 PDT, Alexey Shvayka 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Alexey Shvayka
Comment 1 2021-07-04 07:44:15 PDT
Created attachment 432862 [details] Patch
Alexey Shvayka
Comment 2 2021-07-04 08:02:58 PDT
Created attachment 432864 [details] Patch Revert bindings/js/WebCoreBuiltinNames.h change.
Alexey Shvayka
Comment 3 2021-07-05 02:39:44 PDT
Created attachment 432876 [details] Patch Adjust WPT expectations.
youenn fablet
Comment 4 2021-07-05 03:15:29 PDT
Comment on attachment 432876 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=432876&action=review > LayoutTests/imported/w3c/web-platform-tests/streams/piping/abort.any.worker-expected.txt:8 > +FAIL a signal argument '[object AbortSignal]' should cause pipeTo() to reject Argument 1 ('object') to AbortSignal.whenSignalAborted must be an instance of AbortSignal Hum, it seems that our check of (signal instanceof @AbortSignal) is not good enough since Object.create(AbortSignal.prototype) passes it. That makes ReadableStream.js pipeTo and pipeThrough not throwing/rejecting when checking signal while they should do. We should fix this as a follow-up. I am not sure what the best way to do is. Is there a better way than adding a PrivateIdentifier method to AbortSignal itself?
Alexey Shvayka
Comment 5 2021-07-05 07:30:28 PDT
(In reply to youenn fablet from comment #4) Thanks for review! > We should fix this as a follow-up. > I am not sure what the best way to do is. Is there a better way than adding > a PrivateIdentifier method to AbortSignal itself? In a follow-up, I will introduce @isAbortSignal global private function, mimicking JSC intrinsics, because WebIDL code generator is not equipped to generate such brand check.
Alexey Shvayka
Comment 6 2021-07-05 07:34:26 PDT
Committed r279567 (239398@main): <https://commits.webkit.org/239398@main>
Radar WebKit Bug Importer
Comment 7 2021-07-05 07:35:16 PDT
<rdar://problem/80177224>
WebKit Commit Bot
Comment 8 2021-07-06 20:09:39 PDT
Re-opened since this is blocked by bug 227732
Ahmad Saleem
Comment 9 2022-08-10 09:45:03 PDT
This got backed out in following commit: https://github.com/WebKit/WebKit/commit/d0333311de3aab7c2067acc61e7b9e0eb881eb03 But Safari Technical Preview 150 passes all test but fail harness test: https://wpt.fyi/results/streams/piping?label=master&label=experimental&aligned&view=subtest&q=pipe-through Is something still required since all tests passes here? Thanks!
Alexey Shvayka
Comment 10 2022-08-10 09:50:26 PDT
(In reply to Ahmad Saleem from comment #9) > This got backed out in following commit: > > https://github.com/WebKit/WebKit/commit/ > d0333311de3aab7c2067acc61e7b9e0eb881eb03 > > But Safari Technical Preview 150 passes all test but fail harness test: > > https://wpt.fyi/results/streams/ > piping?label=master&label=experimental&aligned&view=subtest&q=pipe-through > > Is something still required since all tests passes here? Thanks! Purely a refactoring at this point: it would be nice to drop `whenSignalAborted` from JSDOMGlobalObject.cpp.
Alexey Proskuryakov
Comment 11 2022-08-19 16:46:24 PDT
Comment on attachment 432876 [details] Patch Clearing r+ given the above.
Alexey Shvayka
Comment 12 2023-01-25 17:05:30 PST
Closing since whenSignalAborted() was removed in https://github.com/WebKit/WebKit/commit/cb01f4ca942d355c84e1c6248560cf63a58ebfd0.
Note You need to log in before you can comment on or make changes to this bug.