Bug 163805

Summary: IndexedDB 2.0: Support IDBIndex name assignment
Product: WebKit Reporter: Brady Eidson <beidson>
Component: WebCore Misc.Assignee: Brady Eidson <beidson>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, alecflett, cdumez, commit-queue, esprehn+autocc, jsbell, kondapallykalyan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 160306    
Attachments:
Description Flags
Patch
none
Patch none

Brady Eidson
Reported 2016-10-21 12:38:47 PDT
IndexedDB 2.0: Support IDBIndex name assignment
Attachments
Patch (92.35 KB, patch)
2016-10-21 15:27 PDT, Brady Eidson
no flags
Patch (92.65 KB, patch)
2016-10-24 08:51 PDT, Brady Eidson
no flags
Brady Eidson
Comment 1 2016-10-21 12:39:08 PDT
Brady Eidson
Comment 2 2016-10-21 15:27:35 PDT
Alex Christensen
Comment 3 2016-10-21 19:24:44 PDT
Comment on attachment 292420 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=292420&action=review > Source/WebCore/Modules/indexeddb/client/TransactionOperation.h:197 > +template<typename MP1, typename P1, typename MP2, typename P2, typename MP3, typename P3> Could this be variadic with template<...> template<...> ? > LayoutTests/imported/w3c/web-platform-tests/IndexedDB/idbindex-rename-errors-expected.txt:6 > +FAIL IndexedDB index rename throws in an inactive transaction assert_throws: function "() => authorIndex.name = 'renamed_by_author'" threw object "InvalidStateError (DOM Exception 11): Failed set property..." that is not a DOMException TransactionInactiveError: property "code" is equal to 11, expected 0 > +FAIL IndexedDB index rename to the name of another index throws assert_throws: function "() => index.name = 'by_title'" threw object "InvalidStateError (DOM Exception 11): Failed set property..." that is not a DOMException ConstraintError: property "code" is equal to 11, expected 0 It seems like not all your errors are correct.
Alex Christensen
Comment 4 2016-10-21 19:26:49 PDT
Comment on attachment 292420 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=292420&action=review > LayoutTests/storage/indexeddb/modern/resources/index-rename-1.js:104 > + index.name = "YetAnotherName"; Try setting it to an existing name and make sure it overwrites the other existing index. Try setting it to its same name.
Brady Eidson
Comment 5 2016-10-24 08:43:14 PDT
(In reply to comment #3) > Comment on attachment 292420 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=292420&action=review > > > Source/WebCore/Modules/indexeddb/client/TransactionOperation.h:197 > > +template<typename MP1, typename P1, typename MP2, typename P2, typename MP3, typename P3> > > Could this be variadic with template<...> template<...> ? Possibly, but as we learned with the Function overhauls, a lot of edge cases come up. I already scribbled "Fully templatize TransactionOperation" on my white board. :) > > LayoutTests/imported/w3c/web-platform-tests/IndexedDB/idbindex-rename-errors-expected.txt:6 > > +FAIL IndexedDB index rename throws in an inactive transaction assert_throws: function "() => authorIndex.name = 'renamed_by_author'" threw object "InvalidStateError (DOM Exception 11): Failed set property..." that is not a DOMException TransactionInactiveError: property "code" is equal to 11, expected 0 > > +FAIL IndexedDB index rename to the name of another index throws assert_throws: function "() => index.name = 'by_title'" threw object "InvalidStateError (DOM Exception 11): Failed set property..." that is not a DOMException ConstraintError: property "code" is equal to 11, expected 0 > > It seems like not all your errors are correct. This is *known* to not be correct - We're not making an effort to 100% fix these tests yet. Doing so is clearly part of the IDB 2.0 effort. (In reply to comment #4) > Comment on attachment 292420 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=292420&action=review > > > LayoutTests/storage/indexeddb/modern/resources/index-rename-1.js:104 > > + index.name = "YetAnotherName"; > > Try setting it to an existing name and make sure it overwrites the other > existing index. We already try that, and it does not overwrite the other one; It throws an exception. > Try setting it to its same name. This will just throw an exception, as well, but should be tested. Thanks!
Brady Eidson
Comment 6 2016-10-24 08:46:50 PDT
(In reply to comment #5) > (In reply to comment #3) > > Try setting it to its same name. > > This will just throw an exception, as well, but should be tested. Thanks! Sorry I lied, no exception - It just stops the rename algorithm there. Still will add the test.
Brady Eidson
Comment 7 2016-10-24 08:51:52 PDT
WebKit Commit Bot
Comment 8 2016-10-24 09:26:58 PDT
Comment on attachment 292614 [details] Patch Clearing flags on attachment: 292614 Committed r207761: <http://trac.webkit.org/changeset/207761>
WebKit Commit Bot
Comment 9 2016-10-24 09:27:03 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.