[webkit-reviews] review granted: [Bug 31168] Add [HintAtomic] to IDLs to remove unnecessary string conversions in generated V8 DOM bindings : [Attachment 42907] patch 2

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Nov 10 16:46:01 PST 2009


Eric Seidel <eric at webkit.org> has granted Jens Alfke <snej at chromium.org>'s
request for review:
Bug 31168: Add [HintAtomic] to IDLs to remove unnecessary string conversions in
generated V8 DOM bindings
https://bugs.webkit.org/show_bug.cgi?id=31168

Attachment 42907: patch 2
https://bugs.webkit.org/attachment.cgi?id=42907&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
I think svn-create-patch will order the patches so the ChangeLog is at the top.
 (It will also handle binary data if you ever need that.)

I think this change is a good idea.  I think we should consider getting rid of
implicit String->AtomicString conversions all together.

I'm not sure the naming is perfect here. I'm not sure I have any better
suggestion besides HintAtomic.

Maybe NO_IMPLICIT_ATOMICSTRING should be DISABLE_IMPLICIT_ATOMICSTRING?

ATOMICSTRING_CONVERSION seems like it might make more sense as
EXPENSIVE_CONVERSION since it's hinting about the constructors.

Do we know how much work it would be to remove these implicit conversions all
together?  Probably more work than it makes sense to try and do in one bug.

Clearly those SVG toString  methods should be changed to return String instead
of AtomicString. :)  We should file a bug about that.

LGTM.  Please wait for a bit for Darin to see this and have a chance to comment
if he wants to before you commit this.


More information about the webkit-reviews mailing list