Bug 85513 - IndexedDB: Throw proper exceptions
Summary: IndexedDB: Throw proper exceptions
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alec Flett
URL:
Keywords:
Depends on: 87987 88459 88690 89243 91679 92562
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-03 10:57 PDT by Alec Flett
Modified: 2012-08-06 19:44 PDT (History)
5 users (show)

See Also:


Attachments
Patch (29.20 KB, patch)
2012-07-27 17:18 PDT, Joshua Bell
no flags Details | Formatted Diff | Diff
Patch (53.93 KB, patch)
2012-08-01 14:42 PDT, Joshua Bell
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alec Flett 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.
Comment 1 Alec Flett 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
Comment 2 Alec Flett 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.
Comment 3 Joshua Bell 2012-06-28 09:27:17 PDT
What's left to do here - just the TypeError work in http://webkit.org/b/87987 ?
Comment 4 Alec Flett 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.
Comment 5 Joshua Bell 2012-07-19 09:45:13 PDT
Anything else remaining?
Comment 6 Joshua Bell 2012-07-27 17:18:49 PDT
Created attachment 155094 [details]
Patch
Comment 7 Joshua Bell 2012-08-01 14:42:11 PDT
Created attachment 155889 [details]
Patch
Comment 8 Joshua Bell 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.
Comment 9 David Grogan 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?
Comment 10 Joshua Bell 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.
Comment 11 Joshua Bell 2012-08-06 12:21:57 PDT
tony@ - r? cq?
Comment 12 Tony Chang 2012-08-06 18:16:12 PDT
Comment on attachment 155889 [details]
Patch

rs=me
Comment 13 WebKit Review Bot 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>
Comment 14 WebKit Review Bot 2012-08-06 19:44:40 PDT
All reviewed patches have been landed.  Closing bug.