ResourceLoaderOptions also must be updated by updateRequestForAccessControl()

Source/core/loader/DocumentThreadableLoader.cpp

Index: Source/core/loader/DocumentThreadableLoader.cpp
diff --git a/Source/core/loader/DocumentThreadableLoader.cpp b/Source/core/loader/DocumentThreadableLoader.cpp
index 399abe64ff7f54abe96f17223c524fd269a76411..5755c9b908715f4baa03bee9eb0516ed7047779a 100644
--- a/Source/core/loader/DocumentThreadableLoader.cpp
+++ b/Source/core/loader/DocumentThreadableLoader.cpp
@@ -94,7 +94,7 @@ DocumentThreadableLoader::DocumentThreadableLoader(Document& document, Threadabl
     }
 
     if (m_sameOriginRequest || m_options.crossOriginRequestPolicy == AllowCrossOriginRequests) {
-        loadRequest(request);
+        loadRequest(request, m_resourceLoaderOptions);
         return;
     }
 
@@ -118,21 +118,27 @@ void DocumentThreadableLoader::makeCrossOriginAccessRequest(const ResourceReques
         }
 
         ResourceRequest crossOriginRequest(request);
+        ResourceLoaderOptions crossOriginOptions(m_resourceLoaderOptions);
         updateRequestForAccessControl(crossOriginRequest, securityOrigin(), m_allowCredentials);
-        loadRequest(crossOriginRequest);
+        loadRequest(crossOriginRequest, crossOriginOptions);
     } else {
         m_simpleRequest = false;
 
         OwnPtr<ResourceRequest> crossOriginRequest = adoptPtr(new ResourceRequest(request));
+        OwnPtr<ResourceLoaderOptions> crossOriginOptions = adoptPtr(new ResourceLoaderOptions(m_resourceLoaderOptions));
         // Do not set the Origin header for preflight requests.
         updateRequestForAccessControl(*crossOriginRequest, 0, m_allowCredentials);
         m_actualRequest = crossOriginRequest.release();
+        m_actualOptions = crossOriginOptions.release();
 
         if (CrossOriginPreflightResultCache::shared().canSkipPreflight(securityOrigin()->toString(), m_actualRequest->url(), m_allowCredentials, m_actualRequest->httpMethod(), m_actualRequest->httpHeaderFields())) {
             loadActualRequest();
         } else {
             ResourceRequest preflightRequest = createAccessControlPreflightRequest(*m_actualRequest, securityOrigin());
-            loadRequest(preflightRequest);
+            // Create a ResourceLoaderOptions for preflight.
+            ResourceLoaderOptions preflightOptions = *m_actualOptions;

[Nate Chapin, 2014/06/03 16:24:56]

m_acutalOptions.get() is how we usually dereference an OwnPtr, but I guess this
is consistent with the rest of the file.

[tyoshino (SeeGerritForStatus), 2014/06/04 05:50:35]

Oh, ok. Fixed the rest too instead.

+            preflightOptions.allowCredentials = DoNotAllowStoredCredentials;
+            loadRequest(preflightRequest, preflightOptions);
         }
     }
 }
@@ -376,12 +382,14 @@ void DocumentThreadableLoader::loadActualRequest()
 {
     OwnPtr<ResourceRequest> actualRequest;
     actualRequest.swap(m_actualRequest);
+    OwnPtr<ResourceLoaderOptions> actualOptions;
+    actualOptions.swap(m_actualOptions);
 
     actualRequest->setHTTPOrigin(securityOrigin()->toAtomicString());
 
     clearResource();
 
-    loadRequest(*actualRequest);
+    loadRequest(*actualRequest, *actualOptions);
 }
 
 void DocumentThreadableLoader::handlePreflightFailure(const String& url, const String& errorDescription)
@@ -394,16 +402,16 @@ void DocumentThreadableLoader::handlePreflightFailure(const String& url, const S
     m_client->didFailAccessControlCheck(error);
 }
 
-void DocumentThreadableLoader::loadRequest(const ResourceRequest& request)
+void DocumentThreadableLoader::loadRequest(const ResourceRequest& request, ResourceLoaderOptions resourceLoaderOptions)
 {
     // Any credential should have been removed from the cross-site requests.
     const KURL& requestURL = request.url();
     ASSERT(m_sameOriginRequest || requestURL.user().isEmpty());
     ASSERT(m_sameOriginRequest || requestURL.pass().isEmpty());
 
-    ResourceLoaderOptions resourceLoaderOptions = m_resourceLoaderOptions;
     // Update resourceLoaderOptions with enforced values.
-    resourceLoaderOptions.allowCredentials = m_allowCredentials;
+    if (m_allowCredentials == DoNotAllowStoredCredentials)
+        resourceLoaderOptions.allowCredentials = DoNotAllowStoredCredentials;

[Nate Chapin, 2014/06/03 16:24:56]

I take it there are cases where m_allowCredentials will allow credenetials, but
resourceLoaderOptions.allowCredentials will not? When exactly is this the case?

[tyoshino (SeeGerritForStatus), 2014/06/04 05:50:35]

This code need to be like this because for preflight request we set
resourceLoaderOptions to DoNot and we don't want it to be overridden by
m_allowCredentials here.

m_allowCredentials is only set to DoNotAllowStoredCredentials at L233 when
redirect happens. Otherwise, it's the same as the original allowCredentials
value set in m_resourceLoaderOptions.

I replaced m_allowCredentials with a boolean named
"forceDoNotAllowStoredCredentials" to make it clear.

     resourceLoaderOptions.securityOrigin = m_securityOrigin;
     if (m_async) {
         if (m_actualRequest) {