Move ResourceRequest construction out of SubresourceLoader
authorjaphet@chromium.org <japhet@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 17 Oct 2012 22:55:33 +0000 (22:55 +0000)
committerjaphet@chromium.org <japhet@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 17 Oct 2012 22:55:33 +0000 (22:55 +0000)
https://bugs.webkit.org/show_bug.cgi?id=99627

Reviewed by Adam Barth.

CachedResource::load() fills out a bunch of http headers.
SubresourceLoader::create() adds a bunch more. Merge them.
Note that this merge requires a bit more care in CachedRawResource::canReuse(),
because more headers are set directly on CachedResource::m_resourceRequest, rather
than on a copy of it.

No new tests, no functionality change intended.

* loader/SubresourceLoader.cpp:
(WebCore::SubresourceLoader::create):
* loader/cache/CachedRawResource.cpp:
(WebCore::shouldIgnoreHeaderForCacheReuse):
(WebCore):
(WebCore::CachedRawResource::canReuse):
* loader/cache/CachedResource.cpp:
(WebCore::CachedResource::failBeforeStarting):
(WebCore):
(WebCore::CachedResource::addAdditionalRequestHeaders):
(WebCore::CachedResource::load):
* loader/cache/CachedResource.h:
(CachedResource):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@131660 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/loader/SubresourceLoader.cpp
Source/WebCore/loader/cache/CachedRawResource.cpp
Source/WebCore/loader/cache/CachedResource.cpp
Source/WebCore/loader/cache/CachedResource.h

index ca7f21f..ab76189 100644 (file)
@@ -1,3 +1,32 @@
+2012-10-17  Nate Chapin  <japhet@chromium.org>
+
+        Move ResourceRequest construction out of SubresourceLoader
+        https://bugs.webkit.org/show_bug.cgi?id=99627
+
+        Reviewed by Adam Barth.
+
+        CachedResource::load() fills out a bunch of http headers.
+        SubresourceLoader::create() adds a bunch more. Merge them.
+        Note that this merge requires a bit more care in CachedRawResource::canReuse(),
+        because more headers are set directly on CachedResource::m_resourceRequest, rather
+        than on a copy of it.
+
+        No new tests, no functionality change intended.
+
+        * loader/SubresourceLoader.cpp:
+        (WebCore::SubresourceLoader::create):
+        * loader/cache/CachedRawResource.cpp:
+        (WebCore::shouldIgnoreHeaderForCacheReuse):
+        (WebCore):
+        (WebCore::CachedRawResource::canReuse):
+        * loader/cache/CachedResource.cpp:
+        (WebCore::CachedResource::failBeforeStarting):
+        (WebCore):
+        (WebCore::CachedResource::addAdditionalRequestHeaders):
+        (WebCore::CachedResource::load):
+        * loader/cache/CachedResource.h:
+        (CachedResource):
+
 2012-10-17  Anders Carlsson  <andersca@apple.com>
 
         Clean up Vector.h
index 36b3676..5097579 100644 (file)
 #include "Document.h"
 #include "DocumentLoader.h"
 #include "Frame.h"
-#include "FrameLoader.h"
 #include "Logging.h"
 #include "MemoryCache.h"
 #include "ResourceBuffer.h"
-#include "SecurityOrigin.h"
-#include "SecurityPolicy.h"
 #include "WebCoreMemoryInstrumentation.h"
 #include <wtf/RefCountedLeakCounter.h>
 #include <wtf/StdLibExtras.h>
@@ -83,40 +80,8 @@ SubresourceLoader::~SubresourceLoader()
 
 PassRefPtr<SubresourceLoader> SubresourceLoader::create(Frame* frame, CachedResource* resource, const ResourceRequest& request, const ResourceLoaderOptions& options)
 {
-    if (!frame)
-        return 0;
-
-    FrameLoader* frameLoader = frame->loader();
-    if (options.securityCheck == DoSecurityCheck && (frameLoader->state() == FrameStateProvisional || !frameLoader->activeDocumentLoader() || frameLoader->activeDocumentLoader()->isStopping()))
-        return 0;
-
-    ResourceRequest newRequest = request;
-
-    // Note: We skip the Content-Security-Policy check here because we check
-    // the Content-Security-Policy at the CachedResourceLoader layer so we can
-    // handle different resource types differently.
-
-    String outgoingReferrer;
-    String outgoingOrigin;
-    if (request.httpReferrer().isNull()) {
-        outgoingReferrer = frameLoader->outgoingReferrer();
-        outgoingOrigin = frameLoader->outgoingOrigin();
-    } else {
-        outgoingReferrer = request.httpReferrer();
-        outgoingOrigin = SecurityOrigin::createFromString(outgoingReferrer)->toString();
-    }
-
-    outgoingReferrer = SecurityPolicy::generateReferrerHeader(frame->document()->referrerPolicy(), request.url(), outgoingReferrer);
-    if (outgoingReferrer.isEmpty())
-        newRequest.clearHTTPReferrer();
-    else if (!request.httpReferrer())
-        newRequest.setHTTPReferrer(outgoingReferrer);
-    FrameLoader::addHTTPOriginIfNeeded(newRequest, outgoingOrigin);
-
-    frameLoader->addExtraFieldsToSubresourceRequest(newRequest);
-
     RefPtr<SubresourceLoader> subloader(adoptRef(new SubresourceLoader(frame, resource, options)));
-    if (!subloader->init(newRequest))
+    if (!subloader->init(request))
         return 0;
     return subloader.release();
 }
index 6d6c129..194b15f 100644 (file)
@@ -129,6 +129,24 @@ void CachedRawResource::setDefersLoading(bool defers)
         m_loader->setDefersLoading(defers);
 }
 
+static bool shouldIgnoreHeaderForCacheReuse(AtomicString headerName)
+{
+    // FIXME: This list of headers that don't affect cache policy almost certainly isn't complete.
+    DEFINE_STATIC_LOCAL(HashSet<AtomicString>, m_headers, ());
+    if (m_headers.isEmpty()) {
+        m_headers.add("Accept");
+        m_headers.add("Cache-Control");
+        m_headers.add("If-Modified-Since");
+        m_headers.add("If-None-Match");
+        m_headers.add("Origin");
+        m_headers.add("Pragma");
+        m_headers.add("Purpose");
+        m_headers.add("Referer");
+        m_headers.add("User-Agent");
+    }
+    return m_headers.contains(headerName);
+}
+
 bool CachedRawResource::canReuse(const ResourceRequest& newRequest) const
 {
     if (m_options.shouldBufferData == DoNotBufferData)
@@ -143,20 +161,24 @@ bool CachedRawResource::canReuse(const ResourceRequest& newRequest) const
     if (m_resourceRequest.allowCookies() != newRequest.allowCookies())
         return false;
 
-    // Ensure all headers match the existing headers before continuing.
-    // Note that only headers set by our client will be present in either
-    // ResourceRequest, since SubresourceLoader creates a separate copy
-    // for its purposes.
-    // FIXME: There might be some headers that shouldn't block reuse.
+    // Ensure most headers match the existing headers before continuing.
+    // Note that the list of ignored headers includes some headers explicitly related to caching.
+    // A more detailed check of caching policy will be performed later, this is simply a list of
+    // headers that we might permit to be different and still reuse the existing CachedResource.
     const HTTPHeaderMap& newHeaders = newRequest.httpHeaderFields();
     const HTTPHeaderMap& oldHeaders = m_resourceRequest.httpHeaderFields();
-    if (newHeaders.size() != oldHeaders.size())
-        return false;
 
     HTTPHeaderMap::const_iterator end = newHeaders.end();
     for (HTTPHeaderMap::const_iterator i = newHeaders.begin(); i != end; ++i) {
         AtomicString headerName = i->key;
-        if (i->value != oldHeaders.get(headerName))
+        if (!shouldIgnoreHeaderForCacheReuse(headerName) && i->value != oldHeaders.get(headerName))
+            return false;
+    }
+
+    end = oldHeaders.end();
+    for (HTTPHeaderMap::const_iterator i = oldHeaders.begin(); i != end; ++i) {
+        AtomicString headerName = i->key;
+        if (!shouldIgnoreHeaderForCacheReuse(headerName) && i->value != newHeaders.get(headerName))
             return false;
     }
     return true;
index d60bd62..703c428 100755 (executable)
@@ -32,6 +32,7 @@
 #include "CachedResourceLoader.h"
 #include "CrossOriginAccessControl.h"
 #include "Document.h"
+#include "DocumentLoader.h"
 #include "FrameLoaderClient.h"
 #include "InspectorInstrumentation.h"
 #include "KURL.h"
@@ -40,6 +41,8 @@
 #include "ResourceBuffer.h"
 #include "ResourceHandle.h"
 #include "ResourceLoadScheduler.h"
+#include "SecurityOrigin.h"
+#include "SecurityPolicy.h"
 #include "SubresourceLoader.h"
 #include "WebCoreMemoryInstrumentation.h"
 #include <wtf/CurrentTime.h>
@@ -190,8 +193,55 @@ CachedResource::~CachedResource()
         m_owningCachedResourceLoader->removeCachedResource(this);
 }
 
+void CachedResource::failBeforeStarting()
+{
+    // FIXME: What if resources in other frames were waiting for this revalidation?
+    LOG(ResourceLoading, "Cannot start loading '%s'", url().string().latin1().data());
+    if (m_resourceToRevalidate) 
+        memoryCache()->revalidationFailed(this); 
+    error(CachedResource::LoadError);
+}
+
+void CachedResource::addAdditionalRequestHeaders(CachedResourceLoader* cachedResourceLoader)
+{
+    // Note: We skip the Content-Security-Policy check here because we check
+    // the Content-Security-Policy at the CachedResourceLoader layer so we can
+    // handle different resource types differently.
+
+    FrameLoader* frameLoader = cachedResourceLoader->frame()->loader();
+    String outgoingReferrer;
+    String outgoingOrigin;
+    if (m_resourceRequest.httpReferrer().isNull()) {
+        outgoingReferrer = frameLoader->outgoingReferrer();
+        outgoingOrigin = frameLoader->outgoingOrigin();
+    } else {
+        outgoingReferrer = m_resourceRequest.httpReferrer();
+        outgoingOrigin = SecurityOrigin::createFromString(outgoingReferrer)->toString();
+    }
+
+    outgoingReferrer = SecurityPolicy::generateReferrerHeader(cachedResourceLoader->document()->referrerPolicy(), m_resourceRequest.url(), outgoingReferrer);
+    if (outgoingReferrer.isEmpty())
+        m_resourceRequest.clearHTTPReferrer();
+    else if (!m_resourceRequest.httpReferrer())
+        m_resourceRequest.setHTTPReferrer(outgoingReferrer);
+    FrameLoader::addHTTPOriginIfNeeded(m_resourceRequest, outgoingOrigin);
+
+    frameLoader->addExtraFieldsToSubresourceRequest(m_resourceRequest);
+}
+
 void CachedResource::load(CachedResourceLoader* cachedResourceLoader, const ResourceLoaderOptions& options)
 {
+    if (!cachedResourceLoader->frame()) {
+        failBeforeStarting();
+        return;
+    }
+
+    FrameLoader* frameLoader = cachedResourceLoader->frame()->loader();
+    if (options.securityCheck == DoSecurityCheck && (frameLoader->state() == FrameStateProvisional || !frameLoader->activeDocumentLoader() || frameLoader->activeDocumentLoader()->isStopping())) {
+        failBeforeStarting();
+        return;
+    }
+
     m_options = options;
     m_loading = true;
 
@@ -225,14 +275,11 @@ void CachedResource::load(CachedResourceLoader* cachedResourceLoader, const Reso
         m_resourceRequest.setHTTPHeaderField("Purpose", "prefetch");
 #endif
     m_resourceRequest.setPriority(loadPriority());
-    
-    m_loader = resourceLoadScheduler()->scheduleSubresourceLoad(cachedResourceLoader->document()->frame(), this, m_resourceRequest, m_resourceRequest.priority(), options);
+    addAdditionalRequestHeaders(cachedResourceLoader);
+
+    m_loader = resourceLoadScheduler()->scheduleSubresourceLoad(cachedResourceLoader->frame(), this, m_resourceRequest, m_resourceRequest.priority(), options);
     if (!m_loader) {
-        // FIXME: What if resources in other frames were waiting for this revalidation?
-        LOG(ResourceLoading, "Cannot start loading '%s'", url().string().latin1().data());
-        if (m_resourceToRevalidate) 
-            memoryCache()->revalidationFailed(this); 
-        error(CachedResource::LoadError);
+        failBeforeStarting();
         return;
     }
 
index 9c5d365..c3d093a 100644 (file)
@@ -255,6 +255,8 @@ public:
 protected:
     virtual void checkNotify();
 
+    virtual void addAdditionalRequestHeaders(CachedResourceLoader*);
+
     void setEncodedSize(unsigned);
     void setDecodedSize(unsigned);
     void didAccessDecodedData(double timeStamp);
@@ -301,6 +303,8 @@ private:
     double currentAge() const;
     double freshnessLifetime() const;
 
+    void failBeforeStarting();
+
     RefPtr<CachedMetadata> m_cachedMetadata;
 
     double m_lastDecodedAccessTime; // Used as a "thrash guard" in the cache