[webkit-reviews] review granted: [Bug 180558] Clearing all Website Data should remove service worker registrations on disk : [Attachment 328862] 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 328862: Patch

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




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

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

> Source/WebCore/workers/service/ServiceWorkerRegistrationKey.cpp:86
> +    return scopeOrigin->isSameOriginAs(origin);

This seems unfortunate to create a security origin to check for same origin
which is basically scheme/host/port in our case since we only have https URLs.
Maybe we can use protocolHostAndPortAreEqual instead?

> Source/WebCore/workers/service/server/RegistrationDatabase.cpp:92
> +    return
FileSystem::pathByAppendingComponent(m_databaseDirectory.isolatedCopy(),
databaseFilename());

Why an isolatedCopy() here?
If we are to call this numerous time, should we have it as a member of
RegistrationDatabase?

> Source/WebCore/workers/service/server/RegistrationDatabase.cpp:141
> +    openSQLiteDatabase(databasePath);

Do we need makeScopeExit, it seems there is only one if statement below.

> Source/WebCore/workers/service/server/RegistrationDatabase.cpp:251
> +	  
SQLiteFileSystem::deleteEmptyDatabaseDirectory(m_databaseDirectory.isolatedCopy
());

Why do we have to pass an isolated copy?

> Source/WebCore/workers/service/server/RegistrationDatabase.h:30
> +#include "SecurityOriginData.h"

Is this include needed?