Summary: | IndexedDB: stub out IDBDatabaseBackendInterface::createTransaction | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alec Flett <alecflett> | ||||||||||||||
Component: | WebCore Misc. | Assignee: | Alec Flett <alecflett> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | abarth, dglazkov, dgrogan, fishd, jamesr, jsbell, peter+ews, tkent+wkapi, webkit.review.bot | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Bug Depends on: | |||||||||||||||||
Bug Blocks: | 102732 | ||||||||||||||||
Attachments: |
|
Description
Alec Flett
2012-11-19 16:09:16 PST
Created attachment 175127 [details]
Patch
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. abarth@ - mind a simple review of WebKit API stuff and a little boiler plate to add createTransaction() that replaces transaction()? Comment on attachment 175127 [details] Patch Attachment 175127 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14905459 Created attachment 175133 [details]
Patch
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.) 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.. 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? 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. silviapf@ - I think you got the wrong bug there... fishd@ - mind a quick webkit API review? (and like 2 lines of non-boilerplate IndexedDB) 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.
Created attachment 175457 [details]
Patch
Comment on attachment 175457 [details] Patch Attachment 175457 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14968004 Created attachment 175466 [details]
Patch
Created attachment 176099 [details]
Patch
Comment on attachment 176099 [details] Patch Attachment 176099 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14991821 Comment on attachment 176099 [details] Patch Attachment 176099 [details] did not pass cr-android-ews (chromium-android): Output: http://queues.webkit.org/results/15000562 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. Created attachment 176244 [details]
Patch for landing
Comment on attachment 176244 [details] Patch for landing Clearing flags on attachment: 176244 Committed r135856: <http://trac.webkit.org/changeset/135856> All reviewed patches have been landed. Closing bug. |