[webkit-reviews] review denied: [Bug 31415] [v8] do not copy data twice when converting short v8 string into WebCore::String : [Attachment 43080] Addressing Vitaly's comments

[bugzilla-daemon at webkit.org]

Darin Fisher (:fishd, Google) <fishd at chromium.org> has denied anton muhin
<antonm at chromium.org>'s request for review:
Bug 31415: [v8] do not copy data twice when converting short v8 string into
WebCore::String
https://bugs.webkit.org/show_bug.cgi?id=31415

Attachment 43080: Addressing Vitaly's comments
https://bugs.webkit.org/attachment.cgi?id=43080&action=review

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
> Index: WebCore/bindings/v8/V8Binding.cpp

> +template <class S> struct StringTraits
> +{
> +    static S getFromResource(WebCoreStringResource* resource);
> +
> +    static S convertFromV8(v8::Handle<v8::String> v8String, int length);

how about naming these fromStringResource and fromV8String?


> +template<>
> +struct StringTraits<String>
> +{
> +    static String getFromResource(WebCoreStringResource* resource) {
> +	   return resource->webcoreString();
> +    }

please place the opening bracket on a new line per webkit style.  same goes
for the other functions in this patch.


> +    static AtomicString convertFromV8(v8::Handle<v8::String> v8String, int
length) {
...
> +	       return AtomicString(inlineBuffer, length);
> +	   } else {

nit: no need for else after return.


R- because of style issues.  otherwise, the substance looks good.