Repository navigation
Validate exception ordering against implementations #11
Description
The various "If .... throw ...." statements were left in order from previous iterations of the spec which asserted behavior but didn't define it as formal steps.
We need to verify that the steps match the implementations in the case of conflicts, and reorder if necessary. If implementations disagree, file implementation bugs as necessary.
NOTE: Checkboxes below indicate that at least 2 implementations and the spec are in alignment.
Activity
inexorabletash commentedon Jun 1, 2015
IDBDatabase
Normal Methods
Schema Mutations
inexorabletash commentedon Jun 1, 2015
IDBObjectStore
Normal Methods
Schema Mutations
inexorabletash commentedon Jun 1, 2015
IDBIndex
inexorabletash commentedon Jun 1, 2015
IDBCursor
inexorabletash commentedon Jun 1, 2015
IDBTransaction
Tweak exception ordering for IDBTransaction.objectStore() to match FF…
odinho commentedon Jun 16, 2015
Great. At one point we should also get some tests on these things to make it simpler to implement and follow. :)
inexorabletash commentedon Jun 16, 2015
@velmont - Yes please! Writing tests for these is a bit intimidating, since some cases involve upgrades, many involve async behavior (e.g. trying to use a transaction in a non-IDB callback but before the transaction has committed), etc. And, of course, this is all about combinations of the above. Ugh!
I documented the Blink and Gecko behavior via code inspection, since that was easy.
If anyone wants to help with writing testharness.js-based tests for these please let me know and I'll assist as best I can. I'm also happy to tweak the Chrome ordering; the cursor APIs are particularly funky, but until we have tests I don't want to change anything.
inexorabletash commentedon Jun 25, 2015
Updated notes above for FF based on code inspection, following comments by @benturner #23
Reorder exception priority for deleteIndex() to match Blink/Gecko imp…
inexorabletash commentedon Sep 23, 2016
Need to update Chrome behavior descriptions following https://chromium.googlesource.com/chromium/src/+/dce205414aa1bde4125c3129fef24a8dc8ad32cc
inexorabletash commentedon Sep 23, 2016
And tests there can be upstreamed to wpt
inexorabletash commentedon Oct 12, 2016
Following a change yesterday in Chrome, FF and Cr match in all cases. \o/ But we still need tests - there are a bunch to upstream from blink (see WPT:... lines above) - mostly for "v2" stuff or where we have changed to align recently.
beidson commentedon Oct 21, 2016
Order of exceptions thrown from renaming object stores and indexes differs
For IDBIndex, the name attribute’s setter runs these steps:
...
4. If transaction is not an upgrade transaction, throw an InvalidStateError.
5. If transaction is not active, throw a TransactionInactiveError.
6. If index or index’s object store has been deleted, throw an InvalidStateError.
7. If index’s name is equal to name, terminate these steps.
8. If an index named name already exists in index’s object store, throw a ConstraintError.
...
For IDBObjectStore, the name attribute's setter runs these steps:
...
4. If store has been deleted, throw an InvalidStateError.
5. If transaction is not an upgrade transaction, throw an InvalidStateError.
6. If transaction is not active, throw a TransactionInactiveError.
7. If store’s name is equal to name, terminate these steps.
8. If an object store named name already exists in store’s database, throw a ConstraintError.
...
The "if the is deleted..." exception is in different places for each of these, but should be in the same place.
I'm implementing these in WebKit now and really don't want to follow the spec on this. Could we pick one or the other?
inexorabletash commentedon Oct 21, 2016
@bevis-tseng and @pwnall - what do you think about making the changes @beidson proposes? i.e. swap the order in one of the two methods?
pwnall commentedon Oct 23, 2016
I'm fine with either order.
While building apps, I used to get thrown off by random errors while debugging my code, so I hope we can converge on some ordering. I can change Chrome to adopt any ordering Firefox and Safari would like to use.
bevis-tseng commentedon Oct 24, 2016
FYI.
These were done in identical order in FF for both IDBObjectStore/IDBIndex [1][2]:
4. If transaction is not an upgrade transaction or if the (store|index) was deleted, throw an InvalidStateError.
5. If transaction is not active, throw a TransactionInactiveError.
6. If the name is identical, terminate these steps.
...
[1] http://searchfox.org/mozilla-central/rev/84075be5067b68dc6cb3b89f999645650e68c05b/dom/indexedDB/IDBObjectStore.cpp#2205-2219
[2] http://searchfox.org/mozilla-central/rev/84075be5067b68dc6cb3b89f999645650e68c05b/dom/indexedDB/IDBIndex.cpp#159-172
beidson commentedon Oct 24, 2016
WebKit now matches Gecko's ordering.
I suggest we codify that.
beidson commentedon Oct 24, 2016
For reference:
https://trac.webkit.org/browser/trunk/Source/WebCore/Modules/indexeddb/IDBIndex.cpp#L82
https://trac.webkit.org/browser/trunk/Source/WebCore/Modules/indexeddb/IDBObjectStore.cpp#L98
inexorabletash commentedon Jan 13, 2017
Somehow I missed that comment. Yay!
So far as I know WebKit/Gecko/Blink all match and those match the spec. If that's not the case please submit a web-platform-test case!