IndexedDB: iteration of cursors skip records if updated or deleted
authorsihui_liu@apple.com <sihui_liu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 30 Oct 2018 17:11:39 +0000 (17:11 +0000)
committersihui_liu@apple.com <sihui_liu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 30 Oct 2018 17:11:39 +0000 (17:11 +0000)
https://bugs.webkit.org/show_bug.cgi?id=190917
<rdar://problem/35250410>

Reviewed by Chris Dumez.

LayoutTests/imported/w3c:

Rebaseline the expectation for test that passes.

* web-platform-tests/IndexedDB/idbcursor-iterating-update-expected.txt:

Source/WebCore:

When object store has changes, we cleared cached records and reset the SQLite statement,
updating the boundary to the next key in cursor's direction. Therefore, the cursor could
jump to the next key after update or deletion.
We should cache those records before changing the statement.

Test: storage/indexeddb/cursor-update-while-iterating.html

* Modules/indexeddb/server/SQLiteIDBCursor.cpp:
(WebCore::IDBServer::SQLiteIDBCursor::objectStoreRecordsChanged):
(WebCore::IDBServer::SQLiteIDBCursor::fetch):
* Modules/indexeddb/server/SQLiteIDBCursor.h:

LayoutTests:

* storage/indexeddb/cursor-update-while-iterating-expected.txt: Added.
* storage/indexeddb/cursor-update-while-iterating.html: Added.
* storage/indexeddb/resources/cursor-update-while-iterating.js: Added.
(prepareDatabase):
(onOpenSuccess.request.onsuccess):
(onOpenSuccess):

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

LayoutTests/ChangeLog
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/IndexedDB/idbcursor-iterating-update-expected.txt
LayoutTests/storage/indexeddb/cursor-update-while-iterating-expected.txt [new file with mode: 0644]
LayoutTests/storage/indexeddb/cursor-update-while-iterating.html [new file with mode: 0644]
LayoutTests/storage/indexeddb/resources/cursor-update-while-iterating.js [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/Modules/indexeddb/server/SQLiteIDBCursor.cpp
Source/WebCore/Modules/indexeddb/server/SQLiteIDBCursor.h

index eac110d..4b5c983 100644 (file)
@@ -1,3 +1,18 @@
+2018-10-30  Sihui Liu  <sihui_liu@apple.com>
+
+        IndexedDB: iteration of cursors skip records if updated or deleted
+        https://bugs.webkit.org/show_bug.cgi?id=190917
+        <rdar://problem/35250410>
+
+        Reviewed by Chris Dumez.
+
+        * storage/indexeddb/cursor-update-while-iterating-expected.txt: Added.
+        * storage/indexeddb/cursor-update-while-iterating.html: Added.
+        * storage/indexeddb/resources/cursor-update-while-iterating.js: Added.
+        (prepareDatabase):
+        (onOpenSuccess.request.onsuccess):
+        (onOpenSuccess):
+
 2018-10-28  Antoine Quint  <graouts@apple.com>
 
         [Web Animations] Implement the update animations and send events procedure
index f2504d1..776c19b 100644 (file)
@@ -1,3 +1,15 @@
+2018-10-30  Sihui Liu  <sihui_liu@apple.com>
+
+        IndexedDB: iteration of cursors skip records if updated or deleted
+        https://bugs.webkit.org/show_bug.cgi?id=190917
+        <rdar://problem/35250410>
+
+        Reviewed by Chris Dumez.
+
+        Rebaseline the expectation for test that passes.
+
+        * web-platform-tests/IndexedDB/idbcursor-iterating-update-expected.txt:
+
 2018-10-28  Antoine Quint  <graouts@apple.com>
 
         [Web Animations] Implement the update animations and send events procedure
index 7e8379d..9a7ace7 100644 (file)
@@ -1,8 +1,4 @@
-CONSOLE MESSAGE: line 2659: Error: assert_equals: Cursor should iterate over 4 records expected 4 but got 2
-CONSOLE MESSAGE: line 2659: Error: assert_equals: Cursor should iterate over 4 records expected 4 but got 2
 
-Harness Error (FAIL), message = Error: assert_equals: Cursor should iterate over 4 records expected 4 but got 2
-
-TIMEOUT Calling cursor => cursor.update({}) doesn't affect index iteration Test timed out
-TIMEOUT Calling cursor => cursor.delete() doesn't affect index iteration Test timed out
+PASS Calling cursor => cursor.update({}) doesn't affect index iteration 
+PASS Calling cursor => cursor.delete() doesn't affect index iteration 
 
diff --git a/LayoutTests/storage/indexeddb/cursor-update-while-iterating-expected.txt b/LayoutTests/storage/indexeddb/cursor-update-while-iterating-expected.txt
new file mode 100644 (file)
index 0000000..a8a96e9
--- /dev/null
@@ -0,0 +1,47 @@
+Test IndexedDB's cursor iteration with update and deletion.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+indexedDB = self.indexedDB || self.webkitIndexedDB || self.mozIndexedDB || self.msIndexedDB || self.OIndexedDB;
+
+indexedDB.deleteDatabase(dbname)
+indexedDB.open(dbname)
+
+prepareDatabase():
+db = event.target.result
+Deleted all object stores.
+objectStore = db.createObjectStore('objectStore', {autoIncrement: true})
+objectStore.createIndex('key', 'key', {unique: false})
+
+onOpenSuccess():
+db = event.target.result
+t = db.transaction('objectStore', 'readwrite')
+objectStore = t.objectStore('objectStore')
+index = objectStore.index('key')
+index.openCursor()
+PASS JSON.stringify(cursor.value) is "{\"key\":\"key1\",\"value\":\"value1\"}"
+Update cursor
+Cursor continues
+
+PASS JSON.stringify(cursor.value) is "{\"key\":\"key1\",\"value\":\"value1\"}"
+Update cursor
+Cursor continues
+
+PASS JSON.stringify(cursor.value) is "{\"key\":\"key1\",\"value\":\"value3\"}"
+Update cursor
+Cursor continues
+
+PASS JSON.stringify(cursor.value) is "{\"key\":\"key2\",\"value\":\"value2\"}"
+Delete cursor
+Cursor continues
+
+PASS JSON.stringify(cursor.value) is "{\"key\":\"key2\",\"value\":\"value4\"}"
+Delete cursor
+Cursor continues
+
+PASS Successfully iterated whole array with cursor updates.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/storage/indexeddb/cursor-update-while-iterating.html b/LayoutTests/storage/indexeddb/cursor-update-while-iterating.html
new file mode 100644 (file)
index 0000000..2698228
--- /dev/null
@@ -0,0 +1,9 @@
+<html>
+<head>
+<script src="../../resources/js-test.js"></script>
+<script src="resources/shared.js"></script>
+</head>
+<body>
+<script src="resources/cursor-update-while-iterating.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/storage/indexeddb/resources/cursor-update-while-iterating.js b/LayoutTests/storage/indexeddb/resources/cursor-update-while-iterating.js
new file mode 100644 (file)
index 0000000..48bd90b
--- /dev/null
@@ -0,0 +1,72 @@
+if (this.importScripts) {
+    importScripts('../../../resources/js-test.js');
+    importScripts('shared.js');
+}
+
+description("Test IndexedDB's cursor iteration with update and deletion.");
+
+indexedDBTest(prepareDatabase, onOpenSuccess);
+
+const objectArray = [
+    { key: "key1", value: "value1" },
+    { key: "key1", value: "value1" },
+    { key: "key1", value: "value3" },
+    { key: "key2", value: "value2" },
+    { key: "key2", value: "value4" }
+];
+
+function populateObjectStore() {
+    for (let object of objectArray)
+        objectStore.add(object).onerror = unexpectedErrorCallback;
+}
+
+function prepareDatabase(event)
+{
+    preamble(event);
+    evalAndLog("db = event.target.result");
+    deleteAllObjectStores(db);
+
+    objectStore = evalAndLog("objectStore = db.createObjectStore('objectStore', {autoIncrement: true})");
+    evalAndLog("objectStore.createIndex('key', 'key', {unique: false})");
+
+    populateObjectStore();
+}
+
+function onOpenSuccess(event)
+{
+    preamble(event);
+    evalAndLog("db = event.target.result");
+    t = evalAndLog("t = db.transaction('objectStore', 'readwrite')");
+    t.oncomplete = () => { finishJSTest(); }
+
+    evalAndLog("objectStore = t.objectStore('objectStore')");
+    evalAndLog("index = objectStore.index('key')");
+    request = evalAndLog("index.openCursor()");
+
+    var n = 0;
+    request.onsuccess = function(event) {
+        cursor = event.target.result;
+        if (cursor) {
+            shouldBeEqualToString("JSON.stringify(cursor.value)", JSON.stringify(objectArray[n++]));
+
+            if (cursor.key == "key1") {
+                debug("Update cursor");
+                const {value} = cursor;
+                cursor.update(value);
+            } else {
+                debug("Delete cursor");
+                cursor.delete();
+            }
+
+            debug("Cursor continues\n");
+            cursor.continue();
+        } else {
+            if (n != objectArray.length)
+                testFailed("Cursor didn't go through whole array.");
+            else 
+                testPassed("Successfully iterated whole array with cursor updates.");
+        }
+    }
+
+    request.onerror = unexpectedErrorCallback;
+}
\ No newline at end of file
index 3c41eb6..c232c6e 100644 (file)
@@ -1,3 +1,23 @@
+2018-10-30  Sihui Liu  <sihui_liu@apple.com>
+
+        IndexedDB: iteration of cursors skip records if updated or deleted
+        https://bugs.webkit.org/show_bug.cgi?id=190917
+        <rdar://problem/35250410>
+
+        Reviewed by Chris Dumez.
+
+        When object store has changes, we cleared cached records and reset the SQLite statement, 
+        updating the boundary to the next key in cursor's direction. Therefore, the cursor could 
+        jump to the next key after update or deletion.
+        We should cache those records before changing the statement.
+
+        Test: storage/indexeddb/cursor-update-while-iterating.html
+
+        * Modules/indexeddb/server/SQLiteIDBCursor.cpp:
+        (WebCore::IDBServer::SQLiteIDBCursor::objectStoreRecordsChanged):
+        (WebCore::IDBServer::SQLiteIDBCursor::fetch):
+        * Modules/indexeddb/server/SQLiteIDBCursor.h:
+
 2018-10-29  Zalan Bujtas  <zalan@apple.com>
 
         [LFC][IFC] Introduce InlineFormattingContextGeometry class
index f20db87..ebfdf61 100644 (file)
@@ -214,32 +214,40 @@ void SQLiteIDBCursor::objectStoreRecordsChanged()
     if (m_statementNeedsReset)
         return;
 
+    ASSERT(!m_fetchedRecords.isEmpty());
+
+    m_currentKeyForUniqueness = m_fetchedRecords.first().record.key;
+
+    if (m_cursorDirection != IndexedDB::CursorDirection::Nextunique && m_cursorDirection != IndexedDB::CursorDirection::Prevunique) {
+        if (!m_fetchedRecords.last().isTerminalRecord())
+            fetch(ShouldFetchForSameKey::Yes);
+
+        while (m_fetchedRecords.last().record.key != m_fetchedRecords.first().record.key)
+            m_fetchedRecords.removeLast();
+    } else
+        m_fetchedRecords.clear();
+
     // If ObjectStore or Index contents changed, we need to reset the statement and bind new parameters to it.
     // This is to pick up any changes that might exist.
     // We also need to throw away any fetched records as they may no longer be valid.
 
     m_statementNeedsReset = true;
-    ASSERT(!m_fetchedRecords.isEmpty());
 
     if (m_cursorDirection == IndexedDB::CursorDirection::Next || m_cursorDirection == IndexedDB::CursorDirection::Nextunique) {
-        m_currentLowerKey = m_fetchedRecords.first().record.key;
+        m_currentLowerKey = m_currentKeyForUniqueness;
         if (!m_keyRange.lowerOpen) {
             m_keyRange.lowerOpen = true;
             m_keyRange.lowerKey = m_currentLowerKey;
             m_statement = nullptr;
         }
     } else {
-        m_currentUpperKey = m_fetchedRecords.first().record.key;
+        m_currentUpperKey = m_currentKeyForUniqueness;
         if (!m_keyRange.upperOpen) {
             m_keyRange.upperOpen = true;
             m_keyRange.upperKey = m_currentUpperKey;
             m_statement = nullptr;
         }
     }
-
-    m_currentKeyForUniqueness = m_fetchedRecords.first().record.key;
-
-    m_fetchedRecords.clear();
 }
 
 void SQLiteIDBCursor::resetAndRebindStatement()
@@ -360,23 +368,25 @@ bool SQLiteIDBCursor::advance(uint64_t count)
     return true;
 }
 
-bool SQLiteIDBCursor::fetch()
+bool SQLiteIDBCursor::fetch(ShouldFetchForSameKey shouldFetchForSameKey)
 {
     ASSERT(m_fetchedRecords.isEmpty() || !m_fetchedRecords.last().isTerminalRecord());
 
     m_fetchedRecords.append({ });
 
-    bool isUnique = m_cursorDirection == IndexedDB::CursorDirection::Nextunique || m_cursorDirection == IndexedDB::CursorDirection::Prevunique;
+    bool isUnique = m_cursorDirection == IndexedDB::CursorDirection::Nextunique || m_cursorDirection == IndexedDB::CursorDirection::Prevunique || shouldFetchForSameKey == ShouldFetchForSameKey::Yes;
     if (!isUnique)
         return fetchNextRecord(m_fetchedRecords.last());
 
-    while (!m_fetchedRecords.last().completed) {
-        if (!fetchNextRecord(m_fetchedRecords.last()))
-            return false;
-
-        // If the new current key is different from the old current key, we're done.
+    while (fetchNextRecord(m_fetchedRecords.last())) {
         if (m_currentKeyForUniqueness.compare(m_fetchedRecords.last().record.key))
             return true;
+
+        if (m_fetchedRecords.last().completed)
+            return false;
+
+        if (shouldFetchForSameKey == ShouldFetchForSameKey::Yes)
+            m_fetchedRecords.append({ });
     }
 
     return false;
index 4a67300..de89db8 100644 (file)
@@ -44,6 +44,8 @@ class IDBGetResult;
 
 namespace IDBServer {
 
+enum class ShouldFetchForSameKey : bool { No, Yes };
+
 class SQLiteIDBTransaction;
 
 class SQLiteIDBCursor {
@@ -91,7 +93,7 @@ private:
         ShouldFetchAgain
     };
 
-    bool fetch();
+    bool fetch(ShouldFetchForSameKey = ShouldFetchForSameKey::No);
 
     struct SQLiteCursorRecord {
         IDBCursorRecord record;