[webkit-reviews] review denied: [Bug 29909] [V8] Chromium's implementation of ScriptString is awful for XHR's responseText : [Attachment 40834] Uses StringBuilder internally, adds ChangeLog entry

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has denied James Robinson
<jamesr at chromium.org>'s request for review:
Bug 29909: [V8] Chromium's implementation of ScriptString is awful for XHR's
responseText
https://bugs.webkit.org/show_bug.cgi?id=29909

Attachment 40834: Uses StringBuilder internally, adds ChangeLog entry
https://bugs.webkit.org/attachment.cgi?id=40834&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
This looks great.  Two minor syntactic issues:

1) There are a handful of style problems:

+}
+ScriptStringImpl::ScriptStringImpl(const String& s)

Missing a space between these two linds.

+      return v8ExternalScriptString(string);

Need four space indent here.  (I think there was another indent problem too.)

2) ScriptStringImpl has nothing to do with v8 and should be in platform/text
somewhere.  Maybe it should be called CachingStringBuilder ?