WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
89379
IndexedDB: Implement IDBTransaction internal active flag
https://bugs.webkit.org/show_bug.cgi?id=89379
Summary
IndexedDB: Implement IDBTransaction internal active flag
Joshua Bell
Reported
2012-06-18 14:09:43 PDT
The IDB spec
http://dvcs.w3.org/hg/IndexedDB/raw-file/tip/Overview.html
says: Transaction lifecycle: "Transactions have an active flag, which determines if new requests can be made against the transaction." "When a transaction is created its active flag is initially set to true." - i.e. right after the db.transaction() call "The implementation must allow requests to be placed against the transaction whenever the active flag is true." "Requests may be placed against a transaction only while that transaction is active. If an attempt is made to place a request against a transaction when that transaction is not active, the implementation must reject the attempt by throwing a DOMException of type TransactionInactiveError." Version changes: "This transaction will be active inside the onupgradeneeded event handler, allowing the creation of new object stores and indexes." Abort: "Otherwise this method sets the transaction's active flag to false..." Transaction creation: "When control is returned to the event loop, the implementation must set the active flag to false." Firing events: ... "Set the active flag of transaction to true." ... "Set the active flag of transaction to false." ... Right now we don't have a notion of non-active transaction - until it is |finished| we allow requests to be placed against it.
Attachments
Patch
(86.26 KB, patch)
2012-06-26 17:40 PDT
,
Joshua Bell
no flags
Details
Formatted Diff
Diff
Patch
(84.93 KB, patch)
2012-06-27 14:29 PDT
,
Joshua Bell
no flags
Details
Formatted Diff
Diff
Patch for landing
(84.88 KB, patch)
2012-06-28 14:22 PDT
,
Joshua Bell
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Joshua Bell
Comment 1
2012-06-18 14:30:15 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
Joshua Bell
Comment 2
2012-06-18 15:27:14 PDT
Looks like I'll have to do this as part of
webkit.org/b/89377
Joshua Bell
Comment 3
2012-06-19 09:38:40 PDT
Also, don't abort empty transactions - commit them instead. (May require plumbing for Chromium port.)
Joshua Bell
Comment 4
2012-06-26 17:40:55 PDT
Created
attachment 149646
[details]
Patch
Joshua Bell
Comment 5
2012-06-26 17:43:57 PDT
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.
Alec Flett
Comment 6
2012-06-27 11:56:02 PDT
Comment on
attachment 149646
[details]
Patch This all looks quite reasonable, I wish I had more to add but it seems fine.
Joshua Bell
Comment 7
2012-06-27 11:57:49 PDT
Chromium WebKit public API addition is at:
http://webkit.org/b/90089
Joshua Bell
Comment 8
2012-06-27 13:09:10 PDT
tony@ - can you take a look at your leisure? Various other things need to land before this can be submitted.
Tony Chang
Comment 9
2012-06-27 14:05:35 PDT
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> > ?
Joshua Bell
Comment 10
2012-06-27 14:29:39 PDT
Created
attachment 149795
[details]
Patch
Joshua Bell
Comment 11
2012-06-27 14:31:44 PDT
(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.
Joshua Bell
Comment 12
2012-06-28 14:22:11 PDT
Created
attachment 150014
[details]
Patch for landing
WebKit Review Bot
Comment 13
2012-06-28 17:38:25 PDT
Comment on
attachment 150014
[details]
Patch for landing Clearing flags on attachment: 150014 Committed
r121492
: <
http://trac.webkit.org/changeset/121492
>
WebKit Review Bot
Comment 14
2012-06-28 17:38:30 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug