[webkit-reviews] review requested: [Bug 41462] Move parseDataUrl() from CURL into ResourceHandle : [Attachment 60351] Patch

[bugzilla-daemon at webkit.org]

Patrick R. Gansterer <paroga at paroga.com> has asked  for review:
Bug 41462: Move parseDataUrl() from CURL into ResourceHandle
https://bugs.webkit.org/show_bug.cgi?id=41462

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

------- Additional Comments from Patrick R. Gansterer <paroga at paroga.com>
(In reply to comment #3)
> (From update of attachment 60240 [details])
> I’m not so comfortable with putting this function into the cross platform
file because it's used by two back ends. There are many other back ends that
don't use it. Should the other back ends be using it too? Is there any other
place we could put it?
It can be used by other backends too.
Maybe it can be used at
http://trac.webkit.org/browser/trunk/WebCore/page/Page.cpp#L574 too?

> Naming the function "parse" seems wrong. Parsing is part of what it does, but
it actually performs the loading as well. I think the name should be
loadDataURL or handleDataURL. The word "parse" gives the sense of something
that would output a parsed result, not something that would make actual
loading-related calls to the client.
I didn't liked the name too. ;-)

> Calling latin1() twice on the same string seems like a mistake. That ends up
allocating a CString twice.
I created an overload for base64Decode at bug 41510.

> What data URL test cases do we have?
DataURLs are tested indirect, because other test use it (e.g. embedding
images).

Addressd the other points too.