Title: [237590] trunk
Revision
237590
Author
[email protected]
Date
2018-10-30 10:11:39 -0700 (Tue, 30 Oct 2018)

Log Message

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.

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):

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (237589 => 237590)


--- trunk/LayoutTests/ChangeLog	2018-10-30 14:54:23 UTC (rev 237589)
+++ trunk/LayoutTests/ChangeLog	2018-10-30 17:11:39 UTC (rev 237590)
@@ -1,3 +1,18 @@
+2018-10-30  Sihui Liu  <[email protected]>
+
+        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  <[email protected]>
 
         [Web Animations] Implement the update animations and send events procedure

Modified: trunk/LayoutTests/imported/w3c/ChangeLog (237589 => 237590)


--- trunk/LayoutTests/imported/w3c/ChangeLog	2018-10-30 14:54:23 UTC (rev 237589)
+++ trunk/LayoutTests/imported/w3c/ChangeLog	2018-10-30 17:11:39 UTC (rev 237590)
@@ -1,3 +1,15 @@
+2018-10-30  Sihui Liu  <[email protected]>
+
+        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  <[email protected]>
 
         [Web Animations] Implement the update animations and send events procedure

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/IndexedDB/idbcursor-iterating-update-expected.txt (237589 => 237590)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/IndexedDB/idbcursor-iterating-update-expected.txt	2018-10-30 14:54:23 UTC (rev 237589)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/IndexedDB/idbcursor-iterating-update-expected.txt	2018-10-30 17:11:39 UTC (rev 237590)
@@ -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
+PASS Calling cursor => cursor.update({}) doesn't affect index iteration 
+PASS Calling cursor => cursor.delete() doesn't affect index iteration 
 
-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
-

Added: trunk/LayoutTests/storage/indexeddb/cursor-update-while-iterating-expected.txt (0 => 237590)


--- trunk/LayoutTests/storage/indexeddb/cursor-update-while-iterating-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/storage/indexeddb/cursor-update-while-iterating-expected.txt	2018-10-30 17:11:39 UTC (rev 237590)
@@ -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
+

Added: trunk/LayoutTests/storage/indexeddb/cursor-update-while-iterating.html (0 => 237590)


--- trunk/LayoutTests/storage/indexeddb/cursor-update-while-iterating.html	                        (rev 0)
+++ trunk/LayoutTests/storage/indexeddb/cursor-update-while-iterating.html	2018-10-30 17:11:39 UTC (rev 237590)
@@ -0,0 +1,9 @@
+<html>
+<head>
+<script src=""
+<script src=""
+</head>
+<body>
+<script src=""
+</body>
+</html>

Added: trunk/LayoutTests/storage/indexeddb/resources/cursor-update-while-iterating.js (0 => 237590)


--- trunk/LayoutTests/storage/indexeddb/resources/cursor-update-while-iterating.js	                        (rev 0)
+++ trunk/LayoutTests/storage/indexeddb/resources/cursor-update-while-iterating.js	2018-10-30 17:11:39 UTC (rev 237590)
@@ -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

Modified: trunk/Source/WebCore/ChangeLog (237589 => 237590)


--- trunk/Source/WebCore/ChangeLog	2018-10-30 14:54:23 UTC (rev 237589)
+++ trunk/Source/WebCore/ChangeLog	2018-10-30 17:11:39 UTC (rev 237590)
@@ -1,3 +1,23 @@
+2018-10-30  Sihui Liu  <[email protected]>
+
+        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  <[email protected]>
 
         [LFC][IFC] Introduce InlineFormattingContextGeometry class

Modified: trunk/Source/WebCore/Modules/indexeddb/server/SQLiteIDBCursor.cpp (237589 => 237590)


--- trunk/Source/WebCore/Modules/indexeddb/server/SQLiteIDBCursor.cpp	2018-10-30 14:54:23 UTC (rev 237589)
+++ trunk/Source/WebCore/Modules/indexeddb/server/SQLiteIDBCursor.cpp	2018-10-30 17:11:39 UTC (rev 237590)
@@ -214,15 +214,27 @@
     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;
@@ -229,7 +241,7 @@
             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;
@@ -236,10 +248,6 @@
             m_statement = nullptr;
         }
     }
-
-    m_currentKeyForUniqueness = m_fetchedRecords.first().record.key;
-
-    m_fetchedRecords.clear();
 }
 
 void SQLiteIDBCursor::resetAndRebindStatement()
@@ -360,23 +368,25 @@
     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()))
+    while (fetchNextRecord(m_fetchedRecords.last())) {
+        if (m_currentKeyForUniqueness.compare(m_fetchedRecords.last().record.key))
+            return true;
+
+        if (m_fetchedRecords.last().completed)
             return false;
 
-        // If the new current key is different from the old current key, we're done.
-        if (m_currentKeyForUniqueness.compare(m_fetchedRecords.last().record.key))
-            return true;
+        if (shouldFetchForSameKey == ShouldFetchForSameKey::Yes)
+            m_fetchedRecords.append({ });
     }
 
     return false;

Modified: trunk/Source/WebCore/Modules/indexeddb/server/SQLiteIDBCursor.h (237589 => 237590)


--- trunk/Source/WebCore/Modules/indexeddb/server/SQLiteIDBCursor.h	2018-10-30 14:54:23 UTC (rev 237589)
+++ trunk/Source/WebCore/Modules/indexeddb/server/SQLiteIDBCursor.h	2018-10-30 17:11:39 UTC (rev 237590)
@@ -44,6 +44,8 @@
 
 namespace IDBServer {
 
+enum class ShouldFetchForSameKey : bool { No, Yes };
+
 class SQLiteIDBTransaction;
 
 class SQLiteIDBCursor {
@@ -91,7 +93,7 @@
         ShouldFetchAgain
     };
 
-    bool fetch();
+    bool fetch(ShouldFetchForSameKey = ShouldFetchForSameKey::No);
 
     struct SQLiteCursorRecord {
         IDBCursorRecord record;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to