[webkit-reviews] review granted: [Bug 92857] [BlackBerry] Favicon should be Base64 encoded for cross-process passing : [Attachment 155780] Patch

[bugzilla-daemon at webkit.org]

Yong Li <yoli at rim.com> has granted Charles Wei
<charles.wei at torchmobile.com.cn>'s request for review:
Bug 92857: [BlackBerry] Favicon should be Base64 encoded for cross-process
passing
https://bugs.webkit.org/show_bug.cgi?id=92857

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

------- Additional Comments from Yong Li <yoli at rim.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=155780&action=review


r+ otherwise

> Source/WebKit/blackberry/WebCoreSupport/FrameLoaderClientBlackBerry.cpp:1193
> +    if (!bitmap->bitmap().empty()) {

early return looks better.

> Source/WebKit/blackberry/WebCoreSupport/FrameLoaderClientBlackBerry.cpp:1205
> +	   Vector<char> out;
> +	   base64Encode(static_cast<const char*>(data->data()), data->size(),
out);
> +	   char* buffer = new char[out.size() + 1];
> +	   memcpy(buffer, out.data(), out.size());
> +	   buffer[out.size()] = '\0';

out.append('\0') should work, so no need to create a new buffer and memcpy?

> Source/WebKit/blackberry/WebCoreSupport/FrameLoaderClientBlackBerry.cpp:1208
> +	   delete []buffer;

"delete[] buffer;" is more clear, if we do need this buffer. From my point of
view, Vector<char> always beats new[]/delete[]