[webkit-reviews] review denied: [Bug 110600] webdatabase: Need a method for reliable multi-process quota management. : [Attachment 191194] work in progress 4: one more time to test the ews bots.

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has denied  review:
Bug 110600: webdatabase: Need a method for reliable multi-process quota
management.
https://bugs.webkit.org/show_bug.cgi?id=110600

Attachment 191194: work in progress 4: one more time to test the ews bots.
https://bugs.webkit.org/attachment.cgi?id=191194&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=191194&action=review


I'm not very happy with this design.  In the Chromium port, WebKit never talks
directly to the file system.  As a first step, we should put this file-system
dependent code behind an ifdef that makes it clear that not all port support
FileLock.

Rather than creating dummy "lock" classes that do nothing, we should guard the
uses of the lock with the ifdef to make it clear to people reading the code
that this kind of lock doesn't exist in every configuration.

Really, I think we should use a different design that doesn't involve WebKit
talking directly to the file system, but that might be more than you want to
bite off in this patch.

> Source/WTF/wtf/FileLockUnix.cpp:41
> +static int openLockFile(const char* fileName)
> +{
> +    return open(fileName, O_RDWR | O_CREAT, S_IRUSR | S_IWUSR);
> +}

I'm not sure OS(DARWIN) is the right compile-guard for this code.  chromium-mac
defines OS(DARWIN) on Mac OS X, but Chromium certainly doesn't want to touch
the file system from WebKit.  Perhaps we should have a USE(FILE_LOCK) define
that's set for PLATFORM(MAC)?