Bug 54889 - IndexedDB: Move some SQL code into IDBBackingStore
Summary: IndexedDB: Move some SQL code into IDBBackingStore
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Hans Wennborg
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-21 09:45 PST by Hans Wennborg
Modified: 2011-02-23 07:11 PST (History)
3 users (show)

See Also:

Attachments
Patch (51.49 KB, patch)
2011-02-21 09:56 PST, Hans Wennborg 		no flags Details | Formatted Diff | Diff
Patch (57.08 KB, patch)
2011-02-22 07:18 PST, Hans Wennborg 		jorlow: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Hans Wennborg 2011-02-21 09:45:52 PST 
IndexedDB: Move some SQL code into IDBBackingStore
Comment 1 Hans Wennborg 2011-02-21 09:56:49 PST 
Created attachment 83176 [details]
Patch
Comment 2 Hans Wennborg 2011-02-21 10:00:22 PST 
Note: this patch is based on https://bugs.webkit.org/show_bug.cgi?id=54864, so it will not apply until that has landed. I expect the ews bots to fail.

I figured it might be a good idea to get it reviewed early, though.

It's a big patch, but most of it is just code being moved around. The main thing for a reviewer to look at would be the function names and signatures, I think.
Comment 3 Hans Wennborg 2011-02-22 07:18:52 PST 
Created attachment 83309 [details]
Patch
Comment 4 Jeremy Orlow 2011-02-22 16:20:59 PST 
Comment on attachment 83309 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=83309&action=review

r=me

> Source/WebCore/storage/IDBBackingStore.cpp:465
> +double IDBBackingStore::autoIncrementNumber(int64_t objectStoreId)

nextAUtoIncrementNumber maybe?

> Source/WebCore/storage/IDBBackingStore.h:63
> +    void getIndexes(int64_t objectStoreId, Vector<int64_t>& foundIds, Vector<String>& foundNames, Vector<String>& foundKeyPaths, Vector<bool>& foundUniqueFlags);

maybe split out get/create/delete index like you did for object store?

maybe split out all the calls for puts and make it clear you need to manually call all applicable?
Comment 5 Hans Wennborg 2011-02-23 07:09:41 PST 
(In reply to comment #4)
> (From update of attachment 83309 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=83309&action=review
> 
> r=me
> 
> > Source/WebCore/storage/IDBBackingStore.cpp:465
> > +double IDBBackingStore::autoIncrementNumber(int64_t objectStoreId)
> 
> nextAUtoIncrementNumber maybe?
Yes, that's better. Done.

> 
> > Source/WebCore/storage/IDBBackingStore.h:63
> > +    void getIndexes(int64_t objectStoreId, Vector<int64_t>& foundIds, Vector<String>& foundNames, Vector<String>& foundKeyPaths, Vector<bool>& foundUniqueFlags);
> 
> maybe split out get/create/delete index like you did for object store?
Done.

> 
> maybe split out all the calls for puts and make it clear you need to manually call all applicable?
I've grouped the index and the object store stuff together, respectively. I think that makes more sense, but this is easy to change later if we want to.
Comment 6 Hans Wennborg 2011-02-23 07:11:03 PST 
Committed r79445: <http://trac.webkit.org/changeset/79445>