Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(131)

Issue 1188343002: IndexedDB: Disallow continuePrimaryKey on unique cursors (Closed)

Created:
5 years, 6 months ago by jsbell
Modified:
5 years, 6 months ago
Reviewers:
cmumford
CC:
blink-reviews, jsbell+idb_chromium.org, dgrogan, cmumford
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

IndexedDB: Disallow continuePrimaryKey on unique cursors The experimental 'continuePrimaryKey()' method had undefined behavior when the cursor was a 'unique' cursor (skipping over duplicate primary keys). After some spec discussion[1], it seems like forbidding the use of the method on such cursors is the safest approach. [1] https://github.com/w3c/IndexedDB/issues/14 R=cmumford@chromium.org BUG=497454 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=197309

Patch Set 1 #

Total comments: 2

Patch Set 2 : Make assertion message more specific #

Unified diffs Side-by-side diffs Delta from patch set Stats (+63 lines, -2 lines) Patch
M LayoutTests/storage/indexeddb/idbcursor_continueprimarykey.html View 1 3 chunks +59 lines, -2 lines 0 comments Download
M Source/modules/indexeddb/IDBCursor.cpp View 1 chunk +4 lines, -0 lines 0 comments Download

Messages

Total messages: 8 (2 generated)
jsbell
cmumford@ - please take a look?
5 years, 6 months ago (2015-06-17 21:35:34 UTC) #1
cmumford
lgtm lgtm w/minor comment. https://codereview.chromium.org/1188343002/diff/1/LayoutTests/storage/indexeddb/idbcursor_continueprimarykey.html File LayoutTests/storage/indexeddb/idbcursor_continueprimarykey.html (right): https://codereview.chromium.org/1188343002/diff/1/LayoutTests/storage/indexeddb/idbcursor_continueprimarykey.html#newcode77 LayoutTests/storage/indexeddb/idbcursor_continueprimarykey.html:77: 'key should match'); differentiate message ...
5 years, 6 months ago (2015-06-17 21:58:32 UTC) #2
cmumford
lgtm lgtm w/minor comment.
5 years, 6 months ago (2015-06-17 21:58:34 UTC) #3
jsbell
https://codereview.chromium.org/1188343002/diff/1/LayoutTests/storage/indexeddb/idbcursor_continueprimarykey.html File LayoutTests/storage/indexeddb/idbcursor_continueprimarykey.html (right): https://codereview.chromium.org/1188343002/diff/1/LayoutTests/storage/indexeddb/idbcursor_continueprimarykey.html#newcode77 LayoutTests/storage/indexeddb/idbcursor_continueprimarykey.html:77: 'key should match'); On 2015/06/17 21:58:32, cmumford wrote: > ...
5 years, 6 months ago (2015-06-18 00:26:22 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1188343002/20001
5 years, 6 months ago (2015-06-18 00:27:06 UTC) #7
commit-bot: I haz the power
5 years, 6 months ago (2015-06-18 02:51:39 UTC) #8
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=197309

Powered by Google App Engine
This is Rietveld 408576698