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

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Patrick R. Gansterer
<paroga at paroga.com>'s request for review:
Bug 41462: Move parseDataUrl() from CURL into ResourceHandle
https://bugs.webkit.org/show_bug.cgi?id=41462

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=69809&action=review

I didn’t set commit-queue+ yet because I have some comments you may wish to
address.

> WebCore/platform/network/DataURL.cpp:82
> +	   if (data.length() > 0) {

Is this check really needed? Doesn’t TextEncoding::encode already handle a
length of 0 correctly?

> WebCore/platform/network/DataURL.h:33
> +void handleDataURL(ResourceHandle* handle);

No need for the argument name “handle” here.

> WebCore/platform/network/win/ResourceHandleWin.cpp:353
> +    if (firstRequest().url().protocolIs("data")) {

It seems a shame that we have protocolIs("data") in so many places. Maybe
protocolIsData should be added to KURL.h.