IndexedDB: Indexes should be secondarily sorted on primary key
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 24 Nov 2011 18:10:01 +0000 (18:10 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 24 Nov 2011 18:10:01 +0000 (18:10 +0000)
https://bugs.webkit.org/show_bug.cgi?id=72567

Patch by Joshua Bell <jsbell@chromium.org> on 2011-11-24
Reviewed by Tony Chang.

Source/WebCore:

Implemented by adding the primary key (i.e. unique key in the
object store) to the IndexDataKey (i.e. the composite key used
in the index. Previously, non-unique entries in the index were
stored with a unique (and hidden) sequenceNumber, so ordering was
not predictable by scripts (or per spec). The sequenceNumber
is now deprecated but still present in the LevelDB backing store
to avoid having to do a data migration.

Test: storage/indexeddb/cursor-primary-key-order.html

* storage/IDBLevelDBBackingStore.cpp:
(WebCore::IDBLevelDBBackingStore::putIndexDataForRecord):
* storage/IDBLevelDBCoding.cpp:
(WebCore::IDBLevelDBCoding::compare):
(WebCore::IDBLevelDBCoding::IndexDataKey::decode):
(WebCore::IDBLevelDBCoding::IndexDataKey::encode):
(WebCore::IDBLevelDBCoding::IndexDataKey::compare):
(WebCore::IDBLevelDBCoding::IndexDataKey::primaryKey):
* storage/IDBLevelDBCoding.h:

Source/WebKit/chromium:

Updates the IndexDataKey unit tests to include primary keys. The
deprecated sequenceNumber element of IndexDataKey is still tested,
although no longer used by callers.

* tests/IDBLevelDBCodingTest.cpp:
(IDBLevelDBCoding::TEST):

LayoutTests:

* storage/indexeddb/cursor-primary-key-order-expected.txt: Added.
* storage/indexeddb/cursor-primary-key-order.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@101142 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/storage/indexeddb/cursor-primary-key-order-expected.txt [new file with mode: 0644]
LayoutTests/storage/indexeddb/cursor-primary-key-order.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/storage/IDBLevelDBBackingStore.cpp
Source/WebCore/storage/IDBLevelDBCoding.cpp
Source/WebCore/storage/IDBLevelDBCoding.h
Source/WebKit/chromium/ChangeLog
Source/WebKit/chromium/tests/IDBLevelDBCodingTest.cpp

index dd7784a9a66e096ebde78bb5be92fd9cf4498c8b..46aa4df81bcaf1ee88a70e3d0d9d1047ccc39af3 100644 (file)
@@ -1,3 +1,13 @@
+2011-11-24  Joshua Bell  <jsbell@chromium.org>
+
+        IndexedDB: Indexes should be secondarily sorted on primary key
+        https://bugs.webkit.org/show_bug.cgi?id=72567
+
+        Reviewed by Tony Chang.
+
+        * storage/indexeddb/cursor-primary-key-order-expected.txt: Added.
+        * storage/indexeddb/cursor-primary-key-order.html: Added.
+
 2011-11-23  Pavel Podivilov  <podivilov@chromium.org>
 
         Web Inspector: add integration test for compiler source maps.
diff --git a/LayoutTests/storage/indexeddb/cursor-primary-key-order-expected.txt b/LayoutTests/storage/indexeddb/cursor-primary-key-order-expected.txt
new file mode 100644 (file)
index 0000000..ce26e9f
--- /dev/null
@@ -0,0 +1,97 @@
+Test IndexedDB primary key 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
+
+openRequest = indexedDB.open('cursor-primary-key-order')
+db = openRequest.result
+versionChangeRequest = db.setVersion('1')
+store = db.createObjectStore('store')
+index = store.createIndex('index', 'indexKey')
+
+populating store...
+trans = db.transaction('store', IDBTransaction.READ_WRITE)
+store = trans.objectStore('store');
+store.put({"indexKey":0,"count":0}, 'c')
+store.put({"indexKey":0,"count":1}, 'b')
+store.put({"indexKey":0,"count":2}, 'a')
+store.put({"indexKey":0,"count":3}, 'C')
+store.put({"indexKey":0,"count":4}, 'B')
+store.put({"indexKey":0,"count":5}, 'A')
+store.put({"indexKey":0,"count":6}, '2')
+store.put({"indexKey":0,"count":7}, '1')
+store.put({"indexKey":0,"count":8}, '0')
+store.put({"indexKey":0,"count":9}, Infinity)
+store.put({"indexKey":0,"count":10}, 2)
+store.put({"indexKey":0,"count":11}, 1)
+store.put({"indexKey":0,"count":12}, 0)
+store.put({"indexKey":0,"count":13}, -1)
+store.put({"indexKey":0,"count":14}, -2)
+store.put({"indexKey":0,"count":15}, -Infinity)
+
+iterating cursor...
+trans = db.transaction('store', IDBTransaction.READ_ONLY)
+store = trans.objectStore('store');
+index = store.index('index');
+cursorRequest = index.openCursor()
+count = 0
+cursor = cursorRequest.result
+PASS cursor.key is 0
+PASS cursor.primaryKey is -Infinity
+cursor = cursorRequest.result
+PASS cursor.key is 0
+PASS cursor.primaryKey is -2
+cursor = cursorRequest.result
+PASS cursor.key is 0
+PASS cursor.primaryKey is -1
+cursor = cursorRequest.result
+PASS cursor.key is 0
+PASS cursor.primaryKey is 0
+cursor = cursorRequest.result
+PASS cursor.key is 0
+PASS cursor.primaryKey is 1
+cursor = cursorRequest.result
+PASS cursor.key is 0
+PASS cursor.primaryKey is 2
+cursor = cursorRequest.result
+PASS cursor.key is 0
+PASS cursor.primaryKey is Infinity
+cursor = cursorRequest.result
+PASS cursor.key is 0
+PASS cursor.primaryKey is '0'
+cursor = cursorRequest.result
+PASS cursor.key is 0
+PASS cursor.primaryKey is '1'
+cursor = cursorRequest.result
+PASS cursor.key is 0
+PASS cursor.primaryKey is '2'
+cursor = cursorRequest.result
+PASS cursor.key is 0
+PASS cursor.primaryKey is 'A'
+cursor = cursorRequest.result
+PASS cursor.key is 0
+PASS cursor.primaryKey is 'B'
+cursor = cursorRequest.result
+PASS cursor.key is 0
+PASS cursor.primaryKey is 'C'
+cursor = cursorRequest.result
+PASS cursor.key is 0
+PASS cursor.primaryKey is 'a'
+cursor = cursorRequest.result
+PASS cursor.key is 0
+PASS cursor.primaryKey is 'b'
+cursor = cursorRequest.result
+PASS cursor.key is 0
+PASS cursor.primaryKey is 'c'
+PASS count === keys.length is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/storage/indexeddb/cursor-primary-key-order.html b/LayoutTests/storage/indexeddb/cursor-primary-key-order.html
new file mode 100644 (file)
index 0000000..fcc7d79
--- /dev/null
@@ -0,0 +1,116 @@
+<html>
+<head>
+<link rel="stylesheet" href="../../fast/js/resources/js-test-style.css">
+<script src="../../fast/js/resources/js-test-pre.js"></script>
+<script src="resources/shared.js"></script>
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<script>
+
+description("Test IndexedDB primary key 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");
+
+    prepareDatabase();
+}
+
+function prepareDatabase()
+{
+    debug("");
+    evalAndLog("openRequest = indexedDB.open('cursor-primary-key-order')");
+    openRequest.onerror = unexpectedErrorCallback;
+    openRequest.onsuccess = function() {
+        evalAndLog("db = openRequest.result");
+        evalAndLog("versionChangeRequest = db.setVersion('1')");
+        versionChangeRequest.onerror = unexpectedErrorCallback;
+        versionChangeRequest.onsuccess = function() {
+            evalAndLog("store = db.createObjectStore('store')");
+            evalAndLog("index = store.createIndex('index', 'indexKey')");
+
+            versionChangeRequest.result.oncomplete = populateStore;
+        };
+    };
+}
+
+window.keys = [
+    "-Infinity",
+    "-2",
+    "-1",
+    "0",
+    "1",
+    "2",
+    "Infinity",
+
+    "'0'",
+    "'1'",
+    "'2'",
+    "'A'",
+    "'B'",
+    "'C'",
+    "'a'",
+    "'b'",
+    "'c'"
+];
+
+
+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;
+    var count = 0;
+    var indexKey = 0;
+    var keys = window.keys.slice();
+    keys.reverse();
+    keys.forEach(function(key) {
+        var value = { indexKey: indexKey, count: count++ };
+        evalAndLog("store.put(" + JSON.stringify(value) + ", " + key + ")");
+    });
+    trans.oncomplete = checkStore;
+}
+
+function checkStore()
+{
+    debug("");
+    debug("iterating cursor...");
+    evalAndLog("trans = db.transaction('store', IDBTransaction.READ_ONLY)");
+    evalAndLog("store = trans.objectStore('store');");
+    evalAndLog("index = store.index('index');");
+    trans.onerror = unexpectedErrorCallback;
+    trans.onabort = unexpectedAbortCallback;
+    cursorRequest = evalAndLog("cursorRequest = index.openCursor()");
+    evalAndLog("count = 0");
+    var indexKey = 0;
+    cursorRequest.onerror = unexpectedErrorCallback;
+    cursorRequest.onsuccess = function() {
+        if (cursorRequest.result) {
+            evalAndLog("cursor = cursorRequest.result");
+            shouldBe("cursor.key", String(indexKey));
+            shouldBe("cursor.primaryKey", window.keys[count++]);
+            cursor.continue();
+        } else {
+            shouldBeTrue("count === keys.length");
+            done();
+        }
+    };
+}
+
+test();
+
+</script>
+</body>
+</html>
index 25f2bd504f653dcfc18e6aa9884ad0033e1662e0..c58f35e1c8655c2de7aebe919f5f1770d72eb71d 100644 (file)
@@ -1,3 +1,30 @@
+2011-11-24  Joshua Bell  <jsbell@chromium.org>
+
+        IndexedDB: Indexes should be secondarily sorted on primary key
+        https://bugs.webkit.org/show_bug.cgi?id=72567
+
+        Reviewed by Tony Chang.
+
+        Implemented by adding the primary key (i.e. unique key in the
+        object store) to the IndexDataKey (i.e. the composite key used
+        in the index. Previously, non-unique entries in the index were
+        stored with a unique (and hidden) sequenceNumber, so ordering was
+        not predictable by scripts (or per spec). The sequenceNumber
+        is now deprecated but still present in the LevelDB backing store
+        to avoid having to do a data migration.
+
+        Test: storage/indexeddb/cursor-primary-key-order.html
+
+        * storage/IDBLevelDBBackingStore.cpp:
+        (WebCore::IDBLevelDBBackingStore::putIndexDataForRecord):
+        * storage/IDBLevelDBCoding.cpp:
+        (WebCore::IDBLevelDBCoding::compare):
+        (WebCore::IDBLevelDBCoding::IndexDataKey::decode):
+        (WebCore::IDBLevelDBCoding::IndexDataKey::encode):
+        (WebCore::IDBLevelDBCoding::IndexDataKey::compare):
+        (WebCore::IDBLevelDBCoding::IndexDataKey::primaryKey):
+        * storage/IDBLevelDBCoding.h:
+
 2011-11-24  Pavel Feldman  <pfeldman@google.com>
 
         Web Inspector: WebInspector.inspectedPageDomain is not calculated for about:blank
index 8cff91ba9d1aced459233d93d034e69a5b219ab8..ec75d0fc06d95decafc459df7ca609aa640c6d4c 100644 (file)
@@ -779,8 +779,8 @@ void IDBLevelDBBackingStore::deleteIndex(int64_t databaseId, int64_t objectStore
         return;
     }
 
-    const Vector<char> indexDataStart = IndexDataKey::encode(databaseId, objectStoreId, indexId, minIDBKey(), 0);
-    const Vector<char> indexDataEnd = IndexDataKey::encode(databaseId, objectStoreId, indexId, maxIDBKey(), 0);
+    const Vector<char> indexDataStart = IndexDataKey::encodeMinKey(databaseId, objectStoreId, indexId);
+    const Vector<char> indexDataEnd = IndexDataKey::encodeMaxKey(databaseId, objectStoreId, indexId);
 
     if (!deleteRange(m_currentTransaction.get(), indexDataStart, indexDataEnd)) {
         LOG_ERROR("Internal Indexed DB error.");
@@ -794,8 +794,7 @@ bool IDBLevelDBBackingStore::putIndexDataForRecord(int64_t databaseId, int64_t o
     const LevelDBRecordIdentifier* levelDBRecordIdentifier = static_cast<const LevelDBRecordIdentifier*>(recordIdentifier);
 
     ASSERT(m_currentTransaction);
-    const int64_t globalSequenceNumber = getNewVersionNumber(m_currentTransaction.get(), databaseId, objectStoreId);
-    const Vector<char> indexDataKey = IndexDataKey::encode(databaseId, objectStoreId, indexId, key, globalSequenceNumber);
+    const Vector<char> indexDataKey = IndexDataKey::encode(databaseId, objectStoreId, indexId, encodeIDBKey(key), levelDBRecordIdentifier->primaryKey());
 
     Vector<char> data;
     data.append(encodeVarInt(levelDBRecordIdentifier->version()));
@@ -856,7 +855,7 @@ static bool findKeyInIndex(LevelDBTransaction* transaction, int64_t databaseId,
 {
     ASSERT(foundEncodedPrimaryKey.isEmpty());
 
-    const Vector<char> leveldbKey = IndexDataKey::encode(databaseId, objectStoreId, indexId, key, 0);
+    const Vector<char> leveldbKey = IndexDataKey::encode(databaseId, objectStoreId, indexId, key);
     OwnPtr<LevelDBIterator> it = transaction->createIterator();
     it->seek(leveldbKey);
 
@@ -1293,15 +1292,15 @@ PassRefPtr<IDBBackingStore::Cursor> IDBLevelDBBackingStore::openIndexKeyCursor(i
     cursorOptions.unique = (direction == IDBCursor::NEXT_NO_DUPLICATE || direction == IDBCursor::PREV_NO_DUPLICATE);
 
     if (!lowerBound) {
-        cursorOptions.lowKey = IndexDataKey::encode(databaseId, objectStoreId, indexId, minIDBKey(), 0);
+        cursorOptions.lowKey = IndexDataKey::encodeMinKey(databaseId, objectStoreId, indexId);
         cursorOptions.lowOpen = false; // Included.
     } else {
-        cursorOptions.lowKey = IndexDataKey::encode(databaseId, objectStoreId, indexId, *range->lower(), 0);
+        cursorOptions.lowKey = IndexDataKey::encode(databaseId, objectStoreId, indexId, *range->lower());
         cursorOptions.lowOpen = range->lowerOpen();
     }
 
     if (!upperBound) {
-        cursorOptions.highKey = IndexDataKey::encode(databaseId, objectStoreId, indexId, maxIDBKey(), 0);
+        cursorOptions.highKey = IndexDataKey::encodeMaxKey(databaseId, objectStoreId, indexId);
         cursorOptions.highOpen = false; // Included.
 
         if (!cursorOptions.forward) { // We need a key that exists.
@@ -1310,7 +1309,7 @@ PassRefPtr<IDBBackingStore::Cursor> IDBLevelDBBackingStore::openIndexKeyCursor(i
             cursorOptions.highOpen = false;
         }
     } else {
-        cursorOptions.highKey = IndexDataKey::encode(databaseId, objectStoreId, indexId, *range->upper(), 0);
+        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.
             return 0;
         cursorOptions.highOpen = range->upperOpen();
@@ -1333,15 +1332,15 @@ PassRefPtr<IDBBackingStore::Cursor> IDBLevelDBBackingStore::openIndexCursor(int6
     cursorOptions.unique = (direction == IDBCursor::NEXT_NO_DUPLICATE || direction == IDBCursor::PREV_NO_DUPLICATE);
 
     if (!lowerBound) {
-        cursorOptions.lowKey = IndexDataKey::encode(databaseId, objectStoreId, indexId, minIDBKey(), 0);
+        cursorOptions.lowKey = IndexDataKey::encodeMinKey(databaseId, objectStoreId, indexId);
         cursorOptions.lowOpen = false; // Included.
     } else {
-        cursorOptions.lowKey = IndexDataKey::encode(databaseId, objectStoreId, indexId, *range->lower(), 0);
+        cursorOptions.lowKey = IndexDataKey::encode(databaseId, objectStoreId, indexId, *range->lower());
         cursorOptions.lowOpen = range->lowerOpen();
     }
 
     if (!upperBound) {
-        cursorOptions.highKey = IndexDataKey::encode(databaseId, objectStoreId, indexId, maxIDBKey(), 0);
+        cursorOptions.highKey = IndexDataKey::encodeMaxKey(databaseId, objectStoreId, indexId);
         cursorOptions.highOpen = false; // Included.
 
         if (!cursorOptions.forward) { // We need a key that exists.
@@ -1350,7 +1349,7 @@ PassRefPtr<IDBBackingStore::Cursor> IDBLevelDBBackingStore::openIndexCursor(int6
             cursorOptions.highOpen = false;
         }
     } else {
-        cursorOptions.highKey = IndexDataKey::encode(databaseId, objectStoreId, indexId, *range->upper(), 0);
+        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.
             return 0;
         cursorOptions.highOpen = range->upperOpen();
index 20f45ab5f9a4fd54be517011de69f52b734ec726..1ccac1ed96fec50d456882d7858ff92b06733051 100644 (file)
 //
 // Index data:
 //
-//     The prefix is followed by a type byte. The user key is an encoded IDBKey. The sequence number is a variable length integer.
+//     The prefix is followed by a type byte. The index key is an encoded IDBKey. The sequence number is a variable length integer.
+//     The primary key is an encoded IDBKey.
 //
-//     <database id, object store id, index id, user key, sequence number> => "version", user key [IndexDataKey]
+//     <database id, object store id, index id, index key, sequence number, primary key> => "version", primary key [IndexDataKey]
 //
-//     (The sequence number is used to allow two entries with the same user key
-//     in non-unique indexes. The "version" field is used to weed out stale
+//     (The sequence number is obsolete; it was used to allow two entries with
+//     the same user (index) key in non-unique indexes prior to the inclusion of
+//     the primary key in the data. The "version" field is used to weed out stale
 //     index data. Whenever new object store data is inserted, it gets a new
 //     "version" number, and new index data is written with this number. When
 //     the index is used for look-ups, entries are validated against the
@@ -735,8 +737,8 @@ int compare(const LevelDBSlice& a, const LevelDBSlice& b, bool indexKeys)
         ASSERT(ptrA);
         ASSERT(ptrB);
 
-        bool ignoreSequenceNumber = indexKeys;
-        return indexDataKeyA.compare(indexDataKeyB, ignoreSequenceNumber);
+        bool ignoreDuplicates = indexKeys;
+        return indexDataKeyA.compare(indexDataKeyB, ignoreDuplicates);
     }
 
     ASSERT_NOT_REACHED();
@@ -1400,44 +1402,66 @@ const char* IndexDataKey::decode(const char* start, const char* limit, IndexData
     result->m_databaseId = prefix.m_databaseId;
     result->m_objectStoreId = prefix.m_objectStoreId;
     result->m_indexId = prefix.m_indexId;
+    result->m_sequenceNumber = -1;
+    result->m_encodedPrimaryKey = minIDBKey();
+
     p = extractEncodedIDBKey(p, limit, &result->m_encodedUserKey);
     if (!p)
         return 0;
-    if (p == limit) {
-        result->m_sequenceNumber = -1; // FIXME: We should change it so that all keys have a sequence number. Shouldn't need to handle this case.
+
+    // [optional] sequence number
+    if (p == limit)
         return p;
-    }
-    return decodeVarInt(p, limit, result->m_sequenceNumber);
+    p =  decodeVarInt(p, limit, result->m_sequenceNumber);
+    if (!p)
+        return 0;
+
+    // [optional] primary key
+    if (p == limit)
+        return p;
+    p = extractEncodedIDBKey(p, limit, &result->m_encodedPrimaryKey);
+    if (!p)
+        return 0;
+
+    return p;
 }
 
-Vector<char> IndexDataKey::encode(int64_t databaseId, int64_t objectStoreId, int64_t indexId, const Vector<char>& encodedUserKey, int64_t sequenceNumber)
+Vector<char> IndexDataKey::encode(int64_t databaseId, int64_t objectStoreId, int64_t indexId, const Vector<char>& encodedUserKey, const Vector<char>& encodedPrimaryKey, int64_t sequenceNumber)
 {
     KeyPrefix prefix(databaseId, objectStoreId, indexId);
     Vector<char> ret = prefix.encode();
     ret.append(encodedUserKey);
     ret.append(encodeVarInt(sequenceNumber));
+    ret.append(encodedPrimaryKey);
     return ret;
 }
 
-Vector<char> IndexDataKey::encode(int64_t databaseId, int64_t objectStoreId, int64_t indexId, const IDBKey& userKey, int64_t sequenceNumber)
+Vector<char> IndexDataKey::encode(int64_t databaseId, int64_t objectStoreId, int64_t indexId, const IDBKey& userKey)
+{
+    return encode(databaseId, objectStoreId, indexId, encodeIDBKey(userKey), minIDBKey());
+}
+
+Vector<char> IndexDataKey::encodeMinKey(int64_t databaseId, int64_t objectStoreId, int64_t indexId)
 {
-    return encode(databaseId, objectStoreId, indexId, encodeIDBKey(userKey), sequenceNumber);
+    return encode(databaseId, objectStoreId, indexId, minIDBKey(), minIDBKey());
 }
 
-Vector<char> IndexDataKey::encodeMaxKey(int64_t databaseId, int64_t objectStoreId)
+Vector<char> IndexDataKey::encodeMaxKey(int64_t databaseId, int64_t objectStoreId, int64_t indexId)
 {
-    return encode(databaseId, objectStoreId, INT32_MAX, maxIDBKey(), INT64_MAX);
+    return encode(databaseId, objectStoreId, indexId, maxIDBKey(), maxIDBKey(), INT64_MAX);
 }
 
-int IndexDataKey::compare(const IndexDataKey& other, bool ignoreSequenceNumber)
+int IndexDataKey::compare(const IndexDataKey& other, bool ignoreDuplicates)
 {
     ASSERT(m_databaseId >= 0);
     ASSERT(m_objectStoreId >= 0);
     ASSERT(m_indexId >= 0);
     if (int x = compareEncodedIDBKeys(m_encodedUserKey, other.m_encodedUserKey))
         return x;
-    if (ignoreSequenceNumber)
+    if (ignoreDuplicates)
         return 0;
+    if (int x = compareEncodedIDBKeys(m_encodedPrimaryKey, other.m_encodedPrimaryKey))
+        return x;
     return compareInts(m_sequenceNumber, other.m_sequenceNumber);
 }
 
@@ -1466,6 +1490,13 @@ PassRefPtr<IDBKey> IndexDataKey::userKey() const
     return key;
 }
 
+PassRefPtr<IDBKey> IndexDataKey::primaryKey() const
+{
+    RefPtr<IDBKey> key;
+    decodeIDBKey(m_encodedPrimaryKey.begin(), m_encodedPrimaryKey.end(), key);
+    return key;
+}
+
 } // namespace IDBLevelDBCoding
 } // namespace WebCore
 
index 1581fbbbfd79f2fd67ded0f3a2b58ac194a05fde..14c42ea73361b40206dac4d0dfd17640790eccfa 100644 (file)
@@ -261,20 +261,23 @@ class IndexDataKey {
 public:
     IndexDataKey();
     static const char* decode(const char* start, const char* limit, IndexDataKey* result);
-    static Vector<char> encode(int64_t databaseId, int64_t objectStoreId, int64_t indexId, const Vector<char>& encodedUserKey, int64_t sequenceNumber);
-    static Vector<char> encode(int64_t databaseId, int64_t objectStoreId, int64_t indexId, const IDBKey& userKey, int64_t sequenceNumber);
-    static Vector<char> encodeMaxKey(int64_t databaseId, int64_t objectStoreId);
-    int compare(const IndexDataKey& other, bool ignoreSequenceNumber);
+    static Vector<char> encode(int64_t databaseId, int64_t objectStoreId, int64_t indexId, const Vector<char>& encodedUserKey, const Vector<char>& encodedPrimaryKey, int64_t sequenceNumber = 0);
+    static Vector<char> encode(int64_t databaseId, int64_t objectStoreId, int64_t indexId, const IDBKey& userKey);
+    static Vector<char> encodeMinKey(int64_t databaseId, int64_t objectStoreId, int64_t indexId);
+    static Vector<char> encodeMaxKey(int64_t databaseId, int64_t objectStoreId, int64_t indexId);
+    int compare(const IndexDataKey& other, bool ignoreDuplicates);
     int64_t databaseId() const;
     int64_t objectStoreId() const;
     int64_t indexId() const;
     PassRefPtr<IDBKey> userKey() const;
+    PassRefPtr<IDBKey> primaryKey() const;
 
 private:
     int64_t m_databaseId;
     int64_t m_objectStoreId;
     int64_t m_indexId;
     Vector<char> m_encodedUserKey;
+    Vector<char> m_encodedPrimaryKey;
     int64_t m_sequenceNumber;
 };
 
index 3a7206c5bb308b4542a618fae5afdace50b83769..c51bd795deba994e5f0df6c72648ed8935235bde 100644 (file)
@@ -1,3 +1,17 @@
+2011-11-24  Joshua Bell  <jsbell@chromium.org>
+
+        IndexedDB: Indexes should be secondarily sorted on primary key
+        https://bugs.webkit.org/show_bug.cgi?id=72567
+
+        Reviewed by Tony Chang.
+
+        Updates the IndexDataKey unit tests to include primary keys. The
+        deprecated sequenceNumber element of IndexDataKey is still tested,
+        although no longer used by callers.
+
+        * tests/IDBLevelDBCodingTest.cpp:
+        (IDBLevelDBCoding::TEST):
+
 2011-11-23  Greg Billock  <gbillock@google.com>
 
         Add simple implementation for web intents chromium API data classes.
index 7d6159e6a1b1fc4af335b9e8f4602e3e6334d8ce..60331051f32daec2249e4bc94e3069701535d14d 100644 (file)
@@ -483,13 +483,17 @@ TEST(IDBLevelDBCodingTest, ComparisonTest)
     keys.append(ObjectStoreDataKey::encode(1, 1, maxIDBKey()));
     keys.append(ExistsEntryKey::encode(1, 1, minIDBKey()));
     keys.append(ExistsEntryKey::encode(1, 1, maxIDBKey()));
-    keys.append(IndexDataKey::encode(1, 1, 30, minIDBKey(), 0));
-    keys.append(IndexDataKey::encode(1, 1, 30, minIDBKey(), 1));
-    keys.append(IndexDataKey::encode(1, 1, 30, maxIDBKey(), 0));
-    keys.append(IndexDataKey::encode(1, 1, 30, maxIDBKey(), 1));
-    keys.append(IndexDataKey::encode(1, 1, 31, minIDBKey(), 0));
-    keys.append(IndexDataKey::encode(1, 2, 30, minIDBKey(), 0));
-    keys.append(IndexDataKey::encodeMaxKey(1, 2));
+    keys.append(IndexDataKey::encode(1, 1, 30, minIDBKey(), minIDBKey(), 0));
+    keys.append(IndexDataKey::encode(1, 1, 30, minIDBKey(), minIDBKey(), 1));
+    keys.append(IndexDataKey::encode(1, 1, 30, minIDBKey(), maxIDBKey(), 0));
+    keys.append(IndexDataKey::encode(1, 1, 30, minIDBKey(), maxIDBKey(), 1));
+    keys.append(IndexDataKey::encode(1, 1, 30, maxIDBKey(), minIDBKey(), 0));
+    keys.append(IndexDataKey::encode(1, 1, 30, maxIDBKey(), minIDBKey(), 1));
+    keys.append(IndexDataKey::encode(1, 1, 30, maxIDBKey(), maxIDBKey(), 0));
+    keys.append(IndexDataKey::encode(1, 1, 30, maxIDBKey(), maxIDBKey(), 1));
+    keys.append(IndexDataKey::encode(1, 1, 31, minIDBKey(), minIDBKey(), 0));
+    keys.append(IndexDataKey::encode(1, 2, 30, minIDBKey(), minIDBKey(), 0));
+    keys.append(IndexDataKey::encodeMaxKey(1, 2, INT32_MAX));
 
     for (size_t i = 0; i < keys.size(); ++i) {
         const LevelDBSlice keyA(keys[i]);