IndexedDB: Support NO_DUPLICATE cursors on LevelDB back-end

• Component: New Bugs
• Status: RESOLVED FIXED

[Hans Wennborg]

IndexedDB: Support NO_DUPLICATE cursors on LevelDB back-end

[Hans Wennborg]

Created attachment 94958 [details]
Patch

[David Grogan]

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?

[Hans Wennborg]

(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.

[Tony Gentilcore]

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.

[Hans Wennborg]

(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.

[Hans Wennborg]

Committed r87491: <http://trac.webkit.org/changeset/87491>