Bug 152694 – [WinCairo] Download should use header values from provided request object.

• Component: WebKit Misc.
• Status: RESOLVED FIXED

We currently ignore the header values in the request object, but we shouldn't do that.

Created attachment 268201 [details]
Patch

[Alex Christensen]

Comment on attachment 268201 [details]
Patch

It would be better to just iterate the existing headers, not all possible headers.  What if there is an unrecognized header?

(In reply to comment #2)
> Comment on attachment 268201 [details]
> Patch
> 
> It would be better to just iterate the existing headers, not all possible
> headers.  What if there is an unrecognized header?

Thanks for looking into this :) As far as I can see, the IWebURLRequest interface does not support iterating over existing headers, but maybe I've missed something? It is possible to get all headers as an IPropertyBag object, but this type only seems to support reading known header names.

[Alex Christensen]

Comment on attachment 268201 [details]
Patch

I think this is a bad approach.  I think this should do something more like WebDownload::initWithRequest in WebDownloadCFNet.cpp instead

(In reply to comment #4)
> Comment on attachment 268201 [details]
> Patch
> 
> I think this is a bad approach.  I think this should do something more like
> WebDownload::initWithRequest in WebDownloadCFNet.cpp instead

Thanks, looks good, I'll update the patch :)

Created attachment 268222 [details]
Patch

[Alex Christensen]

Comment on attachment 268222 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=268222&action=review

r=me

> Source/WebKit/win/WebDownloadCurl.cpp:105
> +    init(nullptr, resourceRequest, ResourceResponse(), delegate);

this ResourceResponse() might cause some problems.

(In reply to comment #7)
> Comment on attachment 268222 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=268222&action=review
> 
> r=me
> 
> > Source/WebKit/win/WebDownloadCurl.cpp:105
> > +    init(nullptr, resourceRequest, ResourceResponse(), delegate);
> 
> this ResourceResponse() might cause some problems.

Thanks for reviewing :) I don't think the ResourceResponse object is used, is that what you mean?

[Alex Christensen]

We might run into some code somewhere where we want to use the ResourceResponse object and it's not a meaningful object.  If we run into this, we'll fix it in the future.

(In reply to comment #9)
> We might run into some code somewhere where we want to use the
> ResourceResponse object and it's not a meaningful object.  If we run into
> this, we'll fix it in the future.

Sounds good.

Committed r194581: <http://trac.webkit.org/changeset/194581>