[webkit-reviews] review denied: [Bug 44444] Set the ResourceResponse HTTP headers in RessourceHandleWin : [Attachment 65144] Patch

[bugzilla-daemon at webkit.org]

Adam Roben (aroben) <aroben at apple.com> has denied Patrick R. Gansterer
<paroga at paroga.com>'s request for review:
Bug 44444: Set the ResourceResponse HTTP headers in RessourceHandleWin
https://bugs.webkit.org/show_bug.cgi?id=44444

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

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
> -	   client()->didReceiveData(this, buffer, buffers.dwBufferLength, 0);
> +
> +	   if (ResourceHandleClient* resourceHandleClient = client())
> +	       resourceHandleClient->didReceiveData(this, buffer,
buffers.dwBufferLength, 0);
>	   buffers.dwBufferLength = bufferSize;
>      }
>  
> @@ -272,8 +293,24 @@ void ResourceHandle::onRequestComplete(L
>	   InternetCloseHandle(d->m_secondaryHandle);
>      InternetCloseHandle(d->m_resourceHandle);
>  
> -    client()->didFinishLoading(this);
> -    delete this;
> +    if (ResourceHandleClient* resourceHandleClient = client())
> +	   resourceHandleClient->didFinishLoading(this);
> +
> +    deref(); // balances ref in start
> +}

These changes seem completely separate from what the bug claims to be fixing. I
think you should split them into a separate bug/patch.

> +String ResourceHandle::queryHTTPHeader(DWORD infoLevel)
> +{
> +    DWORD bufferSize = 0;
> +    HttpQueryInfoW(d->m_requestHandle, infoLevel, 0, &bufferSize, 0);
> +
> +    UChar* characters;
> +    String result = String::createUninitialized(bufferSize / sizeof(UChar),
characters);
> +
> +    if (HttpQueryInfoW(d->m_requestHandle, infoLevel, characters,
&bufferSize, 0))
> +	   return result.left(result.length() - 1); // Remove NullTermination.

I think using a Vector<UChar> and String::adopt would be more efficient, since
you won't have to copy the whole string at the end.

It would be clearer to have an early return if the second call to
HttpQueryInfoW fails.

It might be nicer for this function to be a non-member function with internal
linkage.