Bug 69012 - layout test demonstrating IDBCursor inconsistency bug
Summary: layout test demonstrating IDBCursor inconsistency bug
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: David Grogan
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-09-28 11:58 PDT by David Grogan
Modified: 2011-09-29 09:42 PDT (History)
5 users (show)

See Also:


Attachments
Patch (6.28 KB, patch)
2011-09-28 12:05 PDT, David Grogan
no flags Details | Formatted Diff | Diff
Patch (6.60 KB, patch)
2011-09-28 17:21 PDT, David Grogan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Grogan 2011-09-28 11:58:50 PDT
layout test demonstrating IDBCursor inconsistency bug
Comment 1 David Grogan 2011-09-28 12:05:22 PDT
Created attachment 109048 [details]
Patch
Comment 2 David Grogan 2011-09-28 12:21:51 PDT
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.
Comment 3 Michael Nordman 2011-09-28 13:04:41 PDT
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);
Comment 4 Michael Nordman 2011-09-28 15:31:31 PDT
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.
Comment 5 Joshua Bell 2011-09-28 16:03:05 PDT
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.
Comment 6 David Grogan 2011-09-28 17:21:12 PDT
Created attachment 109100 [details]
Patch
Comment 7 David Grogan 2011-09-28 17:21:48 PDT
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
Comment 8 David Grogan 2011-09-28 17:23:19 PDT
Nate, could you review this test?

(Also I'm now a committer, thanks for the nomination!)
Comment 9 Hans Wennborg 2011-09-29 06:22:32 PDT
lgtm too; scary stuff
Comment 10 WebKit Review Bot 2011-09-29 09:41:56 PDT
Comment on attachment 109100 [details]
Patch

Clearing flags on attachment: 109100

Committed r96335: <http://trac.webkit.org/changeset/96335>
Comment 11 WebKit Review Bot 2011-09-29 09:42:00 PDT
All reviewed patches have been landed.  Closing bug.