[webkit-reviews] review canceled: [Bug 177876] Introduce ServiceWorkerOriginStore : [Attachment 324052] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Oct 17 15:28:47 PDT 2017


Chris Dumez <cdumez at apple.com> has canceled Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 177876: Introduce ServiceWorkerOriginStore
https://bugs.webkit.org/show_bug.cgi?id=177876

Attachment 324052: Patch

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




--- Comment #30 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 324052
  --> https://bugs.webkit.org/attachment.cgi?id=324052
Patch

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

>> Source/WebCore/workers/service/ServiceWorkerProvider.h:50
>> +	virtual bool hasRegisteredServiceWorkerForOrigin(const PAL::SessionID&,
const SecurityOrigin&) = 0;
> 
> SessionID is a uint64_t, shouldn't we pass it by value?

Will pass by value.

>> Source/WebCore/workers/service/server/SWServerRegistration.cpp:100
>> +	UNUSED_PARAM(workerID);
> 
> Can we simply remove workerID?

Brady is passing it in the error case so I am passing it in the success case
too. My assumption is that we will need it in a follow-up patch.

>> Source/WebKit/StorageProcess/StorageProcess.cpp:371
>> +	    return std::make_unique<ServiceWorkerOriginStore>(*this,
sessionID);
> 
> Is UniqueRef working with HashMap?

It is but UniqueRef::get() is const-correct so I would've to const_cast in
swOriginStoreForSession() :(

>> Source/WebKit/WebProcess/Storage/ServiceWorkerOriginTableController.h:52
>> +	HashMap<PAL::SessionID, std::unique_ptr<SharedStringHashTable>>
m_serviceWorkerOriginTables;
> 
> The alternative might have been to put the controller as a child of
WebSWClientConnection which is per-session directly.
> This would remove the need for this m_serviceWorkerOriginTables map.
> SetServiceWorkerOriginTable message would probably be in
WebSWClientConnection
> I guess this should be mirrored in StorageProcess as well.

Will look into this.


More information about the webkit-reviews mailing list