[webkit-reviews] review denied: [Bug 79633] ScriptExecutionContext has too many ifdef ENABLE(SQL_DATABASE) : [Attachment 128975] Patch

[bugzilla-daemon at webkit.org]

MORITA Hajime <morrita at google.com> has denied Adam Barth <abarth at webkit.org>'s
request for review:
Bug 79633: ScriptExecutionContext has too many ifdef ENABLE(SQL_DATABASE)
https://bugs.webkit.org/show_bug.cgi?id=79633

Attachment 128975: Patch
https://bugs.webkit.org/attachment.cgi?id=128975&action=review

------- Additional Comments from MORITA Hajime <morrita at google.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=128975&action=review


> Source/WebCore/history/PageCache.cpp:124
> +	   if (DatabaseContext::from(frame->document())->hasOpenDatabases()) {

It looks this from() call makes the lazy instantiation making less sense.
How about to hide hide this behind a static method?

> Source/WebCore/history/PageCache.cpp:267
> +	   && !DatabaseContext::from(document)->hasOpenDatabases()

Ditto.

> Source/WebCore/loader/FrameLoader.cpp:423
> +	   DatabaseContext::from(doc)->stopDatabases(0);

Ditto on from().

> Source/WebCore/storage/DatabaseContext.cpp:78
> +    ASSERT(isContextThread());

Windows bot is right. We don't have isContextThread() on DatabaseContext.