Bug 239066 – Expose workers as service worker clients and implement registration matching for dedicated workers

• Status: RESOLVED
• Resolution: FIXED
• Component: Service Workers

[youenn fablet]

Expose workers as service worker clients and implement registration matching for dedicated workers

[youenn fablet]

Created attachment 457252 [details]
Patch

[youenn fablet]

Created attachment 457324 [details]
Patch

[youenn fablet]

Created attachment 457342 [details]
Patch

[youenn fablet]

Created attachment 457344 [details]
Patch

[youenn fablet]

Created attachment 457346 [details]
Patch

[youenn fablet]

Created attachment 457513 [details]
Patch

[youenn fablet]

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.

[youenn fablet]

Follow-up patch should handle unregistering of workers and better handle blob workers.

[Chris Dumez]

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)) {

[youenn fablet]

Created attachment 457599 [details]
Patch for landing

[EWS]

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].

[Radar WebKit Bug Importer]

<rdar://problem/91742199>