Bug 239066

Summary: Expose workers as service worker clients and implement registration matching for dedicated workers
Product: WebKit Reporter: youenn fablet <youennf>
Component: Service WorkersAssignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, esprehn+autocc, ews-watchlist, japhet, kangil.han, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=239436
https://bugs.webkit.org/show_bug.cgi?id=243409
Bug Depends on: 238992    
Bug Blocks: 239122    
Attachments:
Description Flags
Patch
none
Patch
ews-feeder: commit-queue-
Patch
none
Patch
none
Patch
none
Patch
none
Patch for landing none

Description youenn fablet 2022-04-11 05:51:24 PDT
Expose workers as service worker clients and implement registration matching for dedicated workers
Comment 1 youenn fablet 2022-04-11 06:58:29 PDT
Created attachment 457252 [details]
Patch
Comment 2 youenn fablet 2022-04-12 02:53:26 PDT
Created attachment 457324 [details]
Patch
Comment 3 youenn fablet 2022-04-12 10:36:32 PDT
Created attachment 457342 [details]
Patch
Comment 4 youenn fablet 2022-04-12 10:50:28 PDT
Created attachment 457344 [details]
Patch
Comment 5 youenn fablet 2022-04-12 10:52:32 PDT
Created attachment 457346 [details]
Patch
Comment 6 youenn fablet 2022-04-13 01:29:25 PDT
Created attachment 457513 [details]
Patch
Comment 7 youenn fablet 2022-04-13 06:04:53 PDT
Comment on attachment 457513 [details]
Patch

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

> LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/worker-interception.https-expected.txt:7
> +FAIL Verify a cors worker script served by a service worker fails dedicated worker start. assert_equals: Expected error event, but got message event instead expected "error" but got "message"

Fixed by https://bugs.webkit.org/show_bug.cgi?id=239123.
Comment 8 youenn fablet 2022-04-13 06:05:56 PDT
Follow-up patch should handle unregistering of workers and better handle blob workers.
Comment 9 Chris Dumez 2022-04-13 07:29:02 PDT
Comment on attachment 457513 [details]
Patch

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

r=me but I think there is a bug to fix before landing.

> Source/WebCore/workers/WorkerGlobalScope.cpp:662
> +    SWClientConnection& connection = swClientConnection();

Not sure why we need a local variable here.

> Source/WebCore/workers/WorkerScriptLoader.cpp:50
> +    static NeverDestroyed<HashMap<ScriptExecutionContextIdentifier, WorkerScriptLoader*>> map;

Is this only used on the main thread? I assume so since I see no locking. If so, we should use MainThreadNeverDestroyed.

> Source/WebCore/workers/service/SWClientConnection.cpp:244
> +            updateController(*document, WTFMove(newController));

The WTFMove() here looks like a bug since we're in a for loop.

> Source/WebCore/workers/service/SWClientConnection.cpp:248
> +            worker->postTaskToWorkerGlobalScope([newController = WTFMove(newController).isolatedCopy()] (auto& context) mutable {

ditto.

> Source/WebCore/workers/service/ServiceWorkerClientData.cpp:72
> +    if (is<Document>(context)) {

if (auto* document = dynamicDowncast<Document>(context)) {
Comment 10 youenn fablet 2022-04-14 00:03:03 PDT
Created attachment 457599 [details]
Patch for landing
Comment 11 EWS 2022-04-14 01:49:30 PDT
Committed r292861 (249632@main): <https://commits.webkit.org/249632@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 457599 [details].
Comment 12 Radar WebKit Bug Importer 2022-04-14 01:50:22 PDT
<rdar://problem/91742199>