[webkit-reviews] review denied: [Bug 14124] [CURL] Support data URLs : [Attachment 16876] Add data URL support

[bugzilla-daemon at webkit.org]

Anders Carlsson <andersca at apple.com> has denied Alp Toker <alp at atoker.com>'s
request for review:
Bug 14124: [CURL] Support data URLs
http://bugs.webkit.org/show_bug.cgi?id=14124

Attachment 16876: Add data URL support
http://bugs.webkit.org/attachment.cgi?id=16876&action=edit

------- Additional Comments from Anders Carlsson <andersca at apple.com>
>     unsigned len = in.size();
>     const char* data = in.data();
>

Can these just be folded into the new base64Decode call now?

>+    return base64Decode(data, len, out);
>+}
>+
>+bool base64Decode(const char* data, unsigned len, Vector<char>& out)
>+{
>+    out.clear();
>+    if (len == 0)
>+	  return true;
>+
>+    // If the input string is pathologically large, just return nothing.
>+    if (len > UINT_MAX)
>+	  return false;
>+

Why is this change needed?

>     while (len && data[len-1] == '=')
>	  --len;
> 

>+static void parseDataUrl(ResourceHandle* job)

Please name the variable "handle" instead of "job".

>+{
>+    DeprecatedString data = job->request().url().url().latin1();

Why do you need to call latin1() here? Also, we recommend against using
DeprecatedString in newly written code.

>+
>+    ASSERT(data.startsWith("data:"));

URL schemes are case insensitive so this is not correct.

>+
> void ResourceHandleManager::startJob(ResourceHandle* job)

I take it that startJob is called from the main loop.

> {
>+    KURL kurl = job->request().url();
>+    String protocol = kurl.protocol();
>+
>+    if (protocol == "data") {

URL schemes are insensitive, please use equalIgnoringCase here.

>+	  parseDataUrl(job);
>+	  return;
>+    }
>+
>     ResourceHandleInternal* d = job->getInternal();