Bug 53150 - initial support for close() in indexeddb backend
Summary: initial support for close() in indexeddb backend
Status: RESOLVED FIXED
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
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-25 17:38 PST by dgrogan
Modified: 2011-04-20 18:01 PDT (History)
4 users (show)

See Also:

Attachments
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
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

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]
Patch
Comment 2 Jeremy Orlow 2011-01-25 18:00:30 PST 
Comment on attachment 80153 [details]
Patch

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

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]
Patch
Comment 7 Jeremy Orlow 2011-01-26 18:09:44 PST 
Comment on attachment 80283 [details]
Patch

r=me