[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