DocumentLoader::loadMainResource() should WTFMove() the passed ResourceRequest
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 26 Apr 2018 20:43:32 +0000 (20:43 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 26 Apr 2018 20:43:32 +0000 (20:43 +0000)
https://bugs.webkit.org/show_bug.cgi?id=185002

Reviewed by Youenn Fablet and Alex Christensen.

In r224852 we extracted logic from DocumentLoader::startLoadingMainResource() into a new
function DocumentLoader::loadMainResource() that could be shared by both DocumentLoader::startLoadingMainResource()
and the service worker code. As part of this extraction, DocumentLoader::loadMainResource()
takes a ResourceRequest by rvalue reference, but it never actually takes ownership of this
ResourceRequest and subsequently makes a copy of it when instantiating a CachedResourceRequest.
Instead we should WTFMove() the passed request into the CachedResourceRequest.

* loader/DocumentLoader.cpp:
(WebCore::DocumentLoader::loadMainResource):

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

Source/WebCore/ChangeLog
Source/WebCore/loader/DocumentLoader.cpp

index a3e7e8f..ac10873 100644 (file)
@@ -1,3 +1,20 @@
+2018-04-26  Daniel Bates  <dabates@apple.com>
+
+        DocumentLoader::loadMainResource() should WTFMove() the passed ResourceRequest
+        https://bugs.webkit.org/show_bug.cgi?id=185002
+
+        Reviewed by Youenn Fablet and Alex Christensen.
+
+        In r224852 we extracted logic from DocumentLoader::startLoadingMainResource() into a new
+        function DocumentLoader::loadMainResource() that could be shared by both DocumentLoader::startLoadingMainResource()
+        and the service worker code. As part of this extraction, DocumentLoader::loadMainResource()
+        takes a ResourceRequest by rvalue reference, but it never actually takes ownership of this
+        ResourceRequest and subsequently makes a copy of it when instantiating a CachedResourceRequest.
+        Instead we should WTFMove() the passed request into the CachedResourceRequest.
+
+        * loader/DocumentLoader.cpp:
+        (WebCore::DocumentLoader::loadMainResource):
+
 2018-04-26  Sihui Liu  <sihui_liu@apple.com>
 
         -[WKHTTPCookieStore deleteCookie:completionHandler:] doesn't delete cookies
index 340d092..fa588bd 100644 (file)
@@ -1732,12 +1732,12 @@ void DocumentLoader::startLoadingMainResource(ShouldContinue shouldContinue)
 void DocumentLoader::loadMainResource(ResourceRequest&& request)
 {
     static NeverDestroyed<ResourceLoaderOptions> mainResourceLoadOptions(SendCallbacks, SniffContent, BufferData, StoredCredentialsPolicy::Use, ClientCredentialPolicy::MayAskClientForCredentials, FetchOptions::Credentials::Include, SkipSecurityCheck, FetchOptions::Mode::Navigate, IncludeCertificateInfo, ContentSecurityPolicyImposition::SkipPolicyCheck, DefersLoadingPolicy::AllowDefersLoading, CachingPolicy::AllowCaching);
-    CachedResourceRequest mainResourceRequest(ResourceRequest(request), mainResourceLoadOptions);
+    CachedResourceRequest mainResourceRequest(WTFMove(request), mainResourceLoadOptions);
     if (!m_frame->isMainFrame() && m_frame->document()) {
         // If we are loading the main resource of a subframe, use the cache partition of the main document.
         mainResourceRequest.setDomainForCachePartition(*m_frame->document());
     } else {
-        auto origin = SecurityOrigin::create(request.url());
+        auto origin = SecurityOrigin::create(mainResourceRequest.url());
         origin->setStorageBlockingPolicy(frameLoader()->frame().settings().storageBlockingPolicy());
         mainResourceRequest.setDomainForCachePartition(origin->domainForCachePartition());
     }
@@ -1783,20 +1783,19 @@ void DocumentLoader::loadMainResource(ResourceRequest&& request)
 
     if (!mainResourceLoader()) {
         m_identifierForLoadWithoutResourceLoader = m_frame->page()->progress().createUniqueIdentifier();
-        frameLoader()->notifier().assignIdentifierToInitialRequest(m_identifierForLoadWithoutResourceLoader, this, request);
-        frameLoader()->notifier().dispatchWillSendRequest(this, m_identifierForLoadWithoutResourceLoader, request, ResourceResponse());
+        frameLoader()->notifier().assignIdentifierToInitialRequest(m_identifierForLoadWithoutResourceLoader, this, mainResourceRequest.resourceRequest());
+        frameLoader()->notifier().dispatchWillSendRequest(this, m_identifierForLoadWithoutResourceLoader, mainResourceRequest.resourceRequest(), ResourceResponse());
     }
 
     becomeMainResourceClient();
 
     // A bunch of headers are set when the underlying ResourceLoader is created, and m_request needs to include those.
-    if (mainResourceLoader())
-        request = mainResourceLoader()->originalRequest();
+    ResourceRequest updatedRequest = mainResourceLoader() ? mainResourceLoader()->originalRequest() : mainResourceRequest.resourceRequest();
     // If there was a fragment identifier on m_request, the cache will have stripped it. m_request should include
     // the fragment identifier, so add that back in.
-    if (equalIgnoringFragmentIdentifier(m_request.url(), request.url()))
-        request.setURL(m_request.url());
-    setRequest(request);
+    if (equalIgnoringFragmentIdentifier(m_request.url(), updatedRequest.url()))
+        updatedRequest.setURL(m_request.url());
+    setRequest(updatedRequest);
 }
 
 void DocumentLoader::cancelPolicyCheckIfNeeded()