Bug 195649 - Use a ServiceWorker process per registrable domain
Summary: Use a ServiceWorker process per registrable domain
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-03-12 15:50 PDT by Chris Dumez
Modified: 2019-03-13 13:52 PDT (History)
8 users (show)

See Also:


Attachments
Patch (58.03 KB, patch)
2019-03-12 16:36 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (57.96 KB, patch)
2019-03-13 13:25 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2019-03-12 15:50:57 PDT
Use a ServiceWorker process per registrable domain instead of one per security origin. This is more in line with PSON and avoids launching too many processes.
Comment 1 Chris Dumez 2019-03-12 16:36:46 PDT
Created attachment 364471 [details]
Patch
Comment 2 Build Bot 2019-03-12 16:39:15 PDT
Attachment 364471 [details] did not pass style-queue:


ERROR: Source/WebKit/UIProcess/ServiceWorkerProcessProxy.cpp:55:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 26 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Chris Dumez 2019-03-13 12:18:09 PDT
Comment on attachment 364471 [details]
Patch

ping review?
Comment 4 John Wilander 2019-03-13 12:32:46 PDT
Comment on attachment 364471 [details]
Patch

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

LGTM.

> Source/WebCore/workers/service/server/SWServer.cpp:513
> +    RegistrableDomain registrableDomain(data.scriptURL);

Maybe uniform initialization here?
Comment 5 youenn fablet 2019-03-13 13:11:19 PDT
Comment on attachment 364471 [details]
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.
Comment 6 Chris Dumez 2019-03-13 13:24:17 PDT
Comment on attachment 364471 [details]
Patch

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.

I think you are right but this should probably be dealt with separately.
Comment 7 Chris Dumez 2019-03-13 13:25:09 PDT
Created attachment 364566 [details]
Patch
Comment 8 Build Bot 2019-03-13 13:26:28 PDT
Attachment 364566 [details] did not pass style-queue:


ERROR: Source/WebKit/UIProcess/ServiceWorkerProcessProxy.cpp:55:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 26 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 youenn fablet 2019-03-13 13:30:59 PDT
I filed https://bugs.webkit.org/show_bug.cgi?id=195697 for the singleton issue to not forget about it.
Comment 10 WebKit Commit Bot 2019-03-13 13:51:43 PDT
Comment on attachment 364566 [details]
Patch

Clearing flags on attachment: 364566

Committed r242905: <https://trac.webkit.org/changeset/242905>
Comment 11 WebKit Commit Bot 2019-03-13 13:51:45 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 Radar WebKit Bug Importer 2019-03-13 13:52:24 PDT
<rdar://problem/48861502>