Bug 102730 - IndexedDB: stub out IDBDatabaseBackendInterface::createTransaction
Summary: IndexedDB: stub out IDBDatabaseBackendInterface::createTransaction
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alec Flett
URL:
Keywords:
Depends on:
Blocks: 102732
  Show dependency treegraph
 
Reported: 2012-11-19 16:09 PST by Alec Flett
Modified: 2012-11-27 06:11 PST (History)
9 users (show)

See Also:


Attachments
Patch (16.67 KB, patch)
2012-11-19 20:46 PST, Alec Flett
no flags Details | Formatted Diff | Diff
Patch (17.92 KB, patch)
2012-11-19 21:23 PST, Alec Flett
no flags Details | Formatted Diff | Diff
Patch (18.06 KB, patch)
2012-11-21 09:11 PST, Alec Flett
no flags Details | Formatted Diff | Diff
Patch (18.06 KB, patch)
2012-11-21 09:46 PST, Alec Flett
no flags Details | Formatted Diff | Diff
Patch (17.99 KB, patch)
2012-11-26 15:52 PST, Alec Flett
no flags Details | Formatted Diff | Diff
Patch for landing (17.98 KB, patch)
2012-11-27 05:35 PST, Alec Flett
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alec Flett 2012-11-19 16:09:16 PST
Part of transaction id refactor.

Create a version that takes an int64_t transaction id.
Comment 1 Alec Flett 2012-11-19 20:46:33 PST
Created attachment 175127 [details]
Patch
Comment 2 WebKit Review Bot 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.
Comment 3 Alec Flett 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()?
Comment 4 WebKit Review Bot 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
Comment 5 Alec Flett 2012-11-19 21:23:54 PST
Created attachment 175133 [details]
Patch
Comment 6 Adam Barth 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.)
Comment 7 Alec Flett 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..
Comment 8 Joshua Bell 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?
Comment 9 Alec Flett 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.
Comment 10 Alec Flett 2012-11-20 13:49:08 PST
silviapf@ - I think you got the wrong bug there...
Comment 11 Alec Flett 2012-11-20 15:51:19 PST
fishd@ - mind a quick webkit API review? (and like 2 lines of non-boilerplate IndexedDB)
Comment 12 Alec Flett 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.
Comment 13 Alec Flett 2012-11-21 09:11:02 PST
Created attachment 175457 [details]
Patch
Comment 14 WebKit Review Bot 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
Comment 15 Alec Flett 2012-11-21 09:46:39 PST
Created attachment 175466 [details]
Patch
Comment 16 Alec Flett 2012-11-26 15:52:39 PST
Created attachment 176099 [details]
Patch
Comment 17 WebKit Review Bot 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
Comment 18 Peter Beverloo (cr-android ews) 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
Comment 19 Darin Fisher (:fishd, Google) 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.
Comment 20 Alec Flett 2012-11-27 05:35:50 PST
Created attachment 176244 [details]
Patch for landing
Comment 21 WebKit Review Bot 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>
Comment 22 WebKit Review Bot 2012-11-27 06:11:19 PST
All reviewed patches have been landed.  Closing bug.