Bug 179554 – Massive "Server-process-to-context-process" connection overhaul

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Brady Eidson]

Add standalone WebCore abstraction for "connection to worker context process"

Currently only the StorageProcess in WebKit knows about this. WebCore needs to know.

[Brady Eidson]

Rename:

Massive "Server-process-to-context-process" overhaul

There was so much unused and unnecessary code here, and the new model will make so much sense going forward.

[Brady Eidson]

Created attachment 326783 [details]
WIP patch

[Brady Eidson]

Created attachment 326787 [details]
Patch

[Brady Eidson]

Created attachment 326797 [details]
WIP

[Brady Eidson]

Created attachment 326800 [details]
WIP

[Brady Eidson]

Created attachment 326807 [details]
PFR

[Brady Eidson]

Created attachment 326809 [details]
Patch

[Brady Eidson]

I have the encodeEnum thing cleaned up locally, please keep reviewing.

[Chris Dumez]

Comment on attachment 326809 [details]
Patch

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

r=me with comments.

> Source/WebCore/workers/service/ServiceWorkerContextData.h:54
> +    encoder.encodeEnum(workerType);

Doesn't << work for enums?

> Source/WebCore/workers/service/server/SWServer.cpp:313
> +void SWServer::fireInstallEvent(ServiceWorkerIdentifier serviceWorkerIdentifier)

It can be done in a follow up but this is a bit unfortunate because the call site of this method has a SWServerWorker object. The call site should pass in this object instead of merely the identifier.

> Source/WebCore/workers/service/server/SWServer.cpp:330
> +void SWServer::fireActivateEvent(ServiceWorkerIdentifier serviceWorkerIdentifier)

ditto.

> Source/WebCore/workers/service/server/SWServerJobQueue.cpp:138
> +    m_server.fireInstallEvent(registration->installingWorker()->identifier());

Here we should pass in *registration->installingWorker().

> Source/WebCore/workers/service/server/SWServerJobQueue.cpp:226
> +    server.fireActivateEvent(registration.activeWorker()->identifier());

Here we should pass in *registration.activeWorker().

> Source/WebCore/workers/service/server/SWServerToContextConnection.cpp:57
> +    auto taken = allConnections().take(m_identifier);

Just remove(), not take. renove() returns a boolean that you can check in the assertion. Take() is unnecessarily inefficient here.

> Source/WebKit/StorageProcess/ServiceWorker/WebSWServerToContextConnection.h:51
> +    WebSWServerToContextConnection(Ref<IPC::Connection>&&);

explicit?

> Source/WebKit/StorageProcess/ServiceWorker/WebSWServerToContextConnection.h:57
> +    // Messages to the SW host WebProcess

Missing period at the end.

[Brady Eidson]

Created attachment 326820 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 326820 [details]
Patch

Rejecting attachment 326820 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'apply-attachment', '--no-update', '--non-interactive', 326820, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
 succeeded at 8213 (offset 2 lines).
Hunk #7 succeeded at 9138 (offset 2 lines).
Hunk #8 succeeded at 10805 (offset 2 lines).
patching file Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.cpp
patching file Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.h
patching file Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.messages.in

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: http://webkit-queues.webkit.org/results/5218446

[Brady Eidson]

Created attachment 326821 [details]
Patch

[Brady Eidson]

UGH rebase nightmares.

[Brady Eidson]

Created attachment 326837 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 326837 [details]
Patch

Clearing flags on attachment: 326837

Committed r224801: <https://trac.webkit.org/changeset/224801>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/35562068>