Bug 182301 – Delay service worker process creation until actually needed by SWServer

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

[youenn fablet]

This would allow not creating a service worker process if there is no need for it.

[youenn fablet]

Created attachment 332676 [details]
Patch

[youenn fablet]

Patch tested locally and it waits to create a service worker for a page to match.

[youenn fablet]

Would need API test probably.

[youenn fablet]

Created attachment 332809 [details]
Patch

[EWS Watchlist]

Attachment 332809 [details] did not pass style-queue:


ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:179:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:193:  Missing spaces around /  [whitespace/operators] [3]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:195:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:198:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:203:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
Total errors found: 5 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[youenn fablet]

Created attachment 332813 [details]
Patch

[EWS Watchlist]

Attachment 332813 [details] did not pass style-queue:


ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:734:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:748:  Missing spaces around /  [whitespace/operators] [3]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:750:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:753:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:758:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
Total errors found: 5 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Comment on attachment 332813 [details]
Patch

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

> Source/WebKit/StorageProcess/ServiceWorker/WebSWServerConnection.cpp:210
> +    if (!StorageProcess::singleton().globalServerToContextConnection())

We no longer need this part if we land the patch at Bug 182375 first.

[youenn fablet]

Created attachment 332886 [details]
Patch

[EWS Watchlist]

Attachment 332886 [details] did not pass style-queue:


ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:734:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:748:  Missing spaces around /  [whitespace/operators] [3]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:750:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:753:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:758:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
Total errors found: 5 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Comment on attachment 332813 [details]
Patch

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

> Source/WebCore/workers/service/server/SWServer.h:96
> +        virtual void scheduleJobInServer(ServiceWorkerJobData&&) = 0;

This seems weird. This is called due to IPC iirc. Therefore, I would expect this method to not be virtual and be on WebSWServerConnection instead.
Also, it looks like you kept SWServer::Connection::scheduleJobInServer() in the cpp?

[youenn fablet]

Created attachment 332901 [details]
Patch

[EWS Watchlist]

Attachment 332901 [details] did not pass style-queue:


ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:734:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:748:  Missing spaces around /  [whitespace/operators] [3]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:750:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:753:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:758:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
Total errors found: 5 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Comment on attachment 332901 [details]
Patch

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

r=me with comment.

> Source/WebCore/workers/service/server/SWServer.cpp:219
> +void SWServer::Connection::scheduleJob(ServiceWorkerJobData&& jobData)

I do not think we need this method on SWServer::Connection.

> Source/WebKit/StorageProcess/ServiceWorker/WebSWServerConnection.cpp:210
> +    scheduleJob(WTFMove(jobData));

we can call server().scheduleJob(WTFMove(jobData)); directly, like we do in the other methods. No needs for a scheduleJob on Connection.

[youenn fablet]

(In reply to Chris Dumez from comment #14)
> Comment on attachment 332901 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=332901&action=review
> 
> r=me with comment.
> 
> > Source/WebCore/workers/service/server/SWServer.cpp:219
> > +void SWServer::Connection::scheduleJob(ServiceWorkerJobData&& jobData)
> 
> I do not think we need this method on SWServer::Connection.
> 
> > Source/WebKit/StorageProcess/ServiceWorker/WebSWServerConnection.cpp:210
> > +    scheduleJob(WTFMove(jobData));
> 
> we can call server().scheduleJob(WTFMove(jobData)); directly, like we do in
> the other methods. No needs for a scheduleJob on Connection.

Initially I did that, I kept it there as there is some logging done in SWServer.
I can convert it to release logging in WebKit2 code.

[Chris Dumez]

Comment on attachment 332901 [details]
Patch

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

>>> Source/WebCore/workers/service/server/SWServer.cpp:219
>>> +void SWServer::Connection::scheduleJob(ServiceWorkerJobData&& jobData)
>> 
>> I do not think we need this method on SWServer::Connection.
> 
> Initially I did that, I kept it there as there is some logging done in SWServer.
> I can convert it to release logging in WebKit2 code.

Yes please.

[youenn fablet]

Created attachment 332912 [details]
Patch for landing

[EWS Watchlist]

Attachment 332912 [details] did not pass style-queue:


ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:734:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:748:  Missing spaces around /  [whitespace/operators] [3]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:750:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:753:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:758:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
Total errors found: 5 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Commit Bot]

Comment on attachment 332912 [details]
Patch for landing

Clearing flags on attachment: 332912

Committed r227989: <https://trac.webkit.org/changeset/227989>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/37136277>