Modern IDB: "prevunique" cursors should point at the lowest primary key that matches, not the highest

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Brady Eidson]

Modern IDB: "prevunique" cursors should point at the lowest primary key that matches, not the highest.

This is so that "nextunique" and "prevunique" both point at the same primary key record for an index entry.

[Brady Eidson]

Created attachment 266291 [details]
Patch v1

[Darin Adler]

Comment on attachment 266291 [details]
Patch v1

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

> Source/WebCore/Modules/indexeddb/server/IndexValueEntry.cpp:222
> -    auto iterator = std::set<IDBKeyData>::reverse_iterator(m_orderedKeys->upper_bound(key));
> +
> +    std::set<IDBKeyData>::reverse_iterator iterator = std::set<IDBKeyData>::reverse_iterator(m_orderedKeys->upper_bound(key));

Why the change here? Is it useful to repeat std::set<IDBKeyData>::reverse_iterator twice?

> Source/WebCore/Modules/indexeddb/server/IndexValueEntry.h:38
> +enum class CursorDuplicity;

Is this kind of forward declaration of an enum class sufficient? If so, that’s great and I was not aware until now.

[Brady Eidson]

(In reply to comment #2)
> Comment on attachment 266291 [details]
> Patch v1
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=266291&action=review
> 
> > Source/WebCore/Modules/indexeddb/server/IndexValueEntry.cpp:222
> > -    auto iterator = std::set<IDBKeyData>::reverse_iterator(m_orderedKeys->upper_bound(key));
> > +
> > +    std::set<IDBKeyData>::reverse_iterator iterator = std::set<IDBKeyData>::reverse_iterator(m_orderedKeys->upper_bound(key));
> 
> Why the change here? Is it useful to repeat
> std::set<IDBKeyData>::reverse_iterator twice?

The patch was more complicated originally and required a fully-typed local variable to play with.

Then it got easier again and I forgot to make it auto.

Will-do!

> 
> > Source/WebCore/Modules/indexeddb/server/IndexValueEntry.h:38
> > +enum class CursorDuplicity;
> 
> Is this kind of forward declaration of an enum class sufficient? If so,
> that’s great and I was not aware until now.

Yes, it is! It's one of the understated great things about enum class!

[Brady Eidson]

Created attachment 266300 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 266300 [details]
Patch for landing

Clearing flags on attachment: 266300

Committed r192847: <http://trac.webkit.org/changeset/192847>