IndexedDB: IDBKeyRange constructor should throw when lower > upper
Created attachment 119594 [details] Patch
Ping?
Comment on attachment 119594 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=119594&action=review > LayoutTests/storage/indexeddb/keyrange.html:197 > + try { > + debug("Equal keys, either of the bounds is open, bound(4, 4, true, true)"); Maybe add a (4, 4, false, false) test for completeness?
Created attachment 120297 [details] Patch
(In reply to comment #3) > Maybe add a (4, 4, false, false) test for completeness? Done. Josh: I had to adjust the layout tests for the count() method. Could you take a look?
Comment on attachment 120297 [details] Patch count test changes LGTM
Comment on attachment 120297 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=120297&action=review ... and overall LTGM with nits. > Source/WebCore/ChangeLog:9 > + or lower == upper and one of the bounds is open. Nit: one or both of the bounds is open > LayoutTests/storage/indexeddb/keyrange.html:178 > + testFailed("No exception thrown"); I know this follows the pattern of other tests in the file, but these should really be using evalAndExpectException() and verifying that DATA_ERR was the indeed the error thrown. (Verifying the error type is important as we've unintentionally shifted responsibility to the binding layer in the past and changed the exception type without catching it in tests.)
(In reply to comment #7) > (From update of attachment 120297 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=120297&action=review > > ... and overall LTGM with nits. > > > Source/WebCore/ChangeLog:9 > > + or lower == upper and one of the bounds is open. > > Nit: one or both of the bounds is open Done. > > > LayoutTests/storage/indexeddb/keyrange.html:178 > > + testFailed("No exception thrown"); > > I know this follows the pattern of other tests in the file, but these should really be using evalAndExpectException() and verifying that DATA_ERR was the indeed the error thrown. (Verifying the error type is important as we've unintentionally shifted responsibility to the binding layer in the past and changed the exception type without catching it in tests.) Done. New patch coming up!
Created attachment 120584 [details] Patch
Comment on attachment 120584 [details] Patch Attachment 120584 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11041145 New failing tests: http/tests/inspector/resource-tree/resource-tree-document-url.html
Comment on attachment 120584 [details] Patch LGTM
Comment on attachment 120584 [details] Patch Clearing flags on attachment: 120584 Committed r103762: <http://trac.webkit.org/changeset/103762>
All reviewed patches have been landed. Closing bug.