Bug 69012 – layout test demonstrating IDBCursor inconsistency bug

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[David Grogan]

layout test demonstrating IDBCursor inconsistency bug

[David Grogan]

Created attachment 109048 [details]
Patch

[David Grogan]

Michael, could you review this layout test?  It's the one I mentioned in http://codereview.chromium.org/7834006/.  I can see the inconsistency issue discussion taking a while so want to get this layout test in and out of my local client.

[Michael Nordman]

Comment on attachment 109048 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=109048&action=review

> LayoutTests/storage/indexeddb/cursor-inconsistency.html:80
> +    shouldBeEqualToString("storedCursor.key", "someKey" + counter);

would it make sense to have a test for cursor object equality too...

shouldBeEqualTo(storedCursor, event.target.result);

[Michael Nordman]

Also, this looks like a fairly significant correction bug, if the caller uses the 'wrong' cursor object to retrieve the values, results are completely whacked.

The test lgtm (fwiw, i don't have review rights), but i also think it'd be good to add the stored vs result cursor equality test.

[Joshua Bell]

Comment on attachment 109048 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=109048&action=review

> LayoutTests/storage/indexeddb/cursor-inconsistency.html:83
> +    shouldBeEqualToString("event.target.result.value", "someValue" + counter++);

Nit: Just for readability, I'd pull the counter++ onto its own line.

[David Grogan]

Created attachment 109100 [details]
Patch

[David Grogan]

Comment on attachment 109048 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=109048&action=review

>> LayoutTests/storage/indexeddb/cursor-inconsistency.html:80
>> +    shouldBeEqualToString("storedCursor.key", "someKey" + counter);
> 
> would it make sense to have a test for cursor object equality too...
> 
> shouldBeEqualTo(storedCursor, event.target.result);

Added.

>> LayoutTests/storage/indexeddb/cursor-inconsistency.html:83
>> +    shouldBeEqualToString("event.target.result.value", "someValue" + counter++);
> 
> Nit: Just for readability, I'd pull the counter++ onto its own line.

Done

[David Grogan]

Nate, could you review this test?

(Also I'm now a committer, thanks for the nomination!)

[Hans Wennborg]

lgtm too; scary stuff

[WebKit Review Bot]

Comment on attachment 109100 [details]
Patch

Clearing flags on attachment: 109100

Committed r96335: <http://trac.webkit.org/changeset/96335>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.