[webkit-reviews] review granted: [Bug 189422] Move Service Worker Management from Storage Process to Network Process : [Attachment 350753] Patch

[bugzilla-daemon at webkit.org]

youenn fablet <youennf at gmail.com> has granted Sihui Liu <sihui_liu at apple.com>'s
request for review:
Bug 189422: Move Service Worker Management from Storage Process to Network
Process
https://bugs.webkit.org/show_bug.cgi?id=189422

Attachment 350753: Patch

https://bugs.webkit.org/attachment.cgi?id=350753&action=review




--- Comment #25 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 350753
  --> https://bugs.webkit.org/attachment.cgi?id=350753
Patch

LGTM once bots are happy.
A few nits.

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

> Source/WebKit/NetworkProcess/NetworkProcess.messages.in:28
> +    CreateNetworkConnectionToWebProcess(bool isServiceWorkerProcess, struct
WebCore::SecurityOriginData origin)

In the future, we might want to go with service worker instances run in a web
process that also loads web pages, in which case this will break.
This is fine for now.

> Source/WebKit/NetworkProcess/ServiceWorker/WebSWOriginStore.h:44
> +    void importComplete() final;

Can it be made private?

> Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:1637
> +#if ENABLE(SERVICE_WORKER)

This is for non cocoa ports that do not support yet service workers.
Probably fine to have this change although untested.
Looking at the cocoa version, there is some redundant code for IDB and resolve
directories.
The main difference seems cookie handling. A follow-up patch could try to share
more code.

> Source/WebKit/WebProcess/Network/NetworkProcessConnection.h:32
> +#include "WebSWClientConnection.h"

Maybe a forward declaration of WebSWClientConnection will be all that is
needed.