CachedResource::load() fills out a bunch of http headers. SubresourceLoader::create() adds a bunch more. I don't think there's a good reason for these to be set in 2 different places. Merging them also means one less copy. By putting everything into a helper in CachedResource, it would also be cleaner to override what headers are set for different types of resources which will be useful for adding main resources to the cache. The downside is that it complicates CachedRawResource::canReuse(). Because a bunch of additional headers are now set on CachedResource::m_resourceRequest, we will need to be a little more careful in terms of what headers force a load from network for CachedRawResources.
Created attachment 169234 [details] patch
Comment on attachment 169234 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=169234&action=review > Source/WebCore/ChangeLog:10 > + Note that this merge requires a bit more care in CachedRawReosurce::canReuse(), typo: CachedRawReosurce
Comment on attachment 169234 [details] patch This looks reasonable, but this code melts my brain.
Comment on attachment 169234 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=169234&action=review > Source/WebCore/loader/cache/CachedRawResource.cpp:132 > +static bool shouldIgnoreHeaderForCacheReuse(AtomicString headerName) Where did this list of headers come from?
(In reply to comment #4) > (From update of attachment 169234 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=169234&action=review > > > Source/WebCore/loader/cache/CachedRawResource.cpp:132 > > +static bool shouldIgnoreHeaderForCacheReuse(AtomicString headerName) > > Where did this list of headers come from? It's the new headers that are set on CachedResource::m_resourceRequest, that are currently set directly or indirectly by SubsourceLoader::create() (the indirect ones are from FrameLoader::addExtraFieldsToRequest()).
Comment on attachment 169234 [details] patch Ok. The list of headers seems a bit hacky...
(In reply to comment #6) > (From update of attachment 169234 [details]) > Ok. The list of headers seems a bit hacky... Yep, but I didn't have a better idea. :(
Neither do I.
Created attachment 169271 [details] Patch for landing
Comment on attachment 169271 [details] Patch for landing Clearing flags on attachment: 169271 Committed r131660: <http://trac.webkit.org/changeset/131660>
All reviewed patches have been landed. Closing bug.