Commit 67e71686 authored by Joshua Bell's avatar Joshua Bell Committed by Commit Bot

Indexed DB: Use correct range upper limit for index iteration

When iterating a forwards, comparisons are done between each record
found and the range's bound to know when to stop. There's an special
case for reverse cursors where a starting key is needed at the upper
end of the range, so the uppermost key in the range is looked up as
the starting cursor position.

The code to do this for indexes was not guarded by a check for the
cursor direction, though. This was harmless for most forward
iterations as the uppermost actual key would match the upper bound
anyway. But when iterating a cursor over a range in an index, records
can change their index keys and thus appear again in the iteration.
This would lead to the cursor stopping at what was no longer the
actual uppermost key in the range, missing records in the iteration.

Add the missing check, and a WPT to verify this behavior.

(The code dates back to before 2013, so this is not a recent regression.)

Bug: 1091731
Change-Id: I23336ba03d31607607d496fc7e18c28bcf644cf0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2274085
Commit-Queue: Joshua Bell <jsbell@chromium.org>
Reviewed-by: default avatarenne <enne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#783792}
parent 5dd15b25
......@@ -585,19 +585,22 @@ bool IndexCursorOptions(
database_id, object_store_id, index_id, range.upper());
cursor_options->high_open = range.upper_open();
std::string found_high_key;
// Seek to the *last* key in the set of non-unique keys
if (!FindGreatestKeyLessThanOrEqual(transaction, cursor_options->high_key,
&found_high_key, status))
return false;
if (!cursor_options->forward) {
// For reverse cursors, we need a key that exists.
std::string found_high_key;
// Seek to the *last* key in the set of non-unique keys
if (!FindGreatestKeyLessThanOrEqual(transaction, cursor_options->high_key,
&found_high_key, status))
return false;
// If the target key should not be included, but we end up with a smaller
// key, we should include that.
if (cursor_options->high_open &&
CompareIndexKeys(found_high_key, cursor_options->high_key) < 0)
cursor_options->high_open = false;
// If the target key should not be included, but we end up with a smaller
// key, we should include that.
if (cursor_options->high_open &&
CompareIndexKeys(found_high_key, cursor_options->high_key) < 0)
cursor_options->high_open = false;
cursor_options->high_key = found_high_key;
cursor_options->high_key = found_high_key;
}
}
return true;
......
// META: script=support-promises.js
promise_test(async t => {
const db = await createDatabase(t, db => {
const store = db.createObjectStore('store');
store.createIndex('index', 'value');
store.put({value: 1}, 1);
store.put({value: 2}, 2);
store.put({value: 3}, 3);
});
{
// Iterate over all index entries until an upper bound is reached.
// On each record found, increment the value used as the index
// key, which will make it show again up later in the iteration.
const tx = db.transaction('store', 'readwrite');
const range = IDBKeyRange.upperBound(9);
const index = tx.objectStore('store').index('index');
const request = index.openCursor(range);
request.onsuccess = t.step_func(e => {
const cursor = e.target.result;
if (!cursor)
return;
const record = cursor.value;
record.value += 1;
cursor.update(record);
cursor.continue();
});
await promiseForTransaction(t, tx);
}
{
const tx = db.transaction('store', 'readonly');
const results = await promiseForRequest(t, tx.objectStore('store').getAll());
assert_array_equals(
results.map(record => record.value),
[10, 10, 10],
'Values should all be incremented until bound reached');
}
}, 'Index cursor - indexed values updated during iteration');
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment