Summary: | IndexedDB: Implement IDBTransaction internal active flag | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Joshua Bell <jsbell> | ||||||||
Component: | WebCore Misc. | Assignee: | Joshua Bell <jsbell> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | abarth, alecflett, dgrogan, haraken, japhet, jochen, jsbell, tony, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | 90089 | ||||||||||
Bug Blocks: | |||||||||||
Attachments: |
|
Description
Joshua Bell
2012-06-18 14:09:43 PDT
A plausible test for this looks like: trans = db.transaction('store'); trans.oncomplete = finishJSTest; var done = false; function doRequest() { if (done) return; trans.objectStore('store').get('key').onsuccess = doRequest; }; setTimeout(function () { evalAndExpectException("trans.objectStore('store').get('key')", "IDBDatabaseException.TRANSACTION_INACTIVE_ERR", "'TransactionInactiveError'"); done = true; }, 0); In other words, show Looks like I'll have to do this as part of webkit.org/b/89377 Also, don't abort empty transactions - commit them instead. (May require plumbing for Chromium port.) Created attachment 149646 [details]
Patch
alecflett@ - please take a look Landing sequence will be: (1) Trivial patch with WebKit::WebIDBTransaction::commit() (2) Chromium patch for plumbing through commit() and disabling some layout-tests-as-browser-tests that are getting renamed (3) This patch (4) Re-enable the layout-tests-as-browser-tests Forgot to mention in the ChangeLog, but the transaction-complete-XXX tests are renames of the transaction-abort-XXX tests, now that empty transactions commit rather than abort. Comment on attachment 149646 [details]
Patch
This all looks quite reasonable, I wish I had more to add but it seems fine.
Chromium WebKit public API addition is at: http://webkit.org/b/90089 tony@ - can you take a look at your leisure? Various other things need to land before this can be submitted. Comment on attachment 149646 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=149646&action=review I don't understand all these changes, but it seems OK. > Source/WebCore/Modules/indexeddb/IDBPendingTransactionMonitor.cpp:37 > +static ThreadSpecific<Vector<RefPtr<IDBTransaction> > >& transactions() Nit: Maybe use a typedef for Vector<RefPtr<IDBTransaction> > ? Created attachment 149795 [details]
Patch
(In reply to comment #9) > > Nit: Maybe use a typedef for Vector<RefPtr<IDBTransaction> > ? Good idea - done. New patch just adds this and rebases on http://trac.webkit.org/changeset/121366 Waiting on that to roll into Chromium and landing http://codereview.chromium.org/10692017/ before this can land. Created attachment 150014 [details]
Patch for landing
Comment on attachment 150014 [details] Patch for landing Clearing flags on attachment: 150014 Committed r121492: <http://trac.webkit.org/changeset/121492> All reviewed patches have been landed. Closing bug. |