2011-05-27 Hans Wennborg <hans@chromium.org>
authorhans@chromium.org <hans@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 27 May 2011 14:02:26 +0000 (14:02 +0000)
committerhans@chromium.org <hans@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 27 May 2011 14:02:26 +0000 (14:02 +0000)
        Reviewed by Tony Gentilcore.

        IndexedDB: Support mutating cursors on top of LevelDB
        https://bugs.webkit.org/show_bug.cgi?id=61615

        New test for adding new entries to an objecet store while running a
        cursor over it.

        * storage/indexeddb/mutating-cursor-expected.txt: Added.
        * storage/indexeddb/mutating-cursor.html: Added.
2011-05-27  Hans Wennborg  <hans@chromium.org>

        Reviewed by Tony Gentilcore.

        IndexedDB: Support mutating cursors on top of LevelDB
        https://bugs.webkit.org/show_bug.cgi?id=61615

        We need to support the case where a new node is added to the tree in a
        transaction after the TreeIterator has covered the whole tree.

        Since this is done lazily, i.e. we set a flag that the tree might have
        changed, and act upon it later, some members need to be mutable,
        because we might need to re-seek the tree iterator in a const function.

        Test: storage/indexeddb/mutating-cursor.html
              storage/indexeddb/mozilla/cursor-mutation-objectstore-only.html (existing)

        * platform/leveldb/LevelDBTransaction.cpp:
        (WebCore::LevelDBTransaction::set):
        (WebCore::LevelDBTransaction::TreeIterator::reset):
        (WebCore::LevelDBTransaction::TreeIterator::~TreeIterator):
        (WebCore::LevelDBTransaction::TreeIterator::TreeIterator):
        (WebCore::LevelDBTransaction::TransactionIterator::TransactionIterator):
        (WebCore::LevelDBTransaction::TransactionIterator::~TransactionIterator):
        (WebCore::LevelDBTransaction::TransactionIterator::next):
        (WebCore::LevelDBTransaction::TransactionIterator::prev):
        (WebCore::LevelDBTransaction::TransactionIterator::key):
        (WebCore::LevelDBTransaction::TransactionIterator::value):
        (WebCore::LevelDBTransaction::TransactionIterator::treeChanged):
        (WebCore::LevelDBTransaction::TransactionIterator::refreshTreeIterator):
        (WebCore::LevelDBTransaction::registerIterator):
        (WebCore::LevelDBTransaction::unregisterIterator):
        (WebCore::LevelDBTransaction::notifyIteratorsOfTreeChange):
        * platform/leveldb/LevelDBTransaction.h:
        * storage/IDBFactoryBackendImpl.cpp:
        (WebCore::IDBFactoryBackendImpl::open):
        * storage/IDBLevelDBBackingStore.cpp:
        (WebCore::IDBLevelDBBackingStore::open):

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

LayoutTests/ChangeLog
LayoutTests/storage/indexeddb/mutating-cursor-expected.txt [new file with mode: 0644]
LayoutTests/storage/indexeddb/mutating-cursor.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/platform/leveldb/LevelDBTransaction.cpp
Source/WebCore/platform/leveldb/LevelDBTransaction.h

index 10d12d7..2eab6c9 100644 (file)
@@ -1,3 +1,16 @@
+2011-05-27  Hans Wennborg  <hans@chromium.org>
+
+        Reviewed by Tony Gentilcore.
+
+        IndexedDB: Support mutating cursors on top of LevelDB
+        https://bugs.webkit.org/show_bug.cgi?id=61615
+
+        New test for adding new entries to an objecet store while running a
+        cursor over it.
+
+        * storage/indexeddb/mutating-cursor-expected.txt: Added.
+        * storage/indexeddb/mutating-cursor.html: Added.
+
 2011-05-27  Csaba Osztrogonác  <ossy@webkit.org>
 
         [Qt][Mac] Daily gardening.
diff --git a/LayoutTests/storage/indexeddb/mutating-cursor-expected.txt b/LayoutTests/storage/indexeddb/mutating-cursor-expected.txt
new file mode 100644 (file)
index 0000000..270b24f
--- /dev/null
@@ -0,0 +1,92 @@
+Test mutating an IndexedDB's objectstore from a cursor.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+webkitIndexedDB.open('mutating-cursor')
+db = event.target.result
+db.setVersion('1')
+setVersionSuccess():
+trans = event.target.result
+PASS trans !== null is true
+Deleted all object stores.
+objectStore = db.createObjectStore('store')
+objectStore.add(1, 1).onerror = unexpectedErrorCallback
+objectStore.add(2, 2).onerror = unexpectedErrorCallback
+objectStore.add(3, 3).onerror = unexpectedErrorCallback
+objectStore.add(4, 4).onerror = unexpectedErrorCallback
+openForwardCursor()
+trans = db.transaction([], webkitIDBTransaction.READ_WRITE)
+trans.objectStore('store')
+objectStore.openCursor()
+forwardCursor()
+1
+PASS cursor.key is cursorSteps
+PASS cursor.value is cursorSteps
+event.target.source.add(5, 5)
+cursor.continue()
+forwardCursor()
+2
+PASS cursor.key is cursorSteps
+PASS cursor.value is cursorSteps
+cursor.continue()
+forwardCursor()
+3
+PASS cursor.key is cursorSteps
+PASS cursor.value is cursorSteps
+cursor.continue()
+forwardCursor()
+4
+PASS cursor.key is cursorSteps
+PASS cursor.value is cursorSteps
+cursor.continue()
+forwardCursor()
+5
+PASS cursor.key is cursorSteps
+PASS cursor.value is cursorSteps
+cursor.continue()
+forwardCursor()
+PASS cursorSteps is 5
+forwardCursorComplete()
+openReverseCursor()
+trans = db.transaction([], webkitIDBTransaction.READ_WRITE)
+trans.objectStore('store')
+objectStore.openCursor(null, webkitIDBCursor.PREV)
+reverseCursor()
+5
+PASS cursor.key is cursorSteps
+PASS cursor.value is cursorSteps
+cursor.continue()
+reverseCursor()
+4
+PASS cursor.key is cursorSteps
+PASS cursor.value is cursorSteps
+cursor.continue()
+reverseCursor()
+3
+PASS cursor.key is cursorSteps
+PASS cursor.value is cursorSteps
+cursor.continue()
+reverseCursor()
+2
+PASS cursor.key is cursorSteps
+PASS cursor.value is cursorSteps
+event.target.source.add(0, 0)
+cursor.continue()
+reverseCursor()
+1
+PASS cursor.key is cursorSteps
+PASS cursor.value is cursorSteps
+cursor.continue()
+reverseCursor()
+0
+PASS cursor.key is cursorSteps
+PASS cursor.value is cursorSteps
+cursor.continue()
+reverseCursor()
+PASS cursorSteps is 0
+reverseCursorComplete()
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/storage/indexeddb/mutating-cursor.html b/LayoutTests/storage/indexeddb/mutating-cursor.html
new file mode 100644 (file)
index 0000000..5808db7
--- /dev/null
@@ -0,0 +1,146 @@
+<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="../../fast/js/resources/js-test-post-function.js"></script>
+<script src="resources/shared.js"></script>
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<script>
+
+description("Test mutating an IndexedDB's objectstore from a cursor.");
+if (window.layoutTestController)
+    layoutTestController.waitUntilDone();
+
+test();
+
+function test()
+{
+    request = evalAndLog("webkitIndexedDB.open('mutating-cursor')");
+    request.onsuccess = openSuccess;
+    request.onerror = unexpectedErrorCallback;
+}
+
+function openSuccess()
+{
+    var db = evalAndLog("db = event.target.result");
+
+    request = evalAndLog("db.setVersion('1')");
+    request.onsuccess = setVersionSuccess;
+    request.onerror = unexpectedErrorCallback;
+}
+
+function setVersionSuccess()
+{
+    debug("setVersionSuccess():");
+    window.trans = evalAndLog("trans = event.target.result");
+    shouldBeTrue("trans !== null");
+    trans.onabort = unexpectedAbortCallback;
+    trans.oncomplete = openForwardCursor;
+
+    deleteAllObjectStores(db);
+
+    var objectStore = evalAndLog("objectStore = db.createObjectStore('store')");
+    evalAndLog("objectStore.add(1, 1).onerror = unexpectedErrorCallback");
+    evalAndLog("objectStore.add(2, 2).onerror = unexpectedErrorCallback");
+    evalAndLog("objectStore.add(3, 3).onerror = unexpectedErrorCallback");
+    evalAndLog("objectStore.add(4, 4).onerror = unexpectedErrorCallback");
+}
+
+function openForwardCursor()
+{
+    debug("openForwardCursor()");
+    evalAndLog("trans = db.transaction([], webkitIDBTransaction.READ_WRITE)");
+    trans.onabort = unexpectedAbortCallback;
+    trans.oncomplete = forwardCursorComplete;
+
+    window.objectStore = evalAndLog("trans.objectStore('store')");
+    request = evalAndLog("objectStore.openCursor()");
+    request.onsuccess = forwardCursor;
+    request.onerror = unexpectedErrorCallback;
+    window.cursorSteps = 0;
+}
+
+function forwardCursor()
+{
+    debug("forwardCursor()");
+    window.cursor = event.target.result;
+
+    if (event.target.result == null) {
+        shouldBe("cursorSteps", "5");
+
+        // Let the transaction finish.
+        return;
+    }
+
+    debug(++cursorSteps);
+    shouldBe("cursor.key", "cursorSteps");
+    shouldBe("cursor.value", "cursorSteps");
+
+    if (cursorSteps == 1) {
+        request = evalAndLog("event.target.source.add(5, 5)");
+        request.onsuccess = function() { evalAndLog("cursor.continue()"); }
+        request.onerror = unexpectedErrorCallback;
+    } else {
+        evalAndLog("cursor.continue()");
+    }
+}
+
+function forwardCursorComplete()
+{
+    debug("forwardCursorComplete()");
+    openReverseCursor()
+}
+
+function openReverseCursor()
+{
+    debug("openReverseCursor()");
+    evalAndLog("trans = db.transaction([], webkitIDBTransaction.READ_WRITE)");
+    trans.onabort = unexpectedAbortCallback;
+    trans.oncomplete = reverseCursorComplete;
+
+    window.objectStore = evalAndLog("trans.objectStore('store')");
+    request = evalAndLog("objectStore.openCursor(null, webkitIDBCursor.PREV)");
+    request.onsuccess = reverseCursor;
+    request.onerror = unexpectedErrorCallback;
+    window.cursorSteps = 6;
+}
+
+function reverseCursor()
+{
+    debug("reverseCursor()");
+    window.cursor = event.target.result;
+
+    if (event.target.result == null) {
+        shouldBe("cursorSteps", "0");
+
+        // Let the transaction finish.
+        return;
+    }
+
+    debug(--cursorSteps);
+    shouldBe("cursor.key", "cursorSteps");
+    shouldBe("cursor.value", "cursorSteps");
+
+    if (cursorSteps == 2) {
+        request = evalAndLog("event.target.source.add(0, 0)");
+        request.onsuccess = function() { evalAndLog("cursor.continue()"); }
+        request.onerror = unexpectedErrorCallback;
+    } else {
+        evalAndLog("cursor.continue()");
+    }
+}
+
+function reverseCursorComplete()
+{
+    debug("reverseCursorComplete()");
+    done();
+}
+
+var successfullyParsed = true;
+
+</script>
+</body>
+</html>
index 1a737ba..5348e87 100644 (file)
@@ -1,3 +1,42 @@
+2011-05-27  Hans Wennborg  <hans@chromium.org>
+
+        Reviewed by Tony Gentilcore.
+
+        IndexedDB: Support mutating cursors on top of LevelDB
+        https://bugs.webkit.org/show_bug.cgi?id=61615
+
+        We need to support the case where a new node is added to the tree in a
+        transaction after the TreeIterator has covered the whole tree.
+
+        Since this is done lazily, i.e. we set a flag that the tree might have
+        changed, and act upon it later, some members need to be mutable,
+        because we might need to re-seek the tree iterator in a const function.
+
+        Test: storage/indexeddb/mutating-cursor.html
+              storage/indexeddb/mozilla/cursor-mutation-objectstore-only.html (existing)
+
+        * platform/leveldb/LevelDBTransaction.cpp:
+        (WebCore::LevelDBTransaction::set):
+        (WebCore::LevelDBTransaction::TreeIterator::reset):
+        (WebCore::LevelDBTransaction::TreeIterator::~TreeIterator):
+        (WebCore::LevelDBTransaction::TreeIterator::TreeIterator):
+        (WebCore::LevelDBTransaction::TransactionIterator::TransactionIterator):
+        (WebCore::LevelDBTransaction::TransactionIterator::~TransactionIterator):
+        (WebCore::LevelDBTransaction::TransactionIterator::next):
+        (WebCore::LevelDBTransaction::TransactionIterator::prev):
+        (WebCore::LevelDBTransaction::TransactionIterator::key):
+        (WebCore::LevelDBTransaction::TransactionIterator::value):
+        (WebCore::LevelDBTransaction::TransactionIterator::treeChanged):
+        (WebCore::LevelDBTransaction::TransactionIterator::refreshTreeIterator):
+        (WebCore::LevelDBTransaction::registerIterator):
+        (WebCore::LevelDBTransaction::unregisterIterator):
+        (WebCore::LevelDBTransaction::notifyIteratorsOfTreeChange):
+        * platform/leveldb/LevelDBTransaction.h:
+        * storage/IDBFactoryBackendImpl.cpp:
+        (WebCore::IDBFactoryBackendImpl::open):
+        * storage/IDBLevelDBBackingStore.cpp:
+        (WebCore::IDBLevelDBBackingStore::open):
+
 2011-05-27  Sujin Park  <sujjin.park@gmail.com>
 
         Unreviewed, buildfix if --no-javascript-debugger.
index efc416e..a0938f9 100644 (file)
@@ -87,7 +87,7 @@ bool LevelDBTransaction::set(const LevelDBSlice& key, const Vector<char>& value,
     node->deleted = deleted;
 
     if (newNode)
-        resetIterators();
+        notifyIteratorsOfTreeChange();
     return true;
 }
 
@@ -222,23 +222,19 @@ bool LevelDBTransaction::TreeIterator::isDeleted() const
 
 void LevelDBTransaction::TreeIterator::reset()
 {
-    // FIXME: Be lazy: set a flag and do the actual reset next time we use the iterator.
-    if (isValid()) {
-        m_iterator.start_iter(*m_tree, m_key, TreeType::EQUAL);
-        ASSERT(isValid());
-    }
+    ASSERT(isValid());
+    m_iterator.start_iter(*m_tree, m_key, TreeType::EQUAL);
+    ASSERT(isValid());
 }
 
 LevelDBTransaction::TreeIterator::~TreeIterator()
 {
-    m_transaction->unregisterIterator(this);
 }
 
 LevelDBTransaction::TreeIterator::TreeIterator(LevelDBTransaction* transaction)
     : m_tree(&transaction->m_tree)
     , m_transaction(transaction)
 {
-    transaction->registerIterator(this);
 }
 
 PassOwnPtr<LevelDBTransaction::TransactionIterator> LevelDBTransaction::TransactionIterator::create(PassRefPtr<LevelDBTransaction> transaction)
@@ -253,7 +249,14 @@ LevelDBTransaction::TransactionIterator::TransactionIterator(PassRefPtr<LevelDBT
     , m_dbIterator(m_transaction->m_db->createIterator())
     , m_current(0)
     , m_direction(kForward)
+    , m_treeChanged(false)
+{
+    m_transaction->registerIterator(this);
+}
+
+LevelDBTransaction::TransactionIterator::~TransactionIterator()
 {
+    m_transaction->unregisterIterator(this);
 }
 
 bool LevelDBTransaction::TransactionIterator::isValid() const
@@ -284,6 +287,8 @@ void LevelDBTransaction::TransactionIterator::seek(const LevelDBSlice& target)
 void LevelDBTransaction::TransactionIterator::next()
 {
     ASSERT(isValid());
+    if (m_treeChanged)
+        refreshTreeIterator();
 
     if (m_direction != kForward) {
         // Ensure the non-current iterator is positioned after key().
@@ -307,6 +312,8 @@ void LevelDBTransaction::TransactionIterator::next()
 void LevelDBTransaction::TransactionIterator::prev()
 {
     ASSERT(isValid());
+    if (m_treeChanged)
+        refreshTreeIterator();
 
     if (m_direction != kReverse) {
         // Ensure the non-current iterator is positioned before key().
@@ -336,15 +343,55 @@ void LevelDBTransaction::TransactionIterator::prev()
 LevelDBSlice LevelDBTransaction::TransactionIterator::key() const
 {
     ASSERT(isValid());
+    if (m_treeChanged)
+        refreshTreeIterator();
     return m_current->key();
 }
 
 LevelDBSlice LevelDBTransaction::TransactionIterator::value() const
 {
     ASSERT(isValid());
+    if (m_treeChanged)
+        refreshTreeIterator();
     return m_current->value();
 }
 
+void LevelDBTransaction::TransactionIterator::treeChanged()
+{
+    m_treeChanged = true;
+}
+
+void LevelDBTransaction::TransactionIterator::refreshTreeIterator() const
+{
+    ASSERT(m_treeChanged);
+
+    if (m_treeIterator->isValid()) {
+        m_treeIterator->reset();
+        return;
+    }
+
+    if (m_dbIterator->isValid()) {
+        ASSERT(!m_treeIterator->isValid());
+
+        // There could be new nodes in the tree that we should iterate over.
+
+        if (m_direction == kForward) {
+            // Try to seek tree iterator to something greater than the db iterator.
+            m_treeIterator->seek(m_dbIterator->key());
+            if (m_treeIterator->isValid() && !m_comparator->compare(m_treeIterator->key(), m_dbIterator->key()))
+                m_treeIterator->next(); // If equal, take another step so the tree iterator is strictly greater.
+        } else {
+            // If going backward, seek to a key less than the db iterator.
+            ASSERT(m_direction == kReverse);
+            m_treeIterator->seek(m_dbIterator->key());
+            if (m_treeIterator->isValid())
+                m_treeIterator->prev();
+        }
+    }
+
+    m_treeChanged = false;
+}
+
 void LevelDBTransaction::TransactionIterator::handleConflictsAndDeletes()
 {
     bool loop = true;
@@ -402,23 +449,23 @@ void LevelDBTransaction::TransactionIterator::setCurrentIteratorToLargestKey()
     m_current = largest;
 }
 
-void LevelDBTransaction::registerIterator(TreeIterator* iterator)
+void LevelDBTransaction::registerIterator(TransactionIterator* iterator)
 {
-    ASSERT(!m_treeIterators.contains(iterator));
-    m_treeIterators.add(iterator);
+    ASSERT(!m_iterators.contains(iterator));
+    m_iterators.add(iterator);
 }
 
-void LevelDBTransaction::unregisterIterator(TreeIterator* iterator)
+void LevelDBTransaction::unregisterIterator(TransactionIterator* iterator)
 {
-    ASSERT(m_treeIterators.contains(iterator));
-    m_treeIterators.remove(iterator);
+    ASSERT(m_iterators.contains(iterator));
+    m_iterators.remove(iterator);
 }
 
-void LevelDBTransaction::resetIterators()
+void LevelDBTransaction::notifyIteratorsOfTreeChange()
 {
-    for (HashSet<TreeIterator*>::iterator i = m_treeIterators.begin(); i != m_treeIterators.end(); ++i) {
-        TreeIterator* treeIterator = *i;
-        treeIterator->reset();
+    for (HashSet<TransactionIterator*>::iterator i = m_iterators.begin(); i != m_iterators.end(); ++i) {
+        TransactionIterator* transactionIterator = *i;
+        transactionIterator->treeChanged();
     }
 }
 
index 72e862b..a9140fb 100644 (file)
@@ -121,7 +121,7 @@ private:
 
     class TransactionIterator : public LevelDBIterator {
     public:
-        ~TransactionIterator() { };
+        ~TransactionIterator();
         static PassOwnPtr<TransactionIterator> create(PassRefPtr<LevelDBTransaction>);
 
         virtual bool isValid() const;
@@ -131,16 +131,18 @@ private:
         virtual void prev();
         virtual LevelDBSlice key() const;
         virtual LevelDBSlice value() const;
+        void treeChanged();
 
     private:
         TransactionIterator(PassRefPtr<LevelDBTransaction>);
         void handleConflictsAndDeletes();
         void setCurrentIteratorToSmallestKey();
         void setCurrentIteratorToLargestKey();
+        void refreshTreeIterator() const;
 
         RefPtr<LevelDBTransaction> m_transaction;
         const LevelDBComparator* m_comparator;
-        OwnPtr<TreeIterator> m_treeIterator;
+        mutable OwnPtr<TreeIterator> m_treeIterator;
         OwnPtr<LevelDBIterator> m_dbIterator;
         LevelDBIterator* m_current;
 
@@ -149,19 +151,20 @@ private:
             kReverse
         };
         Direction m_direction;
+        mutable bool m_treeChanged;
     };
 
     bool set(const LevelDBSlice& key, const Vector<char>& value, bool deleted);
     void clearTree();
-    void registerIterator(TreeIterator*);
-    void unregisterIterator(TreeIterator*);
-    void resetIterators();
+    void registerIterator(TransactionIterator*);
+    void unregisterIterator(TransactionIterator*);
+    void notifyIteratorsOfTreeChange();
 
     LevelDBDatabase* m_db;
     const LevelDBComparator* m_comparator;
     TreeType m_tree;
     bool m_finished;
-    HashSet<TreeIterator*> m_treeIterators;
+    HashSet<TransactionIterator*> m_iterators;
 };
 
 }