Bug 85513

Summary: IndexedDB: Throw proper exceptions
Product: WebKit Reporter: Alec Flett <alecflett>
Component: WebCore Misc.Assignee: Alec Flett <alecflett>
Status: RESOLVED FIXED    
Severity: Normal CC: alecflett, dgrogan, jsbell, tony, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 87987, 88459, 88690, 89243, 91679, 92562    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch none

Alec Flett
Reported 2012-05-03 10:57:38 PDT
This is the catch-all bug for the exception work that we need to do in IndexedDB - mostly this involves aligning our exceptions to DOM4 exceptions, but there are a few places where the spec says we should throw a JavaScript TypeError.
Attachments
Patch (29.20 KB, patch)
2012-07-27 17:18 PDT, Joshua Bell 		no flags
DetailsFormatted DiffDiff
Patch (53.93 KB, patch)
2012-08-01 14:42 PDT, Joshua Bell 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Alec Flett
Comment 1 2012-05-25 15:24:16 PDT
Most of these are straight forward, but I spent some time trying to figure out how easy it would be to support the "Javascript TypeError" requirements in the indexeddb spec - it's kind of a nightmare in WebKit, since it's a JSEngine-specific call. The only way to support these are via additional features in WebIDL. there are only 3 in the spec, I suspect we should pushback and/or spin off into a separate bug. The three places are: 1) in open(), the version has to be > 0 2) in advance() the count must be > 0 3) in openCursor() the direction must be one of a small set of valid strings (and for legacy support, also possibly a number between 0 and 3 inclusive
Alec Flett
Comment 2 2012-06-15 11:46:36 PDT
Adding the metadata-snapshot stuff because that has the side benefit of letting the render-side (frontend) objects know that they've been deleted - i.e. by adding a deleted flag to IDBObjectStore and the like, and throwing an InvalidStateError when that occurs.
Joshua Bell
Comment 3 2012-06-28 09:27:17 PDT
What's left to do here - just the TypeError work in http://webkit.org/b/87987 ?
Alec Flett
Comment 4 2012-06-28 09:31:46 PDT
I think there are one or two one-off fixes left - last I did a sweep through, about a week ago, I found one and made a unit test - I'll go dig up that branch.
Joshua Bell
Comment 5 2012-07-19 09:45:13 PDT
Anything else remaining?
Joshua Bell
Comment 6 2012-07-27 17:18:49 PDT
Created attachment 155094 [details] Patch
Joshua Bell
Comment 7 2012-08-01 14:42:11 PDT
Created attachment 155889 [details] Patch
Joshua Bell
Comment 8 2012-08-01 14:43:55 PDT
This is ready to land. Just rearranged some of the tests slightly for readability and to match the spec order (except in a few places to avoid horrible async contortions). dgrogan@, alecflett@ - please take a look.
David Grogan
Comment 9 2012-08-01 15:03:48 PDT
Comment on attachment 155889 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=155889&action=review LGTM > LayoutTests/storage/indexeddb/resources/exceptions.js:213 > + // "Occurs if a request is made on a source object that has been deleted or removed." - covered in deleted-objects.html Are these comments intentional?
Joshua Bell
Comment 10 2012-08-01 15:12:29 PDT
(In reply to comment #9) > > LayoutTests/storage/indexeddb/resources/exceptions.js:213 > > + // "Occurs if a request is made on a source object that has been deleted or removed." - covered in deleted-objects.html > > Are these comments intentional? Yeah - those are the only exceptions listed in the spec that this patch didn't test, and I wanted to make sure that the coverage in that other test was referenced.
Joshua Bell
Comment 11 2012-08-06 12:21:57 PDT
tony@ - r? cq?
Tony Chang
Comment 12 2012-08-06 18:16:12 PDT
Comment on attachment 155889 [details] Patch rs=me
WebKit Review Bot
Comment 13 2012-08-06 19:44:36 PDT
Comment on attachment 155889 [details] Patch Clearing flags on attachment: 155889 Committed r124837: <http://trac.webkit.org/changeset/124837>
WebKit Review Bot
Comment 14 2012-08-06 19:44:40 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.