[webkit-reviews] review granted: [Bug 110600] webdatabase: Need a method for reliable multi-process quota management. : [Attachment 191361] the updated fix.

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Mark Lam <mark.lam at apple.com>'s
request for review:
Bug 110600: webdatabase: Need a method for reliable multi-process quota
management.
https://bugs.webkit.org/show_bug.cgi?id=110600

Attachment 191361: the updated fix.
https://bugs.webkit.org/attachment.cgi?id=191361&action=review

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=191361&action=review


r=me

Please address comments before landing.

It looks like this patch addresses Adam's request for an #if USE(FILE_LOCK).

> Source/WebCore/Modules/webdatabase/DatabaseTracker.cpp:287
> +    if (diskUsage > quota)
> +	   return databaseFileSize;

Would be nice to fix SQLiteDatabase::setMaximumSize() in a follow-up patch, to
fix the bug you observed: that setting the size to less than the page size can
be interpreted as an infinite size.

> Source/WebCore/Modules/webdatabase/DatabaseTracker.cpp:659
> +    RefPtr<OriginLock> lock = m_originLockMap.get(databaseIdentifier);

add(), please.

> Source/WebCore/Modules/webdatabase/DatabaseTracker.cpp:667
> +	   // The originLockMap is accessed from multiple DatabaseThreads since

> +	   // different script contexts can be writing to different databases
from
> +	   // the same origin. Hence, the databaseIdentifier needs to be an
> +	   // isolated copy.

I would say, "An isolated copy gives us a value whose refcounting is
thread-safe, since our copy is guarded by a mutex."

> Source/WebCore/Modules/webdatabase/DatabaseTracker.cpp:692
> +    lock.clear();

Instead of clear, just don't put the value in a variable. In fact, we should
just call remove().