WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
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
Show Obsolete
(5)
View All
Add attachment
proposed patch, testcase, etc.
Alec Flett
Comment 1
2012-11-19 20:46:33 PST
Created
attachment 175127
[details]
Patch
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
Created
attachment 175133
[details]
Patch
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
Created
attachment 175457
[details]
Patch
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
Created
attachment 175466
[details]
Patch
Alec Flett
Comment 16
2012-11-26 15:52:39 PST
Created
attachment 176099
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug