Split the constructor of ThreadableLoader into two methods (ctor and start())

Source/core/loader/DocumentThreadableLoader.cpp

 /*
  * Copyright (C) 2011, 2012 Google Inc. All rights reserved.
  * Copyright (C) 2013, Intel Corporation
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
@@ skipping 49 matching lines @@
 // Max number of CORS redirects handled in DocumentThreadableLoader.
 // Same number as net/url_request/url_request.cc, and
 // same number as https://fetch.spec.whatwg.org/#concept-http-fetch, Step 4.
 // FIXME: currently the number of redirects is counted and limited here and in
 // net/url_request/url_request.cc separately.
 static const int kMaxCORSRedirects = 20;
 
 void DocumentThreadableLoader::loadResourceSynchronously(Document& document, const ResourceRequest& request, ThreadableLoaderClient& client, const ThreadableLoaderOptions& options, const ResourceLoaderOptions& resourceLoaderOptions)
 {
     // The loader will be deleted as soon as this function exits.
-    RefPtr<DocumentThreadableLoader> loader = adoptRef(new DocumentThreadableLoader(document, &client, LoadSynchronously, request, options, resourceLoaderOptions));
+    RefPtr<DocumentThreadableLoader> loader = adoptRef(new DocumentThreadableLoader(document, &client, LoadSynchronously, options, resourceLoaderOptions));
+    loader->start(request);
     ASSERT(loader->hasOneRef());
 }
 
-PassRefPtr<DocumentThreadableLoader> DocumentThreadableLoader::create(Document& document, ThreadableLoaderClient* client, const ResourceRequest& request, const ThreadableLoaderOptions& options, const ResourceLoaderOptions& resourceLoaderOptions)
+PassRefPtr<DocumentThreadableLoader> DocumentThreadableLoader::create(Document& document, ThreadableLoaderClient* client, const ThreadableLoaderOptions& options, const ResourceLoaderOptions& resourceLoaderOptions)
 {
-    RefPtr<DocumentThreadableLoader> loader = adoptRef(new DocumentThreadableLoader(document, client, LoadAsynchronously, request, options, resourceLoaderOptions));
-    if (!loader->resource())
-        loader = nullptr;
+    RefPtr<DocumentThreadableLoader> loader = adoptRef(new DocumentThreadableLoader(document, client, LoadAsynchronously, options, resourceLoaderOptions));
     return loader.release();

[hiroshige, 2015/07/30 12:07:21]

Now can we merge the two lines above into one?

[tyoshino (SeeGerritForStatus), 2015/07/31 08:00:42]

Done.

 }
 
-DocumentThreadableLoader::DocumentThreadableLoader(Document& document, ThreadableLoaderClient* client, BlockingBehavior blockingBehavior, const ResourceRequest& request, const ThreadableLoaderOptions& options, const ResourceLoaderOptions& resourceLoaderOptions)
+DocumentThreadableLoader::DocumentThreadableLoader(Document& document, ThreadableLoaderClient* client, BlockingBehavior blockingBehavior, const ThreadableLoaderOptions& options, const ResourceLoaderOptions& resourceLoaderOptions)
     : m_client(client)
     , m_document(document)
     , m_options(options)
     , m_resourceLoaderOptions(resourceLoaderOptions)
     , m_forceDoNotAllowStoredCredentials(false)
     , m_securityOrigin(m_resourceLoaderOptions.securityOrigin)
-    , m_sameOriginRequest(securityOrigin()->canRequestNoSuborigin(request.url()))
+    , m_sameOriginRequest(false)

[hiroshige, 2015/07/30 12:07:21]

optional: Should we add a comment in the header that |m_sameOriginRequest| is
not available on construction and only available after start()?

[tyoshino (SeeGerritForStatus), 2015/07/31 08:00:42]

Said that start() must always follow create().

     , m_crossOriginNonSimpleRequest(false)
     , m_isUsingDataConsumerHandle(false)
     , m_async(blockingBehavior == LoadAsynchronously)
-    , m_requestContext(request.requestContext())
+    , m_requestContext(WebURLRequest::RequestContextUnspecified)

[hiroshige, 2015/07/30 12:07:21]

ditto.

[tyoshino (SeeGerritForStatus), 2015/07/31 08:00:42]

Done.

     , m_timeoutTimer(this, &DocumentThreadableLoader::didTimeout)
     , m_requestStartedSeconds(0.0)
     , m_corsRedirectLimit(kMaxCORSRedirects)
 {
     ASSERT(client);
+}
+
+void DocumentThreadableLoader::start(const ResourceRequest& request)
+{
     // Setting an outgoing referer is only supported in the async code path.
     ASSERT(m_async || request.httpReferrer().isEmpty());
 
+    m_sameOriginRequest = securityOrigin()->canRequestNoSuborigin(request.url());
+    m_requestContext = request.requestContext();
+
     if (!m_sameOriginRequest && m_options.crossOriginRequestPolicy == DenyCrossOriginRequests) {
         m_client->didFail(ResourceError(errorDomainBlinkInternal, 0, request.url().string(), "Cross origin requests are not supported."));
         return;
     }
 
     m_requestStartedSeconds = monotonicallyIncreasingTime();
 
     // Save any CORS simple headers on the request here. If this request redirects cross-origin, we cancel the old request
     // create a new one, and copy these headers.
     const HTTPHeaderMap& headerMap = request.httpHeaderFields();
     for (const auto& header : headerMap) {
         if (FetchUtils::isSimpleHeader(header.key, header.value))
             m_simpleRequestHeaders.add(header.key, header.value);
     }
 
     // If the fetch request will be handled by the ServiceWorker, the
     // FetchRequestMode of the request must be FetchRequestModeCORS or
     // FetchRequestModeCORSWithForcedPreflight. Otherwise the ServiceWorker can
     // return a opaque response which is from the other origin site and the
     // script in the page can read the content.
     //
     // We assume that ServiceWorker is skipped for sync requests and non-HTTP
     // familiy requests by content/ code.
     if (m_async && !request.skipServiceWorker() && request.url().protocolIsInHTTPFamily() && m_document.fetcher()->isControlledByServiceWorker()) {
         ResourceRequest newRequest(request);
         // FetchRequestMode should be set by the caller. But the expected value
         // of FetchRequestMode is not speced yet except for XHR. So we set here.
         // FIXME: When we support fetch API in document, this value should not
         // be overridden here.
-        if (options.preflightPolicy == ForcePreflight)
+        if (m_options.preflightPolicy == ForcePreflight)
             newRequest.setFetchRequestMode(WebURLRequest::FetchRequestModeCORSWithForcedPreflight);
         else
             newRequest.setFetchRequestMode(WebURLRequest::FetchRequestModeCORS);
 
         m_fallbackRequestForServiceWorker = adoptPtr(new ResourceRequest(request));
         m_fallbackRequestForServiceWorker->setSkipServiceWorker(true);
 
         loadRequest(newRequest, m_resourceLoaderOptions);
         return;
     }
@@ skipping 444 matching lines @@
         if (m_actualRequest)
             resourceLoaderOptions.dataBufferingPolicy = BufferData;
 
         if (m_options.timeoutMilliseconds > 0)
             m_timeoutTimer.startOneShot(m_options.timeoutMilliseconds / 1000.0, FROM_HERE);
 
         FetchRequest newRequest(request, m_options.initiator, resourceLoaderOptions);
         if (m_options.crossOriginRequestPolicy == AllowCrossOriginRequests)
             newRequest.setOriginRestriction(FetchRequest::NoOriginRestriction);
         ASSERT(!resource());
+
         if (request.requestContext() == WebURLRequest::RequestContextVideo || request.requestContext() == WebURLRequest::RequestContextAudio)
             setResource(RawResource::fetchMedia(newRequest, m_document.fetcher()));
         else
             setResource(RawResource::fetch(newRequest, m_document.fetcher()));
-        if (resource() && resource()->loader()) {
+
+        if (!resource()) {
+            m_client->didFail(ResourceError(errorDomainBlinkInternal, 0, requestURL.string(), "Failed to start loading."));
+            return;
+        }
+
+        if (resource()->loader()) {
             unsigned long identifier = resource()->identifier();
             InspectorInstrumentation::documentThreadableLoaderStartedLoadingForClient(&m_document, identifier, m_client);
         }
         return;
     }
 
     FetchRequest fetchRequest(request, m_options.initiator, resourceLoaderOptions);
     if (m_options.crossOriginRequestPolicy == AllowCrossOriginRequests)
         fetchRequest.setOriginRestriction(FetchRequest::NoOriginRestriction);
     ResourcePtr<Resource> resource = RawResource::fetchSynchronously(fetchRequest, m_document.fetcher());
@@ skipping 53 matching lines @@
         return DoNotAllowStoredCredentials;
     return m_resourceLoaderOptions.allowCredentials;
 }
 
 SecurityOrigin* DocumentThreadableLoader::securityOrigin() const
 {
     return m_securityOrigin ? m_securityOrigin.get() : m_document.securityOrigin();
 }
 
 } // namespace blink