RESOLVED FIXED 102730
IndexedDB: stub out IDBDatabaseBackendInterface::createTransaction
https://bugs.webkit.org/show_bug.cgi?id=102730
Summary IndexedDB: stub out IDBDatabaseBackendInterface::createTransaction
Alec Flett
Reported 2012-11-19 16:09:16 PST
Part of transaction id refactor. Create a version that takes an int64_t transaction id.
Attachments
Patch (16.67 KB, patch)
2012-11-19 20:46 PST, Alec Flett
no flags
Patch (17.92 KB, patch)
2012-11-19 21:23 PST, Alec Flett
no flags
Patch (18.06 KB, patch)
2012-11-21 09:11 PST, Alec Flett
no flags
Patch (18.06 KB, patch)
2012-11-21 09:46 PST, Alec Flett
no flags
Patch (17.99 KB, patch)
2012-11-26 15:52 PST, Alec Flett
no flags
Patch for landing (17.98 KB, patch)
2012-11-27 05:35 PST, Alec Flett
no flags
Alec Flett
Comment 1 2012-11-19 20:46:33 PST
WebKit Review Bot
Comment 2 2012-11-19 20:50:03 PST
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Alec Flett
Comment 3 2012-11-19 21:14:19 PST
abarth@ - mind a simple review of WebKit API stuff and a little boiler plate to add createTransaction() that replaces transaction()?
WebKit Review Bot
Comment 4 2012-11-19 21:19:35 PST
Comment on attachment 175127 [details] Patch Attachment 175127 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14905459
Alec Flett
Comment 5 2012-11-19 21:23:54 PST
Adam Barth
Comment 6 2012-11-19 23:45:04 PST
Comment on attachment 175133 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=175133&action=review > Source/WebCore/ChangeLog:10 > + Stub out and support passing an int64_t-based transaction id > + so the frontend can refer to transactions by id rather than > + a proxy object or a direct pointer reference. Why is that useful? (I feel a bit ignorant.)
Alec Flett
Comment 7 2012-11-20 10:49:53 PST
Comment on attachment 175133 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=175133&action=review >> Source/WebCore/ChangeLog:10 >> + a proxy object or a direct pointer reference. > > Why is that useful? (I feel a bit ignorant.) Sorry this is part of a longer refactoring of pretty much all of the IDB backend to remove extra layers of abstraction in the name of performance and maintainability. I'm going to have 10+ patches in the next few weeks about this..
Joshua Bell
Comment 8 2012-11-20 11:44:58 PST
Comment on attachment 175133 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=175133&action=review lgtm, but only c/o some offline conversations > Source/WebCore/Modules/indexeddb/IDBTransactionBackendImpl.h:92 > + const int64_t m_id; Eventually this will be referenced via a map id => IDBTransactionBackendImpl inside IDBDatabaseBackendImpl. Unused right now, correct?
Alec Flett
Comment 9 2012-11-20 12:20:46 PST
Comment on attachment 175133 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=175133&action=review >> Source/WebCore/Modules/indexeddb/IDBTransactionBackendImpl.h:92 >> + const int64_t m_id; > > Eventually this will be referenced via a map id => IDBTransactionBackendImpl inside IDBDatabaseBackendImpl. Unused right now, correct? Correct.
Alec Flett
Comment 10 2012-11-20 13:49:08 PST
silviapf@ - I think you got the wrong bug there...
Alec Flett
Comment 11 2012-11-20 15:51:19 PST
fishd@ - mind a quick webkit API review? (and like 2 lines of non-boilerplate IndexedDB)
Alec Flett
Comment 12 2012-11-20 17:37:38 PST
Comment on attachment 175133 [details] Patch Had to take a step back - I think the transaction id is going to have to be a string.
Alec Flett
Comment 13 2012-11-21 09:11:02 PST
WebKit Review Bot
Comment 14 2012-11-21 09:42:33 PST
Comment on attachment 175457 [details] Patch Attachment 175457 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14968004
Alec Flett
Comment 15 2012-11-21 09:46:39 PST
Alec Flett
Comment 16 2012-11-26 15:52:39 PST
WebKit Review Bot
Comment 17 2012-11-26 20:36:36 PST
Comment on attachment 176099 [details] Patch Attachment 176099 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14991821
Peter Beverloo (cr-android ews)
Comment 18 2012-11-26 20:57:37 PST
Comment on attachment 176099 [details] Patch Attachment 176099 [details] did not pass cr-android-ews (chromium-android): Output: http://queues.webkit.org/results/15000562
Darin Fisher (:fishd, Google)
Comment 19 2012-11-26 21:18:46 PST
Comment on attachment 176099 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=176099&action=review > Source/WebKit/chromium/public/WebIDBDatabase.h:67 > + // Transfers ownership of the WebIDBTransaction to the caller. nit: No need for this comment now that you are using the create* naming convention. All create* functions in the API have this property.
Alec Flett
Comment 20 2012-11-27 05:35:50 PST
Created attachment 176244 [details] Patch for landing
WebKit Review Bot
Comment 21 2012-11-27 06:11:14 PST
Comment on attachment 176244 [details] Patch for landing Clearing flags on attachment: 176244 Committed r135856: <http://trac.webkit.org/changeset/135856>
WebKit Review Bot
Comment 22 2012-11-27 06:11:19 PST
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.