Split requestResource into request preparation and loading

third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp

 /*
     Copyright (C) 1998 Lars Knoll (knoll@mpi-hd.mpg.de)
     Copyright (C) 2001 Dirk Mueller (mueller@kde.org)
     Copyright (C) 2002 Waldo Bastian (bastian@kde.org)
     Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc. All
     rights reserved.
     Copyright (C) 2009 Torch Mobile Inc. http://www.torchmobile.com/
 
     This library is free software; you can redistribute it and/or
     modify it under the terms of the GNU Library General Public
@@ skipping 435 matching lines @@
   // Aims to count Resource only referenced from MemoryCache (i.e. what would be
   // dead if MemoryCache holds weak references to Resource). Currently we check
   // references to Resource from ResourceClient and |m_preloads| only, because
   // they are major sources of references.
   if (resource && !resource->isAlive() &&
       (!m_preloads || !m_preloads->contains(resource))) {
     DEFINE_RESOURCE_HISTOGRAM("Dead.");
   }
 }
 
-Resource* ResourceFetcher::requestResource(
-    FetchRequest& request,
-    const ResourceFactory& factory,
-    const SubstituteData& substituteData) {
+Resource* ResourceFetcher::prepareRequest(FetchRequest& request,
+                                          const ResourceFactory& factory,
+                                          const SubstituteData& substituteData,
+                                          unsigned long identifier,
+                                          bool& isStaticData,
+                                          bool& shouldContinue) {
   ResourceRequest& resourceRequest = request.mutableResourceRequest();
 
-  unsigned long identifier = createUniqueIdentifier();
-  network_instrumentation::ScopedResourceLoadTracker scopedResourceLoadTracker(
-      identifier, resourceRequest);
-  SCOPED_BLINK_UMA_HISTOGRAM_TIMER("Blink.Fetch.RequestResourceTime");
   DCHECK(request.options().synchronousPolicy == RequestAsynchronously ||
          factory.type() == Resource::Raw ||
          factory.type() == Resource::XSLStyleSheet);
 
   context().populateResourceRequest(
       factory.type(), request.clientHintsPreferences(),
       request.getResourceWidth(), resourceRequest);
 
   // TODO(dproy): Remove this. http://crbug.com/659666
   TRACE_EVENT1("blink", "ResourceFetcher::requestResource", "url",

[Charlie Harrison, 2017/01/30 22:02:58]

This changes behavior, as trace events are scoped to the function they are
defined in. Could you just move this to the top of requestResource, so it is
aligned with the SCOPED_BLINK_UMA histogram?

                urlForTraceEvent(request.url()));
 
   if (!request.url().isValid())
     return nullptr;
 
   resourceRequest.setPriority(computeLoadPriority(
       factory.type(), request.resourceRequest(), ResourcePriority::NotVisible,
       request.defer(), request.forPreload()));
   initializeResourceRequest(resourceRequest, factory.type(), request.defer());
   network_instrumentation::resourcePrioritySet(identifier,
                                                resourceRequest.priority());
 
   ResourceRequestBlockedReason blockedReason = context().canRequest(
       factory.type(), resourceRequest,
       MemoryCache::removeFragmentIdentifierIfNeeded(request.url()),
       request.options(), request.forPreload(), request.getOriginRestriction());
+  Resource* resource = nullptr;
   if (blockedReason != ResourceRequestBlockedReason::None) {
     DCHECK(!substituteData.forceSynchronousLoad());
     return resourceForBlockedRequest(request, factory, blockedReason);
   }
 
   context().willStartLoadingResource(
       identifier, resourceRequest, factory.type(),
       request.options().initiatorInfo.name, request.forPreload());
   if (!request.url().isValid())
     return nullptr;
 
   bool isDataUrl = resourceRequest.url().protocolIsData();
-  bool isStaticData = isDataUrl || substituteData.isValid() || m_archive;
-  Resource* resource(nullptr);
+  isStaticData = isDataUrl || substituteData.isValid() || m_archive;
   if (isStaticData) {
     resource = resourceForStaticData(request, factory, substituteData);
     // Abort the request if the archive doesn't contain the resource, except in
     // the case of data URLs which might have resources such as fonts that need
     // to be decoded only on demand.  These data URLs are allowed to be
     // processed using the normal ResourceFetcher machinery.
     if (!resource && !isDataUrl && m_archive)
       return nullptr;
   }
+  resourceRequest.setAllowStoredCredentials(
+      request.options().allowCredentials == AllowStoredCredentials);
+  shouldContinue = true;
+  return resource;
+}
+
+Resource* ResourceFetcher::requestResource(
+    FetchRequest& request,
+    const ResourceFactory& factory,
+    const SubstituteData& substituteData) {
+  unsigned long identifier = createUniqueIdentifier();
+  ResourceRequest& resourceRequest = request.mutableResourceRequest();
+  network_instrumentation::ScopedResourceLoadTracker scopedResourceLoadTracker(
+      identifier, resourceRequest);
+  SCOPED_BLINK_UMA_HISTOGRAM_TIMER("Blink.Fetch.RequestResourceTime");
+
+  bool isStaticData = false;
+  bool shouldContinue = false;
+  Resource* resource = prepareRequest(request, factory, substituteData,
+                                      identifier, isStaticData, shouldContinue);
+  if (shouldContinue)
+    return resource;
+
   if (!resource) {
     resource =
         memoryCache()->resourceForURL(request.url(), getCacheIdentifier());
   }
 
   // See if we can use an existing resource from the cache. If so, we need to
   // move it to be load blocking.
   moveCachedNonBlockingResourceToBlocking(resource, request);
 
   const RevalidationPolicy policy = determineRevalidationPolicy(
       factory.type(), request, resource, isStaticData);
   TRACE_EVENT_INSTANT1("blink", "ResourceFetcher::determineRevalidationPolicy",
                        TRACE_EVENT_SCOPE_THREAD, "revalidationPolicy", policy);
 
   updateMemoryCacheStats(resource, policy, request, factory, isStaticData);
 
-  resourceRequest.setAllowStoredCredentials(
-      request.options().allowCredentials == AllowStoredCredentials);
-
   switch (policy) {
     case Reload:
       memoryCache()->remove(resource);
     // Fall through
     case Load:
       resource = createResourceForLoading(request, request.charset(), factory);
       break;
     case Revalidate:
       initializeRevalidation(resourceRequest, resource);
       break;
@@ skipping 962 matching lines @@
   visitor->trace(m_context);
   visitor->trace(m_archive);
   visitor->trace(m_loaders);
   visitor->trace(m_nonBlockingLoaders);
   visitor->trace(m_documentResources);
   visitor->trace(m_preloads);
   visitor->trace(m_resourceTimingInfoMap);
 }
 
 }  // namespace blink