Bug 50075 – IndexedDB: Better errors for not yet implemented features

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Hans Wennborg]

IndexedDB: Better errors for not yet implemented features

[Hans Wennborg]

Created attachment 74872 [details]
Patch

[Jeremy Orlow]

Comment on attachment 74872 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=74872&action=review

close

> LayoutTests/ChangeLog:11
> +        Test that failing to supply a key when inserting data in an

Also test that supplying one when autoIncrement is set produces an error.

> LayoutTests/storage/indexeddb/objectstore-basics.html:152
> +        testPassed("Exception thrown: " + err);

From now on, let's do this instead:

code = e.code
shouldBe(code, THE_EXCEPTION)

This way when we change the codes, everything still works fine.  And it makes the output less confusing in the mean time.

> WebCore/ChangeLog:8
> +        In createObjectStore, make autoIncrement default (as per the spec).

remove

> WebCore/storage/IDBDatabase.cpp:68
> +    bool autoIncrement = true;

revert this.  the spec needs to be fixed instead

> WebCore/storage/IDBObjectStoreBackendImpl.cpp:211
> +    } else if (!key && objectStore->autoIncrement()) {
> +        callbacks->onError(IDBDatabaseError::create(IDBDatabaseException::DATA_ERR, "No key supplied. Auto increment not currently supported."));

If autoIncrement is true and a key is supplied, it's an error..not something where we should ignore the key.

Add a fixme to implement this for realz.

[Hans Wennborg]

(In reply to comment #2)
> (From update of attachment 74872 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=74872&action=review
> 
> close
> 
> > LayoutTests/ChangeLog:11
> > +        Test that failing to supply a key when inserting data in an
> 
> Also test that supplying one when autoIncrement is set produces an error.
Since we now decided autoIncrement should default to false, I think it should be sufficient to throw an exception if the user tries to set it to true?

> 
> > LayoutTests/storage/indexeddb/objectstore-basics.html:152
> > +        testPassed("Exception thrown: " + err);
> 
> From now on, let's do this instead:
> 
> code = e.code
> shouldBe(code, THE_EXCEPTION)
Doing this.

> 
> This way when we change the codes, everything still works fine.  And it makes the output less confusing in the mean time.
> 
> > WebCore/ChangeLog:8
> > +        In createObjectStore, make autoIncrement default (as per the spec).
> 
> remove
Done

> 
> > WebCore/storage/IDBDatabase.cpp:68
> > +    bool autoIncrement = true;
> 
> revert this.  the spec needs to be fixed instead
Done (http://www.w3.org/Bugs/Public/show_bug.cgi?id=11407)

> 
> > WebCore/storage/IDBObjectStoreBackendImpl.cpp:211
> > +    } else if (!key && objectStore->autoIncrement()) {
> > +        callbacks->onError(IDBDatabaseError::create(IDBDatabaseException::DATA_ERR, "No key supplied. Auto increment not currently supported."));
> 
> If autoIncrement is true and a key is supplied, it's an error..not something where we should ignore the key.
Changing to just make having autoIncrement true forbidden for now.

> 
> Add a fixme to implement this for realz.
Done.

[Hans Wennborg]

Created attachment 74888 [details]
Patch

[Jeremy Orlow]

Comment on attachment 74888 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=74888&action=review

> LayoutTests/storage/indexeddb/create-object-store-options.html:48
> +        debug("db.createObjectStore('c', {autoIncrement: true});");

FIXME that this _should_ work..but not yet

> LayoutTests/storage/indexeddb/create-object-store-options.html:50
> +        fail('createObjectStore with autoIncrement = true should throw');

testFailed

> LayoutTests/storage/indexeddb/objectstore-basics.html:146
> +    debug("Try to insert data with a Date key:");

FIXME

> LayoutTests/storage/indexeddb/objectstore-basics.html:160
> +    result.onerror = function () {};

This should print pass and then run the next part.

[Hans Wennborg]

(In reply to comment #5)
> (From update of attachment 74888 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=74888&action=review
> 
> > LayoutTests/storage/indexeddb/create-object-store-options.html:48
> > +        debug("db.createObjectStore('c', {autoIncrement: true});");
> 
> FIXME that this _should_ work..but not yet
Done.

> 
> > LayoutTests/storage/indexeddb/create-object-store-options.html:50
> > +        fail('createObjectStore with autoIncrement = true should throw');
> 
> testFailed
Done.

> 
> > LayoutTests/storage/indexeddb/objectstore-basics.html:146
> > +    debug("Try to insert data with a Date key:");
> 
> FIXME
Done.

> 
> > LayoutTests/storage/indexeddb/objectstore-basics.html:160
> > +    result.onerror = function () {};
> 
> This should print pass and then run the next part.
Done.

[Hans Wennborg]

Created attachment 74889 [details]
Patch

[Jeremy Orlow]

Comment on attachment 74889 [details]
Patch

r=me

[Hans Wennborg]

Committed r72738: <http://trac.webkit.org/changeset/72738>