From: hans@chromium.org Date: Fri, 27 May 2011 14:02:26 +0000 (+0000) Subject: 2011-05-27 Hans Wennborg X-Git-Tag: 070512121124~31407 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=84895b0387e7010a324ff3a9d78c25f3c253244a;p=profile%2Fivi%2Fwebkit-efl.git 2011-05-27 Hans Wennborg 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 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 --- diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog index 10d12d7..2eab6c9 100644 --- a/LayoutTests/ChangeLog +++ b/LayoutTests/ChangeLog @@ -1,3 +1,16 @@ +2011-05-27 Hans Wennborg + + 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 [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 index 0000000..270b24f --- /dev/null +++ b/LayoutTests/storage/indexeddb/mutating-cursor-expected.txt @@ -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 index 0000000..5808db7 --- /dev/null +++ b/LayoutTests/storage/indexeddb/mutating-cursor.html @@ -0,0 +1,146 @@ + + + + + + + + +

+
+ + + diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index 1a737ba..5348e87 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,42 @@ +2011-05-27 Hans Wennborg + + 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 Unreviewed, buildfix if --no-javascript-debugger. diff --git a/Source/WebCore/platform/leveldb/LevelDBTransaction.cpp b/Source/WebCore/platform/leveldb/LevelDBTransaction.cpp index efc416e..a0938f9 100644 --- a/Source/WebCore/platform/leveldb/LevelDBTransaction.cpp +++ b/Source/WebCore/platform/leveldb/LevelDBTransaction.cpp @@ -87,7 +87,7 @@ bool LevelDBTransaction::set(const LevelDBSlice& key, const Vector& 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::create(PassRefPtr transaction) @@ -253,7 +249,14 @@ LevelDBTransaction::TransactionIterator::TransactionIterator(PassRefPtrm_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::iterator i = m_treeIterators.begin(); i != m_treeIterators.end(); ++i) { - TreeIterator* treeIterator = *i; - treeIterator->reset(); + for (HashSet::iterator i = m_iterators.begin(); i != m_iterators.end(); ++i) { + TransactionIterator* transactionIterator = *i; + transactionIterator->treeChanged(); } } diff --git a/Source/WebCore/platform/leveldb/LevelDBTransaction.h b/Source/WebCore/platform/leveldb/LevelDBTransaction.h index 72e862b..a9140fb 100644 --- a/Source/WebCore/platform/leveldb/LevelDBTransaction.h +++ b/Source/WebCore/platform/leveldb/LevelDBTransaction.h @@ -121,7 +121,7 @@ private: class TransactionIterator : public LevelDBIterator { public: - ~TransactionIterator() { }; + ~TransactionIterator(); static PassOwnPtr create(PassRefPtr); 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); void handleConflictsAndDeletes(); void setCurrentIteratorToSmallestKey(); void setCurrentIteratorToLargestKey(); + void refreshTreeIterator() const; RefPtr m_transaction; const LevelDBComparator* m_comparator; - OwnPtr m_treeIterator; + mutable OwnPtr m_treeIterator; OwnPtr 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& 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 m_treeIterators; + HashSet m_iterators; }; }