Bug 61517 - IndexedDB: Support NO_DUPLICATE cursors on LevelDB back-end
Summary: IndexedDB: Support NO_DUPLICATE cursors on LevelDB back-end
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Hans Wennborg
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-05-26 03:55 PDT by Hans Wennborg
Modified: 2011-05-27 03:54 PDT (History)
2 users (show)

See Also:

Attachments
Patch (9.43 KB, patch)
2011-05-26 03:56 PDT, Hans Wennborg 		tonyg: review+
Details | Formatted Diff | Diff
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-05-26 03:55:01 PDT 
IndexedDB: Support NO_DUPLICATE cursors on LevelDB back-end
Comment 1 Hans Wennborg 2011-05-26 03:56:11 PDT 
Created attachment 94958 [details]
Patch
Comment 2 David Grogan 2011-05-26 11:26:04 PDT 
Comment on attachment 94958 [details]
Patch

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

LGTM

> Source/WebCore/ChangeLog:8
> +        This is tested by storage/indexeddb/mozilla/indexes.html

The layout tests aren't run with the leveldb backend on the bots, right?  You have a local change that forces leveldb?
Comment 3 Hans Wennborg 2011-05-27 01:34:01 PDT 
(In reply to comment #2)
> (From update of attachment 94958 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=94958&action=review
> 
> LGTM
> 
> > Source/WebCore/ChangeLog:8
> > +        This is tested by storage/indexeddb/mozilla/indexes.html
> 
> The layout tests aren't run with the leveldb backend on the bots, right?  You have a local change that forces leveldb?
Exactly.
Comment 4 Tony Gentilcore 2011-05-27 03:27:51 PDT 
Comment on attachment 94958 [details]
Patch

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

> Source/WebCore/ChangeLog:5
> +        IndexedDB: Support NO_DUPLICATE cursors on LevelDB back-end

If possible, a link to this part of the spec in the ChangeLog would be nice.

> Source/WebCore/storage/IDBLevelDBBackingStore.cpp:839
> +    CursorImplCommon(LevelDBTransaction* transaction, const Vector<char>& lowKey, bool lowOpen, const Vector<char>& highKey, bool highOpen, bool forward, bool unique)

The bools are out of hand. I don't think you should hold up this change, but a future cleanup might be nice to use enums, an options struct or something else to clean up these signatures.
Comment 5 Hans Wennborg 2011-05-27 03:54:13 PDT 
(In reply to comment #4)
> (From update of attachment 94958 [details])
Thanks for the review!

> View in context: https://bugs.webkit.org/attachment.cgi?id=94958&action=review
> 
> > Source/WebCore/ChangeLog:5
> > +        IndexedDB: Support NO_DUPLICATE cursors on LevelDB back-end
> 
> If possible, a link to this part of the spec in the ChangeLog would be nice.
Done.

> 
> > Source/WebCore/storage/IDBLevelDBBackingStore.cpp:839
> > +    CursorImplCommon(LevelDBTransaction* transaction, const Vector<char>& lowKey, bool lowOpen, const Vector<char>& highKey, bool highOpen, bool forward, bool unique)
> 
> The bools are out of hand. I don't think you should hold up this change, but a future cleanup might be nice to use enums, an options struct or something else to clean up these signatures.
Will do.
Comment 6 Hans Wennborg 2011-05-27 03:54:58 PDT 
Committed r87491: <http://trac.webkit.org/changeset/87491>