Bug 44444 - Set the ResourceResponse HTTP headers in RessourceHandleWin
Summary: Set the ResourceResponse HTTP headers in RessourceHandleWin
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows XP
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 43712
  Show dependency treegraph
 
Reported: 2010-08-23 11:39 PDT by Patrick R. Gansterer
Modified: 2010-08-23 20:26 PDT (History)
2 users (show)

See Also:


Attachments
Patch (4.38 KB, patch)
2010-08-23 11:46 PDT, Patrick R. Gansterer
aroben: review-
Details | Formatted Diff | Diff
Patch (3.34 KB, patch)
2010-08-23 12:21 PDT, Patrick R. Gansterer
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick R. Gansterer 2010-08-23 11:39:44 PDT
see patch
Comment 1 Patrick R. Gansterer 2010-08-23 11:46:10 PDT
Created attachment 65144 [details]
Patch
Comment 2 Adam Roben (:aroben) 2010-08-23 11:54:18 PDT
Comment on attachment 65144 [details]
Patch

> -        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.
Comment 3 Patrick R. Gansterer 2010-08-23 12:21:25 PDT
Created attachment 65150 [details]
Patch
Comment 4 Adam Roben (:aroben) 2010-08-23 12:31:30 PDT
Comment on attachment 65150 [details]
Patch

r=me
Comment 5 WebKit Commit Bot 2010-08-23 20:26:33 PDT
Comment on attachment 65150 [details]
Patch

Clearing flags on attachment: 65150

Committed r65859: <http://trac.webkit.org/changeset/65859>
Comment 6 WebKit Commit Bot 2010-08-23 20:26:38 PDT
All reviewed patches have been landed.  Closing bug.