[webkit-reviews] review denied: [Bug 53150] initial support for close() in indexeddb backend : [Attachment 80267] Patch

[bugzilla-daemon at webkit.org]

Jeremy Orlow <jorlow at chromium.org> has denied dgrogan at google.com's request for
review:
Bug 53150: initial support for close() in indexeddb backend
https://bugs.webkit.org/show_bug.cgi?id=53150

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

------- Additional Comments from Jeremy Orlow <jorlow at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=80267&action=review

> LayoutTests/storage/indexeddb/transaction-after-close.html:66
> +    evalAndLog("objectStore.put('a', 'b')");

Whenever you do something, you should at least set an onerror handler to be
unexpectedErrorCallback

> LayoutTests/storage/indexeddb/transaction-after-close.html:87
> +    currentTransaction.onabort = done;

It's probably worth doing something on this transaction and verifying its
onsuccess fires

> Source/WebCore/storage/IDBDatabase.cpp:48
> +    : m_backend(backend), m_noNewTransactions(false)

, and such on newline

> Source/WebCore/storage/IDBDatabase.cpp:-129
> -    m_backend->close();

The setVersion logic is going to need to run in the backend, so you still need
to do this.

> Source/WebCore/storage/IDBDatabase.cpp:133
> +    m_noNewTransactions = true;

Right now, we try to do as much of the logic as is possible in the backend.  I
think we should probably move this there especially since we need to plumb
close anyway.

> Source/WebCore/storage/IDBDatabase.h:80
> +    bool m_noNewTransactions;

this should be on the backend object