Make ResourceLoadPriority calculation simpler

third_party/WebKit/Source/core/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
     License as published by the Free Software Foundation; either
@@ skipping 116 matching lines @@
         // Also async scripts (set explicitly in loadPriority)
         return ResourceLoadPriorityLow;
     case Resource::LinkPrefetch:
         return ResourceLoadPriorityVeryLow;
     }
 
     ASSERT_NOT_REACHED();
     return ResourceLoadPriorityUnresolved;
 }
 
-ResourceLoadPriority ResourceFetcher::loadPriority(Resource::Type type, const FetchRequest& request, ResourcePriority::VisibilityStatus visibility)
+ResourceLoadPriority ResourceFetcher::computeLoadPriority(Resource::Type type, const FetchRequest& request, ResourcePriority::VisibilityStatus visibility)
 {
-    // TODO(yoav): Change it here so that priority can be changed even after it was resolved.
-    if (request.priority() != ResourceLoadPriorityUnresolved)
-        return request.priority();
-
-    // Synchronous requests should always be max priority, lest they hang the renderer.
-    if (request.options().synchronousPolicy == RequestSynchronously)
-        return ResourceLoadPriorityHighest;
-
     ResourceLoadPriority priority = typeToPriority(type);
 
     // Visible resources (images in practice) get a boost to High priority.
     if (visibility == ResourcePriority::Visible)
         priority = ResourceLoadPriorityHigh;
 
     // Resources before the first image are considered "early" in the document
     // and resources after the first image are "late" in the document.  Important to
     // note that this is based on when the preload scanner discovers a resource
     // for the most part so the main parser may not have reached the image element yet.
     if (type == Resource::Image)
         m_imageFetched = true;
 
     // Special handling for scripts.
     // Default/Parser-Blocking/Preload early in document: High (set in typeToPriority)
     // Async/Defer: Low Priority (applies to both preload and parser-inserted)
     // Preload late in document: Medium
     if (type == Resource::Script) {
         if (FetchRequest::LazyLoad == request.defer())
             priority = ResourceLoadPriorityLow;
         else if (request.forPreload() && m_imageFetched)
             priority = ResourceLoadPriorityMedium;
+    } else if (FetchRequest::LazyLoad == request.defer()) {
+        priority = ResourceLoadPriorityVeryLow;

[Yoav Weiss, 2016/06/25 08:37:59]

If I understand it correctly, this would mean that deferred and async scripts
will now be downloaded with very low priority. Is that the case? If so, is this
intentional? Seems like a significant change from current behavior.

[Pat Meenan, 2016/06/27 14:56:55]

Scripts go through the first clause and shouldn't be changed.  This basically
just makes anything else that gets tagged as defer load at veryLow (AFAIK,
scripts previously were the only thing with the LazyLoad setting so this
shouldn't change any existing behavior.

[Nate Chapin, 2016/06/27 19:46:16]

Correct.

     }
 
-    return context().modifyPriorityForExperiments(priority);
+    // A manually set priority acts as a floor. This is used to ensure that synchronous requests
+    // are always given the highest possible priority, as well as to ensure that there isn't priority
+    // churn if images move in and out of the viewport.
+    return std::max(context().modifyPriorityForExperiments(priority), request.resourceRequest().priority());

[Yoav Weiss, 2016/06/25 08:37:59]

In the image case, the "priority churn" can also have advantages, if the user
flicks through some images which we haven't yet downloaded, it might be better
to give higher priority to the ones that are in current viewport over ones in
past viewports. Is there a cost to said churn?

[Pat Meenan, 2016/06/27 14:56:55]

We had logic previously to prevent exactly this churn because it also applies to
the same image being reference in the viewport as well as late in the viewport. 
We want to make sure not to incorrectly decrease the priority of a resource that
is still in the viewport.  Dealing with the fast scroll case would actually be
new behavior (and should be done carefully if we want to bother).

[Nate Chapin, 2016/06/27 19:46:16]

I added a bit more detail to this comment, please review for clarity/correctness
:)

 }
 
 static void populateResourceTiming(ResourceTimingInfo* info, Resource* resource)
 {
     info->setInitialRequest(resource->resourceRequest());
     info->setFinalResponse(resource->response());
 }
 
 static WebURLRequest::RequestContext requestContextFromType(bool isMainFrame, Resource::Type type)
 {
@@ skipping 234 matching lines @@
 
     TRACE_EVENT1("blink", "ResourceFetcher::requestResource", "url", urlForTraceEvent(request.url()));
 
     if (!request.url().isValid())
         return nullptr;
 
     if (!context().canRequest(factory.type(), request.resourceRequest(), MemoryCache::removeFragmentIdentifierIfNeeded(request.url()), request.options(), request.forPreload(), request.getOriginRestriction()))
         return nullptr;
 
     unsigned long identifier = createUniqueIdentifier();
-    request.setPriority(loadPriority(factory.type(), request, ResourcePriority::NotVisible));
-    request.mutableResourceRequest().setPriority(request.priority());
+    request.mutableResourceRequest().setPriority(computeLoadPriority(factory.type(), request, ResourcePriority::NotVisible));
     initializeResourceRequest(request.mutableResourceRequest(), factory.type(), request.defer());
     context().willStartLoadingResource(identifier, request.mutableResourceRequest(), factory.type());
     if (!request.url().isValid())
         return nullptr;
 
     if (!request.forPreload()) {
         V8DOMActivityLogger* activityLogger = nullptr;
         if (request.options().initiatorInfo.name == FetchInitiatorTypeNames::xmlhttprequest)
             activityLogger = V8DOMActivityLogger::currentActivityLogger();
         else
@@ skipping 55 matching lines @@
 
     if (policy != Use)
         resource->setIdentifier(identifier);
 
     if (!request.forPreload() || policy != Use) {
         // When issuing another request for a resource that is already in-flight make
         // sure to not demote the priority of the in-flight request. If the new request
         // isn't at the same priority as the in-flight request, only allow promotions.
         // This can happen when a visible image's priority is increased and then another
         // reference to the image is parsed (which would be at a lower priority).
-        if (request.priority() > resource->resourceRequest().priority())
-            resource->didChangePriority(request.priority(), 0);
+        if (request.resourceRequest().priority() > resource->resourceRequest().priority())
+            resource->didChangePriority(request.resourceRequest().priority(), 0);
     }
 
     // If only the fragment identifiers differ, it is the same resource.
     DCHECK(equalIgnoringFragmentIdentifier(resource->url(), request.url()));
     requestLoadStarted(identifier, resource, request, policy == Use ? ResourceLoadingFromCache : ResourceLoadingFromNetwork, isStaticData);
     m_documentResources.set(MemoryCache::removeFragmentIdentifierIfNeeded(request.url()), resource);
 
     // Returns with an existing resource if the resource does not need to start
     // loading immediately.
     // If revalidation policy was determined as |Revalidate|, the resource was
@@ skipping 555 matching lines @@
 
 void ResourceFetcher::updateAllImageResourcePriorities()
 {
     TRACE_EVENT0("blink", "ResourceLoadPriorityOptimizer::updateAllImageResourcePriorities");
     for (const auto& documentResource : m_documentResources) {
         Resource* resource = documentResource.value.get();
         if (!resource || !resource->isImage() || !resource->isLoading())
             continue;
 
         ResourcePriority resourcePriority = resource->priorityFromObservers();
-        ResourceLoadPriority resourceLoadPriority = loadPriority(Resource::Image, FetchRequest(resource->resourceRequest(), FetchInitiatorInfo()), resourcePriority.visibility);
+        ResourceLoadPriority resourceLoadPriority = computeLoadPriority(Resource::Image, FetchRequest(resource->resourceRequest(), FetchInitiatorInfo()), resourcePriority.visibility);
         if (resourceLoadPriority == resource->resourceRequest().priority())
             continue;
 
         resource->didChangePriority(resourceLoadPriority, resourcePriority.intraPriorityValue);
         TRACE_EVENT_ASYNC_STEP_INTO1("blink.net", "Resource", resource, "ChangePriority", "priority", resourceLoadPriority);
         context().dispatchDidChangeResourcePriority(resource->identifier(), resourceLoadPriority, resourcePriority.intraPriorityValue);
     }
 }
 
 void ResourceFetcher::reloadLoFiImages()
@@ skipping 170 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