[webkit-reviews] review granted: [Bug 195649] Use a ServiceWorker process per registrable domain : [Attachment 364471] Patch

[bugzilla-daemon at webkit.org]

youenn fablet <youennf at gmail.com> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 195649: Use a ServiceWorker process per registrable domain
https://bugs.webkit.org/show_bug.cgi?id=195649

Attachment 364471: Patch

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




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

LGTM.

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

> Source/WebCore/workers/service/server/SWServerToContextConnection.cpp:44
> +    static NeverDestroyed<HashMap<RegistrableDomain,
SWServerToContextConnection*>> connectionsByOrigin;

Not an issue for this patch, but I wonder how this singleton interacts with
multiple NetworkProcess instances.
Ideally, we would not rely on this singleton and instead rely on
NetworkProcess::m_serverToContextConnections.

> Source/WebCore/workers/service/server/SWServerToContextConnection.h:31
>  #include "SecurityOriginData.h"

Can we remove this include?

> Source/WebCore/workers/service/server/SWServerWorker.cpp:55
> +    , m_registrableDomain(scriptURL)

Can we use m_data.scriptURL here instead of scriptURL?
We might want to make scriptURL a URL&& and move it as part of m_data
construction.