Bug 84467

Summary:

IndexedDB: cursor does not correctly iterate over keys added and removed during iteration

Product:

WebKit

Reporter:

dstockwell

Component:

New Bugs

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

alecflett, dgrogan, hclam, jsbell, ojan, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none

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	Details Formatted Diff Diff
Patch (10.61 KB, patch) 2012-04-23 15:53 PDT, dstockwell	no flags	Details Formatted Diff Diff
Patch (10.63 KB, patch) 2012-04-23 16:20 PDT, dstockwell	no flags	Details Formatted Diff Diff
Patch (10.79 KB, patch) 2012-04-25 20:14 PDT, dstockwell	no flags	Details Formatted Diff Diff
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?