[webkit-reviews] review canceled: [Bug 100580] buildHTTPHeaders() should use a StringBuilder instead of a Vector<UChar> : [Attachment 171063] Patch

[bugzilla-daemon at webkit.org]

Michael Saboff <msaboff at apple.com> has canceled Michael Saboff
<msaboff at apple.com>'s request for review:
Bug 100580: buildHTTPHeaders() should use a StringBuilder instead of a
Vector<UChar>
https://bugs.webkit.org/show_bug.cgi?id=100580

Attachment 171063: Patch
https://bugs.webkit.org/attachment.cgi?id=171063&action=review

------- Additional Comments from Michael Saboff <msaboff at apple.com>
(In reply to comment #2)
> (From update of attachment 171063 [details])
> View in context:
https://bugs.webkit.org/attachment.cgi?id=171063&action=review
> 
> > Source/WebKit2/ChangeLog:4
> > +	     WKStringCopyCFString() should directly use 8 bit Strings data
instead of up converting
> > +	     https://bugs.webkit.org/show_bug.cgi?id=100579
> 
> This is the wrong bug title and number.

Indeed!  Fixed this.

> > Source/WebKit2/WebProcess/Plugins/PluginView.cpp:167
> >	 String separator(": ");
> 
> Is there a benefit to creating this String rather than just appending the
characters directly - stringBuilder.append(": ", 2) ?

I'm inclined to keep separator as a named string variable.  The append(": ", 2)
construct doesn't say what it is for and could be prone to errors if either the
string or length was changed without changing the other.  This is a weak
argument and I could be convinced the other way.