[webkit-reviews] review granted: [Bug 180558] Clearing all Website Data should remove service worker registrations on disk : [Attachment 328762] Patch

[bugzilla-daemon at webkit.org]

youenn fablet <youennf at gmail.com> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 180558: Clearing all Website Data should remove service worker
registrations on disk
https://bugs.webkit.org/show_bug.cgi?id=180558

Attachment 328762: Patch

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




--- Comment #3 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 328762
  --> https://bugs.webkit.org/attachment.cgi?id=328762
Patch

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

> Source/WebCore/workers/service/server/SWServer.h:121
>      WEBCORE_EXPORT void clear(const SecurityOrigin&);

I guess a follow-up patch should do the same for clear().

> Source/WebKit/StorageProcess/StorageProcess.cpp:299
> +    RefPtr<CallbackAggregator> callbackAggregator =
CallbackAggregator::create([this, callbackID] {

I got some comments in the past to use auto there, and use copyRef() below as a
way to more explicitly state that a ref is taken.

> Source/WebKit/StorageProcess/StorageProcess.cpp:312
> +	  
idbServer(sessionID).closeAndDeleteDatabasesModifiedSince(modifiedSince,
[callbackAggregator] { });

Could move callbackAggregator.