RESOLVED FIXED 100539
IndexedDB: Use sequence<> instead of DOMString[] in IDL 
https://bugs.webkit.org/show_bug.cgi?id=100539
Summary IndexedDB: Use sequence<> instead of DOMString[] in IDL
Joshua Bell
Reported 2012-10-26 10:50:06 PDT
IndexedDB: Use sequence<> instead of DOMString[] in IDL
Attachments
Patch (9.75 KB, patch)
2012-10-26 10:55 PDT, Joshua Bell 		no flags
DetailsFormatted DiffDiff
Patch (9.28 KB, patch)
2012-11-12 13:45 PST, Joshua Bell 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Joshua Bell
Comment 1 2012-10-26 10:55:49 PDT
Created attachment 170958 [details] Patch
Joshua Bell
Comment 2 2012-10-26 10:56:06 PDT
alecflett@ - please take a look
Adam Barth
Comment 3 2012-10-26 14:10:44 PDT
Comment on attachment 170958 [details] Patch What benefit does this change have? It looks like all you've done is introduce an extra copy operation.
Joshua Bell
Comment 4 2012-10-26 14:37:53 PDT
Sorry, needs context - The DOMString[] support in the binding code is a hack that treats it as DOMStringList. This predates sequence<T> support. I'd like to remove the hack, and removing uses of DOMString[] is part of it. This can wait until the other bugs land and the copy is unnecessary, and/or I can bundle it with more DOMString[] removal/cleanup. Either way, I'll park this for now as it's not useful on its own.
Adam Barth
Comment 5 2012-10-26 14:43:05 PDT
That sounds worth doing, but it's probably worth waiting until we can avoid introducing extra copies.
Joshua Bell
Comment 6 2012-11-12 13:45:22 PST
Created attachment 173711 [details] Patch
Joshua Bell
Comment 7 2012-11-12 13:46:08 PST
Updated the patch now that the copy is no longer needed, but it's still not urgent to land this, and can be done in conjunction with other DOMString[] cleanup.
Alec Flett
Comment 8 2012-11-12 13:50:36 PST
I'd love to see this land sooner rather than later, since it's a working patch right now...LGTM
Joshua Bell
Comment 9 2012-11-12 13:51:18 PST
Comment on attachment 173711 [details] Patch Well, in that case - r? cq?
Adam Barth
Comment 10 2012-11-12 13:51:53 PST
Comment on attachment 173711 [details] Patch ok
WebKit Review Bot
Comment 11 2012-11-12 19:10:05 PST
Comment on attachment 173711 [details] Patch Clearing flags on attachment: 173711 Committed r134342: <http://trac.webkit.org/changeset/134342>
WebKit Review Bot
Comment 12 2012-11-12 19:10:10 PST
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.