IndexedDB: Cursors should skip deleted entries
Created attachment 81073 [details] Patch
Comment on attachment 81073 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=81073&action=review Maybe you should make your test function a bit more generic and then you can specifically test more corner cases: the first element, the last, the current, the previous, a couple previous, the next, and a couple later. (The last is all you test now.) I don't think it'll take too much time and it'll definitely make this more robust in the future. Looks great though. > Source/WebCore/storage/IDBCursorBackendImpl.cpp:104 > + newline seems kinda useless....you could even use a ternary operator if you wanted...don't care much either way tho
(In reply to comment #2) > (From update of attachment 81073 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=81073&action=review > > Maybe you should make your test function a bit more generic and then you can specifically test more corner cases: the first element, the last, the current, the previous, a couple previous, the next, and a couple later. (The last is all you test now.) I don't think it'll take too much time and it'll definitely make this more robust in the future. You're right. Trying to make the test a bit more generic and less spaghetti. > > Looks great though. > > > Source/WebCore/storage/IDBCursorBackendImpl.cpp:104 > > + > > newline seems kinda useless....you could even use a ternary operator if you wanted...don't care much either way tho Done.
Created attachment 81226 [details] Patch
Comment on attachment 81226 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=81226&action=review This test should also verify that anything not deleted is seen while iterating. r=me assuming you do that. > LayoutTests/storage/indexeddb/cursor-skip-deleted.html:61 > + resetObjectStore(); // No callback; let the setVersion transaction complete. Seems cleaner to pass in an empty function here...you probably don't even need a comment then. > LayoutTests/storage/indexeddb/cursor-skip-deleted.html:140 > + {id: 15, targets: [13,14]} while you're at it, might as well throw a few more interesting combinations in. :-) > LayoutTests/storage/indexeddb/cursor-skip-deleted.html:169 > + testCursor(deletes, "trans.objectStore('store').index('nameIndex').openCursor(webkitIDBKeyRange.lowerBound('Alpha'))", function() { resetObjectStore(); }); why do a final reset?
Committed r78035: <http://trac.webkit.org/changeset/78035>