Bug 92857 – [BlackBerry] Favicon should be Base64 encoded for cross-process passing

• Component: WebKit BlackBerry
• Status: RESOLVED FIXED

[Charles Wei]

Internal tracking No. 186242

Favicon should be Base64 encoded for cross-process passing to Chrome which is in another process.

[Charles Wei]

Created attachment 155780 [details]
Patch

[Yong Li]

Comment on attachment 155780 [details]
Patch

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[]

[George Staikos]

Comment on attachment 155780 [details]
Patch

I agree with your comments on the buffer and had this chat with Charles earlier today.  r- for these comments in fact.

[Charles Wei]

Created attachment 155957 [details]
Patch

[George Staikos]

Comment on attachment 155957 [details]
Patch

Better

[Charles Wei]

Comment on attachment 155957 [details]
Patch

Thanks, George and Yong, for the comments.

[WebKit Review Bot]

Comment on attachment 155957 [details]
Patch

Clearing flags on attachment: 155957

Committed r124403: <http://trac.webkit.org/changeset/124403>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.