Modern IDB: SQLite backend doesn't handle mutation during cursor iteration.

https://bugs.webkit.org/show_bug.cgi?id=153614

Reviewed by Alex Christensen.

Source/WebCore:

No new tests (A few failing tests pass, a few get closer).

* Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:
(WebCore::IDBServer::SQLiteIDBBackingStore::deleteRange): Call notifyCursorsOfChanges.
(WebCore::IDBServer::SQLiteIDBBackingStore::addRecord): Ditto.

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

* Modules/indexeddb/server/SQLiteIDBTransaction.cpp:
(WebCore::IDBServer::SQLiteIDBTransaction::maybeOpenBackingStoreCursor): Remember these transient backing
  store cursors so they can be notified of changes.
(WebCore::IDBServer::SQLiteIDBTransaction::closeCursor): Handle removing the cursor from the right set.
(WebCore::IDBServer::SQLiteIDBTransaction::notifyCursorsOfChanges):
* Modules/indexeddb/server/SQLiteIDBTransaction.h:

LayoutTests:

* platform/mac-wk1/TestExpectations:

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 6ccd92c..fe57658 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,5 +1,14 @@
 2016-01-28  Brady Eidson  <beidson@apple.com>
 
+        Modern IDB: SQLite backend doesn't handle mutation during cursor iteration.
+        https://bugs.webkit.org/show_bug.cgi?id=153614
+
+        Reviewed by Alex Christensen.
+
+        * platform/mac-wk1/TestExpectations:
+
+2016-01-28  Brady Eidson  <beidson@apple.com>
+
         Modern IDB: SQLite backend doesn't support deleting ranges with more than one key.
         https://bugs.webkit.org/show_bug.cgi?id=153604
 

diff --git a/LayoutTests/platform/mac-wk1/TestExpectations b/LayoutTests/platform/mac-wk1/TestExpectations
index 5b257b8..1340fd7 100644 (file)
--- a/LayoutTests/platform/mac-wk1/TestExpectations
+++ b/LayoutTests/platform/mac-wk1/TestExpectations
@@ -461,8 +461,6 @@ storage/indexeddb/key-generator.html [ Failure ]
 storage/indexeddb/modern/cursor-7.html [ Failure ]
 storage/indexeddb/modern/get-keyrange.html [ Failure ]
 storage/indexeddb/modern/index-3.html [ Failure ]
-storage/indexeddb/mozilla/cursor-mutation.html [ Failure ]
-storage/indexeddb/mozilla/cursors.html [ Failure ]
 storage/indexeddb/objectstore-autoincrement.html [ Failure ]
 
 # SQLite backend tests that timeout

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 9db62b8..3c2cff4 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,5 +1,30 @@
 2016-01-28  Brady Eidson  <beidson@apple.com>
 
+        Modern IDB: SQLite backend doesn't handle mutation during cursor iteration.
+        https://bugs.webkit.org/show_bug.cgi?id=153614
+
+        Reviewed by Alex Christensen.
+
+        No new tests (A few failing tests pass, a few get closer).
+
+        * Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:
+        (WebCore::IDBServer::SQLiteIDBBackingStore::deleteRange): Call notifyCursorsOfChanges.
+        (WebCore::IDBServer::SQLiteIDBBackingStore::addRecord): Ditto.
+
+        * Modules/indexeddb/server/SQLiteIDBCursor.cpp:
+        (WebCore::IDBServer::SQLiteIDBCursor::SQLiteIDBCursor):
+        (WebCore::IDBServer::SQLiteIDBCursor::~SQLiteIDBCursor):
+        * Modules/indexeddb/server/SQLiteIDBCursor.h:
+
+        * Modules/indexeddb/server/SQLiteIDBTransaction.cpp:
+        (WebCore::IDBServer::SQLiteIDBTransaction::maybeOpenBackingStoreCursor): Remember these transient backing 
+          store cursors so they can be notified of changes.
+        (WebCore::IDBServer::SQLiteIDBTransaction::closeCursor): Handle removing the cursor from the right set.
+        (WebCore::IDBServer::SQLiteIDBTransaction::notifyCursorsOfChanges):
+        * Modules/indexeddb/server/SQLiteIDBTransaction.h:
+
+2016-01-28  Brady Eidson  <beidson@apple.com>
+
         Modern IDB: SQLite backend doesn't support deleting ranges with more than one key.
         https://bugs.webkit.org/show_bug.cgi?id=153604
 

diff --git a/Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp b/Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp
index dcd8700..e7cfbf9 100644 (file)
--- a/Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp
+++ b/Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp
@@ -1055,6 +1055,8 @@ IDBError SQLiteIDBBackingStore::deleteRange(const IDBResourceIdentifier& transac
         }
     }
 
+    transaction->notifyCursorsOfChanges(objectStoreID);
+
     return error;
 }
 
@@ -1144,6 +1146,8 @@ IDBError SQLiteIDBBackingStore::addRecord(const IDBResourceIdentifier& transacti
 
     // FIXME: If there was an error indexing this record, remove it.
 
+    transaction->notifyCursorsOfChanges(objectStoreInfo.identifier());
+
     return error;
 }
 

diff --git a/Source/WebCore/Modules/indexeddb/server/SQLiteIDBCursor.cpp b/Source/WebCore/Modules/indexeddb/server/SQLiteIDBCursor.cpp
index 33ccd44..b9a0ef5 100644 (file)
--- a/Source/WebCore/Modules/indexeddb/server/SQLiteIDBCursor.cpp
+++ b/Source/WebCore/Modules/indexeddb/server/SQLiteIDBCursor.cpp
@@ -83,10 +83,17 @@ SQLiteIDBCursor::SQLiteIDBCursor(SQLiteIDBTransaction& transaction, const uint64
     , m_indexID(indexID ? indexID : IDBIndexMetadata::InvalidId)
     , m_cursorDirection(IndexedDB::CursorDirection::Next)
     , m_keyRange(range)
+    , m_backingStoreCursor(true)
 {
     ASSERT(m_objectStoreID);
 }
 
+SQLiteIDBCursor::~SQLiteIDBCursor()
+{
+    if (m_backingStoreCursor)
+        m_transaction->closeCursor(*this);
+}
+
 void SQLiteIDBCursor::currentData(IDBGetResult& result)
 {
     if (m_completed) {

diff --git a/Source/WebCore/Modules/indexeddb/server/SQLiteIDBCursor.h b/Source/WebCore/Modules/indexeddb/server/SQLiteIDBCursor.h
index 37b11c7..1704cee 100644 (file)
--- a/Source/WebCore/Modules/indexeddb/server/SQLiteIDBCursor.h
+++ b/Source/WebCore/Modules/indexeddb/server/SQLiteIDBCursor.h
@@ -52,6 +52,8 @@ public:
     SQLiteIDBCursor(SQLiteIDBTransaction&, const IDBCursorInfo&);
     SQLiteIDBCursor(SQLiteIDBTransaction&, uint64_t objectStoreID, uint64_t indexID, const IDBKeyRangeData&);
 
+    ~SQLiteIDBCursor();
+
     const IDBResourceIdentifier& identifier() const { return m_cursorIdentifier; }
     SQLiteIDBTransaction* transaction() const { return m_transaction; }
 
@@ -109,6 +111,8 @@ private:
 
     bool m_completed { false };
     bool m_errored { false };
+
+    bool m_backingStoreCursor { false };
 };
 
 

diff --git a/Source/WebCore/Modules/indexeddb/server/SQLiteIDBTransaction.cpp b/Source/WebCore/Modules/indexeddb/server/SQLiteIDBTransaction.cpp
index b1f37e1..dcbe331 100644 (file)
--- a/Source/WebCore/Modules/indexeddb/server/SQLiteIDBTransaction.cpp
+++ b/Source/WebCore/Modules/indexeddb/server/SQLiteIDBTransaction.cpp
@@ -104,7 +104,12 @@ std::unique_ptr<SQLiteIDBCursor> SQLiteIDBTransaction::maybeOpenBackingStoreCurs
     ASSERT(m_sqliteTransaction);
     ASSERT(m_sqliteTransaction->inProgress());
 
-    return SQLiteIDBCursor::maybeCreateBackingStoreCursor(*this, objectStoreID, indexID, range);
+    auto cursor = SQLiteIDBCursor::maybeCreateBackingStoreCursor(*this, objectStoreID, indexID, range);
+
+    if (cursor)
+        m_backingStoreCursors.add(cursor.get());
+
+    return cursor;
 }
 
 SQLiteIDBCursor* SQLiteIDBTransaction::maybeOpenCursor(const IDBCursorInfo& info)
@@ -128,6 +133,12 @@ SQLiteIDBCursor* SQLiteIDBTransaction::maybeOpenCursor(const IDBCursorInfo& info
 
 void SQLiteIDBTransaction::closeCursor(SQLiteIDBCursor& cursor)
 {
+    auto backingStoreTake = m_backingStoreCursors.take(&cursor);
+    if (backingStoreTake) {
+        ASSERT(!m_cursors.contains(cursor.identifier()));
+        return;
+    }
+
     ASSERT(m_cursors.contains(cursor.identifier()));
 
     m_backingStore.unregisterCursor(cursor);
@@ -140,6 +151,11 @@ void SQLiteIDBTransaction::notifyCursorsOfChanges(int64_t objectStoreID)
         if (i.value->objectStoreID() == objectStoreID)
             i.value->objectStoreRecordsChanged();
     }
+
+    for (auto* cursor : m_backingStoreCursors) {
+        if (cursor->objectStoreID() == objectStoreID)
+            cursor->objectStoreRecordsChanged();
+    }
 }
 
 void SQLiteIDBTransaction::clearCursors()

diff --git a/Source/WebCore/Modules/indexeddb/server/SQLiteIDBTransaction.h b/Source/WebCore/Modules/indexeddb/server/SQLiteIDBTransaction.h
index 5ef48c3..0efe744 100644 (file)
--- a/Source/WebCore/Modules/indexeddb/server/SQLiteIDBTransaction.h
+++ b/Source/WebCore/Modules/indexeddb/server/SQLiteIDBTransaction.h
@@ -33,6 +33,7 @@
 #include "IDBTransactionInfo.h"
 #include "IndexedDB.h"
 #include <wtf/HashMap.h>
+#include <wtf/HashSet.h>
 #include <wtf/Noncopyable.h>
 
 namespace WebCore {
@@ -79,6 +80,7 @@ private:
     SQLiteIDBBackingStore& m_backingStore;
     std::unique_ptr<SQLiteTransaction> m_sqliteTransaction;
     HashMap<IDBResourceIdentifier, std::unique_ptr<SQLiteIDBCursor>> m_cursors;
+    HashSet<SQLiteIDBCursor*> m_backingStoreCursors;
 };
 
 } // namespace IDBServer