Bug 69012

Summary: layout test demonstrating IDBCursor inconsistency bug
Product: WebKit Reporter: David Grogan <dgrogan>
Component: New BugsAssignee: David Grogan <dgrogan>
Status: RESOLVED FIXED    
Severity: Normal CC: hans, japhet, jsbell, michaeln, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

David Grogan
Reported 2011-09-28 11:58:50 PDT
layout test demonstrating IDBCursor inconsistency bug
Attachments
Patch (6.28 KB, patch)
2011-09-28 12:05 PDT, David Grogan 		no flags
DetailsFormatted DiffDiff
Patch (6.60 KB, patch)
2011-09-28 17:21 PDT, David Grogan 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
David Grogan
Comment 1 2011-09-28 12:05:22 PDT
Created attachment 109048 [details] Patch
David Grogan
Comment 2 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.
Michael Nordman
Comment 3 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);
Michael Nordman
Comment 4 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.
Joshua Bell
Comment 5 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.
David Grogan
Comment 6 2011-09-28 17:21:12 PDT
Created attachment 109100 [details] Patch
David Grogan
Comment 7 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
David Grogan
Comment 8 2011-09-28 17:23:19 PDT
Nate, could you review this test? (Also I'm now a committer, thanks for the nomination!)
Hans Wennborg
Comment 9 2011-09-29 06:22:32 PDT
lgtm too; scary stuff
WebKit Review Bot
Comment 10 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>
WebKit Review Bot
Comment 11 2011-09-29 09:42:00 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.