Bug 180382 - Implement setting of service worker to client in SWServer without going to WebProcess
Summary: Implement setting of service worker to client in SWServer without going to We...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Service Workers (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on:
Blocks: 180261
  Show dependency tree / graph
 
Reported: 2017-12-04 15:41 PST by youenn fablet
Modified: 2017-12-05 11:44 PST (History)
9 users (show)

See Also:

Attachments
Patch (28.20 KB, patch)
2017-12-04 15:56 PST, youenn fablet 		no flags Details | Formatted Diff | Diff
Patch (29.11 KB, patch)
2017-12-04 16:35 PST, youenn fablet 		no flags Details | Formatted Diff | Diff
Patch (28.99 KB, patch)
2017-12-04 20:32 PST, youenn fablet 		no flags Details | Formatted Diff | Diff
Patch (29.11 KB, patch)
2017-12-05 10:21 PST, youenn fablet 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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>