Bug 100539 - IndexedDB: Use sequence<> instead of DOMString[] in IDL
Summary: IndexedDB: Use sequence<> instead of DOMString[] in IDL
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Joshua Bell
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-26 10:50 PDT by Joshua Bell
Modified: 2012-11-12 19:10 PST (History)
5 users (show)

See Also:


Attachments
Patch (9.75 KB, patch)
2012-10-26 10:55 PDT, Joshua Bell
no flags Details | Formatted Diff | Diff
Patch (9.28 KB, patch)
2012-11-12 13:45 PST, 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 Joshua Bell 2012-10-26 10:50:06 PDT
IndexedDB: Use sequence<> instead of DOMString[] in IDL
Comment 1 Joshua Bell 2012-10-26 10:55:49 PDT
Created attachment 170958 [details]
Patch
Comment 2 Joshua Bell 2012-10-26 10:56:06 PDT
alecflett@ - please take a look
Comment 3 Adam Barth 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.
Comment 4 Joshua Bell 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.
Comment 5 Adam Barth 2012-10-26 14:43:05 PDT
That sounds worth doing, but it's probably worth waiting until we can avoid introducing extra copies.
Comment 6 Joshua Bell 2012-11-12 13:45:22 PST
Created attachment 173711 [details]
Patch
Comment 7 Joshua Bell 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.
Comment 8 Alec Flett 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
Comment 9 Joshua Bell 2012-11-12 13:51:18 PST
Comment on attachment 173711 [details]
Patch

Well, in that case - r? cq?
Comment 10 Adam Barth 2012-11-12 13:51:53 PST
Comment on attachment 173711 [details]
Patch

ok
Comment 11 WebKit Review Bot 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>
Comment 12 WebKit Review Bot 2012-11-12 19:10:10 PST
All reviewed patches have been landed.  Closing bug.