[webkit-reviews] review granted: [Bug 62622] Implement IDBFactory.deleteDatabase : [Attachment 112841] Patch

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has granted jochen at chromium.org's request for
review:
Bug 62622: Implement IDBFactory.deleteDatabase
https://bugs.webkit.org/show_bug.cgi?id=62622

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

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=112841&action=review


Just some style nits.

> Source/WebCore/storage/IDBDatabaseBackendImpl.cpp:293
> +    if (m_runningVersionChangeTransaction ||
!m_pendingSetVersionCalls.isEmpty() || !m_pendingDeleteCalls.isEmpty())

Nit: Is the !m_pendingDeleteCalls.isEmpty() necessary at this point?

> Source/WebCore/storage/IDBDatabaseBackendImpl.cpp:340
> +    // FIXME: Only fire onVersionChange if there the connection isn't in
> +    // the process of closing.
> +    // http://crbug.com/100645

Nit: Would be nice to have webkit bugs here instead of chromium bugs.

> Source/WebCore/storage/IDBDatabaseBackendImpl.cpp:345
> +    // FIXME: Only fire onBlocked if there are open connections after the
> +    // VersionChangeEvents are received, not just set up to fire.
> +    // http://crbug.com/100123

ditto

> LayoutTests/storage/indexeddb/factory-deletedatabase-interactions.html:228
> +var successfullyParsed = true;

Nit: After https://bugs.webkit.org/show_bug.cgi?id=70784 , this is no longer
required.

> LayoutTests/storage/indexeddb/factory-deletedatabase.html:101
> +var successfullyParsed = true;

Ditto.