Diff
Modified: trunk/LayoutTests/ChangeLog (101647 => 101648)
--- trunk/LayoutTests/ChangeLog 2011-12-01 11:14:58 UTC (rev 101647)
+++ trunk/LayoutTests/ChangeLog 2011-12-01 11:20:58 UTC (rev 101648)
@@ -1,3 +1,16 @@
+2011-11-28 Hans Wennborg <h...@chromium.org>
+
+ IndexedDB: Fix reverse cursor with non-existent upper bound
+ https://bugs.webkit.org/show_bug.cgi?id=73220
+
+ Reviewed by Tony Chang.
+
+ Add test to check that opening a reverse cursor works even when the
+ key specified as the upper bound does not exist.
+
+ * storage/indexeddb/cursor-reverse-bug-expected.txt: Added.
+ * storage/indexeddb/cursor-reverse-bug.html: Added.
+
2011-12-01 Hayato Ito <hay...@chromium.org>
Unreviewed. Chromium rebaselines after r101638.
Added: trunk/LayoutTests/storage/indexeddb/cursor-reverse-bug-expected.txt (0 => 101648)
--- trunk/LayoutTests/storage/indexeddb/cursor-reverse-bug-expected.txt (rev 0)
+++ trunk/LayoutTests/storage/indexeddb/cursor-reverse-bug-expected.txt 2011-12-01 11:20:58 UTC (rev 101648)
@@ -0,0 +1,109 @@
+Test IndexedDB keys ordering and readback from cursors.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+indexedDB = window.indexedDB || window.webkitIndexedDB || window.mozIndexedDB;
+PASS indexedDB == null is false
+IDBCursor = window.IDBCursor || window.webkitIDBCursor;
+PASS IDBCursor == null is false
+IDBTransaction = window.IDBTransaction || window.webkitIDBTransaction
+PASS IDBTransaction == null is false
+IDBKeyRange = window.IDBKeyRange || window.webkitIDBKeyRange
+PASS IDBKeyRange == null is false
+
+openreq = indexedDB.open('cursor-reverse-bug')
+db = openreq.result
+verreq = db.setVersion('1')
+Deleted all object stores.
+store = db.createObjectStore('store')
+store.createIndex('index', '')
+
+populating store...
+trans = db.transaction('store', IDBTransaction.READ_WRITE)
+store = trans.objectStore('store');
+store.put(1, 1)
+store.put(2, 2)
+store.put(3, 3)
+testCursor()
+trans = db.transaction('store', IDBTransaction.READ_ONLY)
+store = trans.objectStore('store');
+index = store.index('index');
+upperBound: 7 open: false expected: 3
+storeReq = store.openCursor(IDBKeyRange.upperBound(test.upperBound, test.open), IDBCursor.PREV)
+indexReq = index.openCursor(IDBKeyRange.upperBound(test.upperBound, test.open), IDBCursor.PREV)
+indexKeyReq = index.openKeyCursor(IDBKeyRange.upperBound(test.upperBound, test.open), IDBCursor.PREV)
+cursor = event.target.result
+PASS cursor.key is test.expected
+PASS cursor.value is test.expected
+PASS cursor.primaryKey is test.expected
+cursor = event.target.result
+PASS cursor.key is test.expected
+PASS cursor.value is test.expected
+PASS cursor.primaryKey is test.expected
+cursor = event.target.result
+PASS cursor.key is test.expected
+PASS cursor.primaryKey is test.expected
+testCursor()
+trans = db.transaction('store', IDBTransaction.READ_ONLY)
+store = trans.objectStore('store');
+index = store.index('index');
+upperBound: 7 open: true expected: 3
+storeReq = store.openCursor(IDBKeyRange.upperBound(test.upperBound, test.open), IDBCursor.PREV)
+indexReq = index.openCursor(IDBKeyRange.upperBound(test.upperBound, test.open), IDBCursor.PREV)
+indexKeyReq = index.openKeyCursor(IDBKeyRange.upperBound(test.upperBound, test.open), IDBCursor.PREV)
+cursor = event.target.result
+PASS cursor.key is test.expected
+PASS cursor.value is test.expected
+PASS cursor.primaryKey is test.expected
+cursor = event.target.result
+PASS cursor.key is test.expected
+PASS cursor.value is test.expected
+PASS cursor.primaryKey is test.expected
+cursor = event.target.result
+PASS cursor.key is test.expected
+PASS cursor.primaryKey is test.expected
+testCursor()
+trans = db.transaction('store', IDBTransaction.READ_ONLY)
+store = trans.objectStore('store');
+index = store.index('index');
+upperBound: 3 open: false expected: 3
+storeReq = store.openCursor(IDBKeyRange.upperBound(test.upperBound, test.open), IDBCursor.PREV)
+indexReq = index.openCursor(IDBKeyRange.upperBound(test.upperBound, test.open), IDBCursor.PREV)
+indexKeyReq = index.openKeyCursor(IDBKeyRange.upperBound(test.upperBound, test.open), IDBCursor.PREV)
+cursor = event.target.result
+PASS cursor.key is test.expected
+PASS cursor.value is test.expected
+PASS cursor.primaryKey is test.expected
+cursor = event.target.result
+PASS cursor.key is test.expected
+PASS cursor.value is test.expected
+PASS cursor.primaryKey is test.expected
+cursor = event.target.result
+PASS cursor.key is test.expected
+PASS cursor.primaryKey is test.expected
+testCursor()
+trans = db.transaction('store', IDBTransaction.READ_ONLY)
+store = trans.objectStore('store');
+index = store.index('index');
+upperBound: 3 open: true expected: 2
+storeReq = store.openCursor(IDBKeyRange.upperBound(test.upperBound, test.open), IDBCursor.PREV)
+indexReq = index.openCursor(IDBKeyRange.upperBound(test.upperBound, test.open), IDBCursor.PREV)
+indexKeyReq = index.openKeyCursor(IDBKeyRange.upperBound(test.upperBound, test.open), IDBCursor.PREV)
+cursor = event.target.result
+PASS cursor.key is test.expected
+PASS cursor.value is test.expected
+PASS cursor.primaryKey is test.expected
+cursor = event.target.result
+PASS cursor.key is test.expected
+PASS cursor.value is test.expected
+PASS cursor.primaryKey is test.expected
+cursor = event.target.result
+PASS cursor.key is test.expected
+PASS cursor.primaryKey is test.expected
+testCursor()
+No more tests.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Property changes on: trunk/LayoutTests/storage/indexeddb/cursor-reverse-bug-expected.txt
___________________________________________________________________
Added: svn:eol-style
Added: trunk/LayoutTests/storage/indexeddb/cursor-reverse-bug.html (0 => 101648)
--- trunk/LayoutTests/storage/indexeddb/cursor-reverse-bug.html (rev 0)
+++ trunk/LayoutTests/storage/indexeddb/cursor-reverse-bug.html 2011-12-01 11:20:58 UTC (rev 101648)
@@ -0,0 +1,120 @@
+<html>
+<head>
+<script src=""
+<script src=""
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<script>
+
+description("Test IndexedDB keys ordering and readback from cursors.");
+if (window.layoutTestController)
+ layoutTestController.waitUntilDone();
+
+function test()
+{
+ evalAndLog("indexedDB = window.indexedDB || window.webkitIndexedDB || window.mozIndexedDB;");
+ shouldBeFalse("indexedDB == null");
+ evalAndLog("IDBCursor = window.IDBCursor || window.webkitIDBCursor;");
+ shouldBeFalse("IDBCursor == null");
+ evalAndLog("IDBTransaction = window.IDBTransaction || window.webkitIDBTransaction");
+ shouldBeFalse("IDBTransaction == null");
+ evalAndLog("IDBKeyRange = window.IDBKeyRange || window.webkitIDBKeyRange");
+ shouldBeFalse("IDBKeyRange == null");
+
+ prepareDatabase();
+}
+
+function prepareDatabase()
+{
+ debug("");
+ evalAndLog("openreq = indexedDB.open('cursor-reverse-bug')");
+ openreq._onerror_ = unexpectedErrorCallback;
+ openreq._onsuccess_ = function() {
+ evalAndLog("db = openreq.result");
+ evalAndLog("verreq = db.setVersion('1')");
+ verreq._onerror_ = unexpectedErrorCallback;
+ verreq._onsuccess_ = function() {
+ deleteAllObjectStores(db);
+ store = evalAndLog("store = db.createObjectStore('store')");
+ evalAndLog("store.createIndex('index', '')");
+ verreq.result._oncomplete_ = populateStore;
+ };
+ };
+}
+
+function populateStore()
+{
+ debug("");
+ debug("populating store...");
+ evalAndLog("trans = db.transaction('store', IDBTransaction.READ_WRITE)");
+ evalAndLog("store = trans.objectStore('store');");
+ trans._onerror_ = unexpectedErrorCallback;
+ trans._onabort_ = unexpectedAbortCallback;
+
+ evalAndLog("store.put(1, 1)");
+ evalAndLog("store.put(2, 2)");
+ evalAndLog("store.put(3, 3)");
+ trans._oncomplete_ = testCursor;
+}
+
+var tests = [
+ {upperBound: 7, open: false, expected: 3},
+ {upperBound: 7, open: true, expected: 3},
+ {upperBound: 3, open: false, expected: 3},
+ {upperBound: 3, open: true, expected: 2}
+ ];
+
+function testCursor()
+{
+ debug("testCursor()");
+
+ if (tests.length === 0) {
+ debug("No more tests.");
+ done();
+ return;
+ }
+
+ test = tests.shift();
+
+ evalAndLog("trans = db.transaction('store', IDBTransaction.READ_ONLY)");
+ trans._onerror_ = unexpectedErrorCallback;
+ trans._onabort_ = unexpectedAbortCallback;
+ trans._oncomplete_ = testCursor;
+ evalAndLog("store = trans.objectStore('store');");
+ evalAndLog("index = store.index('index');");
+
+ var testFunction = function() {
+ evalAndLog("cursor = event.target.result");
+ if (cursor === null) {
+ debug("cursor should not be null");
+ fail();
+ return;
+ }
+
+ shouldBe("cursor.key", "test.expected");
+ if ("value" in cursor)
+ shouldBe("cursor.value", "test.expected");
+ shouldBe("cursor.primaryKey", "test.expected");
+
+ // Let the transaction finish.
+ }
+
+ debug("upperBound: " + test.upperBound + " open: " + test.open + " expected: " + test.expected);
+ storeReq = evalAndLog("storeReq = store.openCursor(IDBKeyRange.upperBound(test.upperBound, test.open), IDBCursor.PREV)");
+ storeReq._onsuccess_ = testFunction;
+
+ indexReq = evalAndLog("indexReq = index.openCursor(IDBKeyRange.upperBound(test.upperBound, test.open), IDBCursor.PREV)");
+ indexReq._onsuccess_ = testFunction;
+
+ indexKeyReq = evalAndLog("indexKeyReq = index.openKeyCursor(IDBKeyRange.upperBound(test.upperBound, test.open), IDBCursor.PREV)");
+ indexKeyReq._onsuccess_ = testFunction;
+}
+
+test();
+
+</script>
+</body>
+</html>
+
Property changes on: trunk/LayoutTests/storage/indexeddb/cursor-reverse-bug.html
___________________________________________________________________
Added: svn:eol-style
Modified: trunk/Source/WebCore/ChangeLog (101647 => 101648)
--- trunk/Source/WebCore/ChangeLog 2011-12-01 11:14:58 UTC (rev 101647)
+++ trunk/Source/WebCore/ChangeLog 2011-12-01 11:20:58 UTC (rev 101648)
@@ -1,3 +1,21 @@
+2011-11-28 Hans Wennborg <h...@chromium.org>
+
+ IndexedDB: Fix reverse cursor with non-existent upper bound
+ https://bugs.webkit.org/show_bug.cgi?id=73220
+
+ Reviewed by Tony Chang.
+
+ The code previously did not properly handle the case where the
+ specified upper bound for a reverse cursor did not exist.
+
+ Test: storage/indexeddb/cursor-reverse-bug.html
+
+ * storage/IDBLevelDBBackingStore.cpp:
+ (WebCore::findGreatestKeyLessThanOrEqual):
+ (WebCore::IDBLevelDBBackingStore::openObjectStoreCursor):
+ (WebCore::IDBLevelDBBackingStore::openIndexKeyCursor):
+ (WebCore::IDBLevelDBBackingStore::openIndexCursor):
+
2011-11-29 Hans Wennborg <h...@chromium.org>
IndexedDB: Cursor pre-fetching
Modified: trunk/Source/WebCore/storage/IDBLevelDBBackingStore.cpp (101647 => 101648)
--- trunk/Source/WebCore/storage/IDBLevelDBBackingStore.cpp 2011-12-01 11:14:58 UTC (rev 101647)
+++ trunk/Source/WebCore/storage/IDBLevelDBBackingStore.cpp 2011-12-01 11:20:58 UTC (rev 101648)
@@ -838,7 +838,7 @@
return m_currentTransaction->put(indexDataKey, data);
}
-static bool findGreatestKeyLessThan(LevelDBTransaction* transaction, const Vector<char>& target, Vector<char>& foundKey)
+static bool findGreatestKeyLessThanOrEqual(LevelDBTransaction* transaction, const Vector<char>& target, Vector<char>& foundKey)
{
OwnPtr<LevelDBIterator> it = transaction->createIterator();
it->seek(target);
@@ -849,14 +849,20 @@
return false;
}
- while (compareIndexKeys(it->key(), target) >= 0) {
+ while (compareIndexKeys(it->key(), target) > 0) {
it->prev();
if (!it->isValid())
return false;
}
- foundKey.clear();
- foundKey.append(it->key().begin(), it->key().end() - it->key().begin());
+ do {
+ foundKey.clear();
+ foundKey.append(it->key().begin(), it->key().end() - it->key().begin());
+
+ // There can be several index keys that compare equal. We want the last one.
+ it->next();
+ } while (it->isValid() && !compareIndexKeys(it->key(), target));
+
return true;
}
@@ -1312,23 +1318,6 @@
}
-static bool findLastIndexKeyEqualTo(LevelDBTransaction* transaction, const Vector<char>& target, Vector<char>& foundKey)
-{
- OwnPtr<LevelDBIterator> it = transaction->createIterator();
- it->seek(target);
-
- if (!it->isValid())
- return false;
-
- while (it->isValid() && !compareIndexKeys(it->key(), target)) {
- foundKey.clear();
- foundKey.append(it->key().begin(), it->key().end() - it->key().begin());
- it->next();
- }
-
- return true;
-}
-
PassRefPtr<IDBBackingStore::Cursor> IDBLevelDBBackingStore::openObjectStoreCursor(int64_t databaseId, int64_t objectStoreId, const IDBKeyRange* range, IDBCursor::Direction direction)
{
ASSERT(m_currentTransaction);
@@ -1349,16 +1338,31 @@
if (!upperBound) {
cursorOptions.highKey = ObjectStoreDataKey::encode(databaseId, objectStoreId, maxIDBKey());
- cursorOptions.highOpen = true; // Not included.
- if (!cursorOptions.forward) { // We need a key that exists.
- if (!findGreatestKeyLessThan(m_currentTransaction.get(), cursorOptions.highKey, cursorOptions.highKey))
+ if (cursorOptions.forward)
+ cursorOptions.highOpen = true; // Not included.
+ else {
+ // We need a key that exists.
+ if (!findGreatestKeyLessThanOrEqual(m_currentTransaction.get(), cursorOptions.highKey, cursorOptions.highKey))
return 0;
cursorOptions.highOpen = false;
}
} else {
cursorOptions.highKey = ObjectStoreDataKey::encode(databaseId, objectStoreId, *range->upper());
cursorOptions.highOpen = range->upperOpen();
+
+ if (!cursorOptions.forward) {
+ // For reverse cursors, we need a key that exists.
+ Vector<char> foundHighKey;
+ if (!findGreatestKeyLessThanOrEqual(m_currentTransaction.get(), cursorOptions.highKey, foundHighKey))
+ return 0;
+
+ // If the target key should not be included, but we end up with a smaller key, we should include that.
+ if (cursorOptions.highOpen && compareIndexKeys(foundHighKey, cursorOptions.highKey) < 0)
+ cursorOptions.highOpen = false;
+
+ cursorOptions.highKey = foundHighKey;
+ }
}
RefPtr<ObjectStoreCursorImpl> cursor = ObjectStoreCursorImpl::create(m_currentTransaction.get(), cursorOptions);
@@ -1390,15 +1394,23 @@
cursorOptions.highOpen = false; // Included.
if (!cursorOptions.forward) { // We need a key that exists.
- if (!findGreatestKeyLessThan(m_currentTransaction.get(), cursorOptions.highKey, cursorOptions.highKey))
+ if (!findGreatestKeyLessThanOrEqual(m_currentTransaction.get(), cursorOptions.highKey, cursorOptions.highKey))
return 0;
cursorOptions.highOpen = false;
}
} else {
cursorOptions.highKey = IndexDataKey::encode(databaseId, objectStoreId, indexId, *range->upper());
- if (!findLastIndexKeyEqualTo(m_currentTransaction.get(), cursorOptions.highKey, cursorOptions.highKey)) // Seek to the *last* key in the set of non-unique keys.
+ cursorOptions.highOpen = range->upperOpen();
+
+ Vector<char> foundHighKey;
+ if (!findGreatestKeyLessThanOrEqual(m_currentTransaction.get(), cursorOptions.highKey, foundHighKey)) // Seek to the *last* key in the set of non-unique keys.
return 0;
- cursorOptions.highOpen = range->upperOpen();
+
+ // If the target key should not be included, but we end up with a smaller key, we should include that.
+ if (cursorOptions.highOpen && compareIndexKeys(foundHighKey, cursorOptions.highKey) < 0)
+ cursorOptions.highOpen = false;
+
+ cursorOptions.highKey = foundHighKey;
}
RefPtr<IndexKeyCursorImpl> cursor = IndexKeyCursorImpl::create(m_currentTransaction.get(), cursorOptions);
@@ -1430,15 +1442,23 @@
cursorOptions.highOpen = false; // Included.
if (!cursorOptions.forward) { // We need a key that exists.
- if (!findGreatestKeyLessThan(m_currentTransaction.get(), cursorOptions.highKey, cursorOptions.highKey))
+ if (!findGreatestKeyLessThanOrEqual(m_currentTransaction.get(), cursorOptions.highKey, cursorOptions.highKey))
return 0;
cursorOptions.highOpen = false;
}
} else {
cursorOptions.highKey = IndexDataKey::encode(databaseId, objectStoreId, indexId, *range->upper());
- if (!findLastIndexKeyEqualTo(m_currentTransaction.get(), cursorOptions.highKey, cursorOptions.highKey)) // Seek to the *last* key in the set of non-unique keys.
+ cursorOptions.highOpen = range->upperOpen();
+
+ Vector<char> foundHighKey;
+ if (!findGreatestKeyLessThanOrEqual(m_currentTransaction.get(), cursorOptions.highKey, foundHighKey)) // Seek to the *last* key in the set of non-unique keys.
return 0;
- cursorOptions.highOpen = range->upperOpen();
+
+ // If the target key should not be included, but we end up with a smaller key, we should include that.
+ if (cursorOptions.highOpen && compareIndexKeys(foundHighKey, cursorOptions.highKey) < 0)
+ cursorOptions.highOpen = false;
+
+ cursorOptions.highKey = foundHighKey;
}
RefPtr<IndexCursorImpl> cursor = IndexCursorImpl::create(m_currentTransaction.get(), cursorOptions);
