Bug 180382

Summary: Implement setting of service worker to client in SWServer without going to WebProcess
Product: WebKit Reporter: youenn fablet <youennf>
Component: Service WorkersAssignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: beidson, cdumez, commit-queue, dbates, esprehn+autocc, ews-watchlist, japhet, kangil.han, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 180261    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Description youenn fablet 2017-12-04 15:41:56 PST
Implement setting of service worker to client in SWServer without going to WebProcess
Comment 1 youenn fablet 2017-12-04 15:56:17 PST
Created attachment 328400 [details]
Patch
Comment 2 youenn fablet 2017-12-04 16:35:50 PST
Created attachment 328408 [details]
Patch
Comment 3 youenn fablet 2017-12-04 20:32:56 PST
Created attachment 328433 [details]
Patch
Comment 4 Chris Dumez 2017-12-05 09:06:20 PST
Comment on attachment 328433 [details]
Patch

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

r=me with minor comments.

> Source/WebCore/workers/service/server/SWServer.cpp:587
> +SWServerRegistration* SWServer::registrationFromServiceWorkerIdentifier(ServiceWorkerIdentifier identifier)

It would be good if we had a registration() getter on the SWServerWorker class instead. It would be more reusable this way.

> Source/WebCore/workers/service/server/SWServer.h:148
> +    WEBCORE_EXPORT void registerServiceWorkerClient(ClientOrigin&&, ServiceWorkerClientIdentifier, ServiceWorkerClientData&&, const std::optional<ServiceWorkerIdentifier>&);

I think we should provide the name for the last parameter as it is non-obvious to me.
Comment 5 youenn fablet 2017-12-05 10:20:14 PST
> > Source/WebCore/workers/service/server/SWServer.cpp:587
> > +SWServerRegistration* SWServer::registrationFromServiceWorkerIdentifier(ServiceWorkerIdentifier identifier)
> 
> It would be good if we had a registration() getter on the SWServerWorker
> class instead. It would be more reusable this way.

Agreed, let's see how we can do that in future refactoring.

> > Source/WebCore/workers/service/server/SWServer.h:148
> > +    WEBCORE_EXPORT void registerServiceWorkerClient(ClientOrigin&&, ServiceWorkerClientIdentifier, ServiceWorkerClientData&&, const std::optional<ServiceWorkerIdentifier>&);
> 
> I think we should provide the name for the last parameter as it is
> non-obvious to me.

OK
Comment 6 youenn fablet 2017-12-05 10:21:46 PST
Created attachment 328463 [details]
Patch
Comment 7 WebKit Commit Bot 2017-12-05 10:42:09 PST
Comment on attachment 328463 [details]
Patch

Clearing flags on attachment: 328463

Committed r225526: <https://trac.webkit.org/changeset/225526>
Comment 8 WebKit Commit Bot 2017-12-05 10:42:10 PST
All reviewed patches have been landed.  Closing bug.
Comment 9 Radar WebKit Bug Importer 2017-12-05 10:45:27 PST
<rdar://problem/35858689>