RESOLVED FIXED 84467
IndexedDB: cursor does not correctly iterate over keys added and removed during iteration 
https://bugs.webkit.org/show_bug.cgi?id=84467
Summary IndexedDB: cursor does not correctly iterate over keys added and removed duri...
dstockwell
Reported 2012-04-20 10:57:52 PDT
Cursor iteration can fail to yield keys added in an order opposite to the cursor's iteration order. If the cursor iterating forwards at "3", and "6" then "5" are added, "5" will never be hit. Would fail to suppress many deleted keys, but this is hidden by logic that issues a get() against the transaction on every continue() call.
Attachments
Patch (9.61 KB, patch)
2012-04-20 11:43 PDT, dstockwell 		no flags
DetailsFormatted DiffDiff
Patch (10.61 KB, patch)
2012-04-23 15:53 PDT, dstockwell 		no flags
DetailsFormatted DiffDiff
Patch (10.63 KB, patch)
2012-04-23 16:20 PDT, dstockwell 		no flags
DetailsFormatted DiffDiff
Patch (10.79 KB, patch)
2012-04-25 20:14 PDT, dstockwell 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
dstockwell
Comment 1 2012-04-20 11:43:39 PDT
Created attachment 138131 [details] Patch
Alec Flett
Comment 2 2012-04-23 15:00:01 PDT
Comment on attachment 138131 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=138131&action=review > Source/WebCore/platform/leveldb/LevelDBTransaction.cpp:417 > + && (!m_dbIterator->isValid() I find the logic of this very difficult to read - can you break at least the direction checks into a separate helper function? these: || (m_direction == kForward && m_comparator->compare(m_treeIterator->key(), m_dbIterator->key()) < 0) || (m_direction == kReverse && m_comparator->compare(m_treeIterator->key(), m_dbIterator->key()) > 0))
dstockwell
Comment 3 2012-04-23 15:53:49 PDT
Created attachment 138445 [details] Patch
Alec Flett
Comment 4 2012-04-23 16:08:57 PDT
Comment on attachment 138445 [details] Patch Thanks, this makes it much clearer...you can make the helpers const too. LGTM with that.
dstockwell
Comment 5 2012-04-23 16:20:18 PDT
Created attachment 138452 [details] Patch
Joshua Bell
Comment 6 2012-04-25 14:30:02 PDT
LGTM. ojan@ - can you review?
WebKit Review Bot
Comment 7 2012-04-25 16:51:29 PDT
Comment on attachment 138452 [details] Patch Clearing flags on attachment: 138452 Committed r115260: <http://trac.webkit.org/changeset/115260>
WebKit Review Bot
Comment 8 2012-04-25 16:51:40 PDT
All reviewed patches have been landed. Closing bug.
Hin-Chung Lam
Comment 9 2012-04-25 18:17:57 PDT
Committed r115275: <http://trac.webkit.org/changeset/115275>
dstockwell
Comment 10 2012-04-25 18:21:34 PDT
Fix was reverted due to failures in debug build.
dstockwell
Comment 11 2012-04-25 20:14:04 PDT
Created attachment 138926 [details] Patch
dstockwell
Comment 12 2012-04-25 20:15:44 PDT
Comment on attachment 138926 [details] Patch ojan@ - can you review again? I had to remove a stale assert.
WebKit Review Bot
Comment 13 2012-04-26 11:11:57 PDT
Comment on attachment 138926 [details] Patch Clearing flags on attachment: 138926 Committed r115333: <http://trac.webkit.org/changeset/115333>
WebKit Review Bot
Comment 14 2012-04-26 11:12:02 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.