Bug 151675

Summary:

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

Product:

WebKit

Reporter:

Brady Eidson <beidson>

Component:

WebCore Misc.

Assignee:

Brady Eidson <beidson>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

achristensen, aestes, alecflett, commit-queue, jsbell

Priority:

Version:

Safari 9

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

149117, 150882, 151684

Attachments:

Description	Flags
Patch v1	darin: review+, beidson: commit-queue-
Patch for landing	none

Brady Eidson

Reported 2015-11-30 14:30:16 PST

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.

Attachments
Patch v1 (22.08 KB, patch) 2015-11-30 15:43 PST, Brady Eidson	darin: review+ beidson: commit-queue-	Details Formatted Diff Diff
Patch for landing (21.67 KB, patch) 2015-11-30 16:42 PST, Brady Eidson	no flags	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Brady Eidson

Comment 1 2015-11-30 15:43:13 PST

Created attachment 266291 [details] Patch v1

Darin Adler

Comment 2 2015-11-30 16:00:30 PST

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

Comment 3 2015-11-30 16:39:18 PST

(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

Comment 4 2015-11-30 16:42:06 PST

Created attachment 266300 [details] Patch for landing

WebKit Commit Bot

Comment 5 2015-11-30 17:28:27 PST

Comment on attachment 266300 [details] Patch for landing Clearing flags on attachment: 266300 Committed r192847: <http://trac.webkit.org/changeset/192847>

Note You need to log in before you can comment on or make changes to this bug.