Bug 53150 - initial support for close() in indexeddb backend
Summary: initial support for close() in indexeddb backend
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Nobody
Depends on:
Reported: 2011-01-25 17:38 PST by dgrogan
Modified: 2011-04-20 18:01 PDT (History)
4 users (show)

See Also:

Patch (2.54 KB, patch)
2011-01-25 17:46 PST, dgrogan
no flags Details | Formatted Diff | Diff
Patch (10.39 KB, patch)
2011-01-26 16:52 PST, dgrogan
no flags Details | Formatted Diff | Diff
Patch (11.07 KB, patch)
2011-01-26 17:54 PST, dgrogan
jorlow: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description dgrogan 2011-01-25 17:38:27 PST
initial support for close() in indexeddb backend
Comment 1 dgrogan 2011-01-25 17:46:26 PST
Created attachment 80153 [details]
Comment 2 Jeremy Orlow 2011-01-25 18:00:30 PST
Comment on attachment 80153 [details]

change the idl and add a test please
Comment 3 dgrogan 2011-01-26 16:52:26 PST
Created attachment 80267 [details]
Comment 4 Jeremy Orlow 2011-01-26 17:17:21 PST
Comment on attachment 80267 [details]

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
Comment 5 Jeremy Orlow 2011-01-26 17:27:05 PST
Oh...the logic does need to be in the frontend.  OK....but still do the layout test and style nits.
Comment 6 dgrogan 2011-01-26 17:54:07 PST
Created attachment 80283 [details]
Comment 7 Jeremy Orlow 2011-01-26 18:09:44 PST
Comment on attachment 80283 [details]