ResourceRequestBase provides both setHTTPHeaderField() and addHTTPHeaderField(). However, ResourceResponseBase only provides setHTTPHeaderField(). This is a bit inconsistent. As a result, the addHTTPHeaderField() functionality's implementation is duplicated in several ports (at least chromium and soup). I think it would be beneficial to implement addHTTPHeaderField() once in ResourceResponseBase to avoid code duplication and make the ResourceResponse's API more consistent with the ResourceRequest one.
Created attachment 188385 [details] Patch
Comment on attachment 188385 [details] Patch Clearing flags on attachment: 188385 Committed r142902: <http://trac.webkit.org/changeset/142902>
All reviewed patches have been landed. Closing bug.
I do not think that the patch is right. There should be is no expectation of consistency between ResourceRequest and ResourceResponse - they are totally asymmetric by design. The response comes form the network, and no one should be adding header fields to it, which is why there should be no public addHTTPHeaderField function.
At least in the case of the soup backend, updateFromSoupMessageHeaders is used to update a ResourceRequest with information from the network.
This patch just moves duplicate code into a location where it can be shared. > The response comes form the network, and no one should be adding header fields to it, which is why there should be no public addHTTPHeaderField function. Chromium as a public API for adding HTTP header fields to ResourceResponses. We can go look at the callers if you want to see why they need that facility.
> At least in the case of the soup backend, updateFromSoupMessageHeaders is used to update a ResourceRequest with information from the network. There is certainly a place where a port needs to set the headers from a low level network response, bit it doesn't need a public function for this. In fact, I don't think that this patch is a big deal - we already have all kinds of setters on ResourceResponseBase because of the way blobs were implemented. But I think that strategically, ResourceResponse should be immutable, and we should be removing setters, not adding them. > Chromium as a public API for adding HTTP header fields to ResourceResponses. We can go look at the callers if you want to see why they need that facility. Interesting!
https://code.google.com/p/chromium/codesearch#search/&q=addHTTPHeaderField&sq=package:chromium&type=cs Looks like it's one of the ways the network stack builds a ResourceResponse.