Drop ResourceLoadPriorityUnresolved resource load priority and use Optional<> instead
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Feb 2015 01:23:58 +0000 (01:23 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Feb 2015 01:23:58 +0000 (01:23 +0000)
https://bugs.webkit.org/show_bug.cgi?id=141186

Reviewed by Antti Koivisto.

Source/WebCore:

Drop ResourceLoadPriorityUnresolved resource load priority value and use
Optional<ResourceLoadPriority> when needed instead. If the Optional
doesn't have a value, then it means it is unresolved. Having
ResourceLoadPriorityUnresolved in ResourceLoadPriority was confusing
because this value is only valid in CachedResourceRequest, it is not
a valid value in CachedResource or in ResourceRequest. After this
refactoring, it now becomes more obvious.

Source/WebKit2:

Update code now that ResourceLoadPriorityUnresolved is not longer a
ResourceLoadPriority enum value.

* NetworkProcess/cache/NetworkCache.cpp:
(WebKit::NetworkCache::retrieve):
* WebProcess/Network/WebResourceLoadScheduler.cpp:
(WebKit::WebResourceLoadScheduler::scheduleLoad):

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

19 files changed:
Source/WebCore/ChangeLog
Source/WebCore/html/HTMLLinkElement.cpp
Source/WebCore/loader/LinkLoader.cpp
Source/WebCore/loader/ResourceLoadScheduler.cpp
Source/WebCore/loader/cache/CachedResource.cpp
Source/WebCore/loader/cache/CachedResource.h
Source/WebCore/loader/cache/CachedResourceLoader.cpp
Source/WebCore/loader/cache/CachedResourceRequest.cpp
Source/WebCore/loader/cache/CachedResourceRequest.h
Source/WebCore/platform/network/ResourceLoadPriority.h
Source/WebCore/platform/network/ResourceRequestBase.cpp
Source/WebCore/platform/network/ResourceRequestBase.h
Source/WebCore/platform/network/cf/ResourceRequestCFNet.cpp
Source/WebCore/platform/network/cf/ResourceRequestCFNet.h
Source/WebCore/platform/network/cocoa/ResourceRequestCocoa.mm
Source/WebCore/platform/network/soup/ResourceRequest.h
Source/WebKit2/ChangeLog
Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp
Source/WebKit2/WebProcess/Network/WebResourceLoadScheduler.cpp

index 5f698acd316864fe92bf688df8227dd80d5377aa..9c3f266ced1bfab004ee0c5916379060e6050daf 100644 (file)
@@ -1,3 +1,18 @@
+2015-02-03  Chris Dumez  <cdumez@apple.com>
+
+        Drop ResourceLoadPriorityUnresolved resource load priority and use Optional<> instead
+        https://bugs.webkit.org/show_bug.cgi?id=141186
+
+        Reviewed by Antti Koivisto.
+
+        Drop ResourceLoadPriorityUnresolved resource load priority value and use
+        Optional<ResourceLoadPriority> when needed instead. If the Optional
+        doesn't have a value, then it means it is unresolved. Having
+        ResourceLoadPriorityUnresolved in ResourceLoadPriority was confusing
+        because this value is only valid in CachedResourceRequest, it is not
+        a valid value in CachedResource or in ResourceRequest. After this
+        refactoring, it now becomes more obvious.
+
 2015-02-03  Chris Dumez  <cdumez@apple.com>
 
         REGRESSION(176609): Very high memory usage in Canvas/reuse.html performance test
index 0091a71facac4025ce0715a0a0ec731a2d87a7d6..0ad1378de4971b5a9facfd929778efb15d0335fe 100644 (file)
@@ -229,7 +229,9 @@ void HTMLLinkElement::process()
         addPendingSheet(isActive ? ActiveSheet : InactiveSheet);
 
         // Load stylesheets that are not needed for the rendering immediately with low priority.
-        ResourceLoadPriority priority = isActive ? ResourceLoadPriorityUnresolved : ResourceLoadPriorityVeryLow;
+        Optional<ResourceLoadPriority> priority;
+        if (!isActive)
+            priority = ResourceLoadPriorityVeryLow;
         CachedResourceRequest request(ResourceRequest(document().completeURL(url)), charset, priority);
         request.setInitiator(this);
         m_cachedSheet = document().cachedResourceLoader().requestCSSStyleSheet(request);
index 2ffaf60b5d9fc9373534772e2e80b2a518877c49..c6d02f422c27f2b2a44cae57afb9efaea8e69fcd 100644 (file)
@@ -105,7 +105,7 @@ bool LinkLoader::loadLink(const LinkRelAttribute& relAttribute, const String& ty
     if ((relAttribute.m_isLinkPrefetch || relAttribute.m_isLinkSubresource) && href.isValid() && document->frame()) {
         if (!m_client->shouldLoadLink())
             return false;
-        ResourceLoadPriority priority = ResourceLoadPriorityUnresolved;
+        Optional<ResourceLoadPriority> priority;
         CachedResource::Type type = CachedResource::LinkPrefetch;
         // We only make one request to the cachedresourcelodaer if multiple rel types are
         // specified, 
index 51aeb8468048d43db61d2ab8a426240cdab8923c..b3eafe8ffb9bb53841016e388fa859fc1f7c5a05 100644 (file)
@@ -163,7 +163,6 @@ void ResourceLoadScheduler::scheduleLoad(ResourceLoader* resourceLoader)
 #endif
 
     ResourceLoadPriority priority = resourceLoader->request().priority();
-    ASSERT(priority != ResourceLoadPriorityUnresolved);
 
     bool hadRequests = host->hasRequests();
     host->schedule(resourceLoader, priority);
index 971d86494599d90d6a992897fe6366e1ae08cc19..65955e09e21d5fde845e69887550658ae9c52caf 100644 (file)
@@ -736,11 +736,12 @@ unsigned CachedResource::overheadSize() const
     return sizeof(CachedResource) + m_response.memoryUsage() + kAverageClientsHashMapSize + m_resourceRequest.url().string().length() * 2;
 }
 
-void CachedResource::setLoadPriority(ResourceLoadPriority loadPriority)
+void CachedResource::setLoadPriority(const Optional<ResourceLoadPriority>& loadPriority)
 {
-    if (loadPriority == ResourceLoadPriorityUnresolved)
-        loadPriority = defaultPriorityForResourceType(type());
-    m_loadPriority = loadPriority;
+    if (loadPriority)
+        m_loadPriority = loadPriority.value();
+    else
+        m_loadPriority = defaultPriorityForResourceType(type());
 }
 
 inline CachedResource::Callback::Callback(CachedResource& resource, CachedResourceClient& client)
index e784d1f61f96f4ec9ae706196e7690410704d21a..66d29cc7409bc28b3cbe35e3a30e180582491946 100644 (file)
@@ -117,7 +117,7 @@ public:
     Type type() const { return static_cast<Type>(m_type); }
     
     ResourceLoadPriority loadPriority() const { return m_loadPriority; }
-    void setLoadPriority(ResourceLoadPriority);
+    void setLoadPriority(const Optional<ResourceLoadPriority>&);
 
     WEBCORE_EXPORT void addClient(CachedResourceClient*);
     WEBCORE_EXPORT void removeClient(CachedResourceClient*);
index 10659fda9269852d128cd681c39af48297899d41..e6989d545504bea46f121ea4cc88908d5fbab46c 100644 (file)
@@ -452,7 +452,7 @@ CachedResourceHandle<CachedResource> CachedResourceLoader::requestResource(Cache
 {
     URL url = request.resourceRequest().url();
     
-    LOG(ResourceLoading, "CachedResourceLoader::requestResource '%s', charset '%s', priority=%d, forPreload=%u", url.stringCenterEllipsizedToLength().latin1().data(), request.charset().latin1().data(), request.priority(), request.forPreload());
+    LOG(ResourceLoading, "CachedResourceLoader::requestResource '%s', charset '%s', priority=%d, forPreload=%u", url.stringCenterEllipsizedToLength().latin1().data(), request.charset().latin1().data(), request.priority() ? request.priority().value() : -1, request.forPreload());
     
     // If only the fragment identifiers differ, it is the same resource.
     url = MemoryCache::removeFragmentIdentifierIfNeeded(url);
index 389e531125b65e78bdfebb0bbf60d1f40b90b3fe..53a53e5ff486c9a5dfbcbf1af262d6cf82ccafcd 100644 (file)
@@ -32,7 +32,7 @@
 
 namespace WebCore {
 
-CachedResourceRequest::CachedResourceRequest(const ResourceRequest& resourceRequest, const String& charset, ResourceLoadPriority priority)
+CachedResourceRequest::CachedResourceRequest(const ResourceRequest& resourceRequest, const String& charset, Optional<ResourceLoadPriority> priority)
     : m_resourceRequest(resourceRequest)
     , m_charset(charset)
     , m_options(CachedResourceLoader::defaultCachedResourceOptions())
@@ -45,13 +45,12 @@ CachedResourceRequest::CachedResourceRequest(const ResourceRequest& resourceRequ
 CachedResourceRequest::CachedResourceRequest(const ResourceRequest& resourceRequest, const ResourceLoaderOptions& options)
     : m_resourceRequest(resourceRequest)
     , m_options(options)
-    , m_priority(ResourceLoadPriorityUnresolved)
     , m_forPreload(false)
     , m_defer(NoDefer)
 {
 }
 
-CachedResourceRequest::CachedResourceRequest(const ResourceRequest& resourceRequest, ResourceLoadPriority priority)
+CachedResourceRequest::CachedResourceRequest(const ResourceRequest& resourceRequest, Optional<ResourceLoadPriority> priority)
     : m_resourceRequest(resourceRequest)
     , m_options(CachedResourceLoader::defaultCachedResourceOptions())
     , m_priority(priority)
index e011d384c6a1b03aff25d34ab91d3ac0dbe6c89a..9938c0896ce9a185b893e3a9f3186ba851e74618 100644 (file)
@@ -40,9 +40,9 @@ class CachedResourceRequest {
 public:
     enum DeferOption { NoDefer, DeferredByClient };
 
-    explicit CachedResourceRequest(const ResourceRequest&, const String& charset = String(), ResourceLoadPriority = ResourceLoadPriorityUnresolved);
+    explicit CachedResourceRequest(const ResourceRequest&, const String& charset = String(), Optional<ResourceLoadPriority> = Nullopt);
     CachedResourceRequest(const ResourceRequest&, const ResourceLoaderOptions&);
-    CachedResourceRequest(const ResourceRequest&, ResourceLoadPriority);
+    CachedResourceRequest(const ResourceRequest&, Optional<ResourceLoadPriority>);
     ~CachedResourceRequest();
 
     ResourceRequest& mutableResourceRequest() { return m_resourceRequest; }
@@ -51,8 +51,7 @@ public:
     void setCharset(const String& charset) { m_charset = charset; }
     const ResourceLoaderOptions& options() const { return m_options; }
     void setOptions(const ResourceLoaderOptions& options) { m_options = options; }
-    void setPriority(ResourceLoadPriority priority) { m_priority = priority; }
-    ResourceLoadPriority priority() const { return m_priority; }
+    const Optional<ResourceLoadPriority>& priority() const { return m_priority; }
     bool forPreload() const { return m_forPreload; }
     void setForPreload(bool forPreload) { m_forPreload = forPreload; }
     DeferOption defer() const { return m_defer; }
@@ -65,7 +64,7 @@ private:
     ResourceRequest m_resourceRequest;
     String m_charset;
     ResourceLoaderOptions m_options;
-    ResourceLoadPriority m_priority;
+    Optional<ResourceLoadPriority> m_priority;
     bool m_forPreload;
     DeferOption m_defer;
     RefPtr<Element> m_initiatorElement;
index 5a7104f97b12928f01c31de07c6c3e1235af71ae..6d1dc5b6451dc8ff87f5a467d3facd68697c3baf 100644 (file)
@@ -29,9 +29,7 @@
 namespace WebCore {
 
 enum ResourceLoadPriority {
-    // The unresolved priority is here for the convenience of the clients. It should not be passed to the ResourceLoadScheduler.
-    ResourceLoadPriorityUnresolved = -1,
-    ResourceLoadPriorityVeryLow = 0,
+    ResourceLoadPriorityVeryLow,
     ResourceLoadPriorityLow,
     ResourceLoadPriorityMedium,
     ResourceLoadPriorityHigh,
index 09d8fb120cf5b764d571b015fe2b47992a97237e..94dccb9c6a4f1a3d86c314c3df7843929bab610d 100644 (file)
@@ -437,7 +437,7 @@ ResourceLoadPriority ResourceRequestBase::priority() const
 {
     updateResourceRequest();
 
-    return m_priority;
+    return static_cast<ResourceLoadPriority>(m_priority);
 }
 
 void ResourceRequestBase::setPriority(ResourceLoadPriority priority)
@@ -448,6 +448,7 @@ void ResourceRequestBase::setPriority(ResourceLoadPriority priority)
         return;
 
     m_priority = priority;
+    ASSERT(static_cast<ResourceLoadPriority>(m_priority) == priority); // Make sure it fits in the bitfield.
 
     if (url().protocolIsInHTTPFamily())
         m_platformRequestUpdated = false;
index 9b5e7970718b044d9c34f22c7bd8ee0558e6606f..3920d2b9b0b57cd7e81d711ee6033f34488fa3b5 100644 (file)
@@ -214,16 +214,16 @@ namespace WebCore {
         Vector<String> m_responseContentDispositionEncodingFallbackArray;
         RefPtr<FormData> m_httpBody;
         unsigned m_cachePolicy : 3;
-        bool m_allowCookies : 1;
-        mutable bool m_resourceRequestUpdated : 1;
-        mutable bool m_platformRequestUpdated : 1;
-        mutable bool m_resourceRequestBodyUpdated : 1;
-        mutable bool m_platformRequestBodyUpdated : 1;
-        bool m_reportUploadProgress : 1;
-        bool m_reportLoadTiming : 1;
-        bool m_reportRawHeaders : 1;
-        bool m_hiddenFromInspector : 1;
-        ResourceLoadPriority m_priority : 4; // not unsigned because ResourceLoadPriority has negative values
+        unsigned m_allowCookies : 1;
+        mutable unsigned m_resourceRequestUpdated : 1;
+        mutable unsigned m_platformRequestUpdated : 1;
+        mutable unsigned m_resourceRequestBodyUpdated : 1;
+        mutable unsigned m_platformRequestBodyUpdated : 1;
+        unsigned m_reportUploadProgress : 1;
+        unsigned m_reportLoadTiming : 1;
+        unsigned m_reportRawHeaders : 1;
+        unsigned m_hiddenFromInspector : 1;
+        unsigned m_priority : 4;
 
     private:
         const ResourceRequest& asResourceRequest() const;
index c6475dfdcaceec128a113e03bd6e72ea37ff521c..f6f2bc0f0cbb3351eae0566aede2f472bf7b223f 100644 (file)
@@ -158,7 +158,7 @@ void ResourceRequest::doUpdatePlatformRequest()
         wkHTTPRequestEnablePipelining(cfRequest);
 
     if (resourcePrioritiesEnabled())
-        wkSetHTTPRequestPriority(cfRequest, toPlatformRequestPriority(m_priority));
+        wkSetHTTPRequestPriority(cfRequest, toPlatformRequestPriority(priority()));
 
 #if !PLATFORM(WIN)
     wkCFURLRequestAllowAllPostCaching(cfRequest);
@@ -274,11 +274,8 @@ void ResourceRequest::doUpdateResourceRequest()
     }
     m_allowCookies = CFURLRequestShouldHandleHTTPCookies(m_cfRequest.get());
 
-    if (resourcePrioritiesEnabled()) {
-        auto priority = toResourceLoadPriority(wkGetHTTPRequestPriority(m_cfRequest.get()));
-        if (priority > ResourceLoadPriorityUnresolved)
-            m_priority = priority;
-    }
+    if (resourcePrioritiesEnabled())
+        m_priority = toResourceLoadPriority(wkGetHTTPRequestPriority(m_cfRequest.get()));
 
     m_httpHeaderFields.clear();
     if (CFDictionaryRef headers = CFURLRequestCopyAllHTTPHeaderFields(m_cfRequest.get())) {
index af5d38ae83b60dc89d7a66b4744b6728e144c790..769c8e59646918ec25a0901af16dacf3b02c9f07 100644 (file)
@@ -44,8 +44,6 @@ CFURLRequestRef cfURLRequest(const ResourceRequest&);
 inline ResourceLoadPriority toResourceLoadPriority(int priority)
 {
     switch (priority) {
-    case -1:
-        return ResourceLoadPriorityUnresolved;
     case 0:
         return ResourceLoadPriorityVeryLow;
     case 1:
@@ -65,8 +63,6 @@ inline ResourceLoadPriority toResourceLoadPriority(int priority)
 inline int toPlatformRequestPriority(ResourceLoadPriority priority)
 {
     switch (priority) {
-    case ResourceLoadPriorityUnresolved:
-        return -1;
     case ResourceLoadPriorityVeryLow:
         return 0;
     case ResourceLoadPriorityLow:
index 0a609fc275059dd528fcd18824d75909829f7a8f..d777a031b75994505866adabd05734e7c90d6a0e 100644 (file)
@@ -78,11 +78,8 @@ void ResourceRequest::doUpdateResourceRequest()
         m_httpMethod = method;
     m_allowCookies = [m_nsRequest.get() HTTPShouldHandleCookies];
 
-    if (resourcePrioritiesEnabled()) {
-        auto priority = toResourceLoadPriority(wkGetHTTPRequestPriority([m_nsRequest.get() _CFURLRequest]));
-        if (priority > ResourceLoadPriorityUnresolved)
-            m_priority = priority;
-    }
+    if (resourcePrioritiesEnabled())
+        m_priority = toResourceLoadPriority(wkGetHTTPRequestPriority([m_nsRequest.get() _CFURLRequest]));
 
     m_httpHeaderFields.clear();
     [[m_nsRequest allHTTPHeaderFields] enumerateKeysAndObjectsUsingBlock: ^(NSString *name, NSString *value, BOOL *) {
@@ -145,7 +142,7 @@ void ResourceRequest::doUpdatePlatformRequest()
         wkHTTPRequestEnablePipelining([nsRequest _CFURLRequest]);
 
     if (ResourceRequest::resourcePrioritiesEnabled())
-        wkSetHTTPRequestPriority([nsRequest _CFURLRequest], toPlatformRequestPriority(m_priority));
+        wkSetHTTPRequestPriority([nsRequest _CFURLRequest], toPlatformRequestPriority(priority()));
 
     [nsRequest setCachePolicy:(NSURLRequestCachePolicy)cachePolicy()];
     wkCFURLRequestAllowAllPostCaching([nsRequest _CFURLRequest]);
index 44c8345cfd4b500c1d770919940854b5de82ad16..3fa0934efb41e95e20f1401dbb5307a5fdbc64ac 100644 (file)
@@ -132,8 +132,6 @@ namespace WebCore {
 inline SoupMessagePriority toSoupMessagePriority(ResourceLoadPriority priority)
 {
     switch (priority) {
-    case ResourceLoadPriorityUnresolved:
-        return SOUP_MESSAGE_PRIORITY_NORMAL;
     case ResourceLoadPriorityVeryLow:
         return SOUP_MESSAGE_PRIORITY_VERY_LOW;
     case ResourceLoadPriorityLow:
index e055d97e1c3e85fa16ca5c8afa04d9f5436ecf32..0c98d1f2c4fd92e8bc53b2c279d4d58a1e1f6c59 100644 (file)
@@ -1,3 +1,18 @@
+2015-02-03  Chris Dumez  <cdumez@apple.com>
+
+        Drop ResourceLoadPriorityUnresolved resource load priority and use Optional<> instead
+        https://bugs.webkit.org/show_bug.cgi?id=141186
+
+        Reviewed by Antti Koivisto.
+
+        Update code now that ResourceLoadPriorityUnresolved is not longer a
+        ResourceLoadPriority enum value.
+
+        * NetworkProcess/cache/NetworkCache.cpp:
+        (WebKit::NetworkCache::retrieve):
+        * WebProcess/Network/WebResourceLoadScheduler.cpp:
+        (WebKit::WebResourceLoadScheduler::scheduleLoad):
+
 2015-02-03  Enrica Casucci  <enrica@apple.com>
 
         [iOS] Add support for deleteFromInputWithFlags.
index 7e4e6c48c0e4941196defafd8e377d109e626bb7..81f24a4e376bcb7fb6002f350a328f6fa83e9233 100644 (file)
@@ -218,7 +218,7 @@ void NetworkCache::retrieve(const WebCore::ResourceRequest& originalRequest, std
 {
     ASSERT(isEnabled());
 
-    LOG(NetworkCache, "(NetworkProcess) retrieving %s priority %d", originalRequest.url().string().ascii().data(), originalRequest.priority());
+    LOG(NetworkCache, "(NetworkProcess) retrieving %s priority %u", originalRequest.url().string().ascii().data(), originalRequest.priority());
 
     if (!canRetrieve(originalRequest)) {
         completionHandler(nullptr);
@@ -249,7 +249,7 @@ void NetworkCache::retrieve(const WebCore::ResourceRequest& originalRequest, std
 #if !LOG_DISABLED
         auto elapsedMS = std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::system_clock::now() - startTime).count();
 #endif
-        LOG(NetworkCache, "(NetworkProcess) retrieve complete success=%d priority=%d time=%lldms", success, capture->originalRequest.priority(), elapsedMS);
+        LOG(NetworkCache, "(NetworkProcess) retrieve complete success=%d priority=%u time=%lldms", success, capture->originalRequest.priority(), elapsedMS);
         capture->completionHandler(WTF::move(decodedEntry));
         return success;
     });
index 80245fb6bfe0faeffb58f3f97361f7a29f9e5182..925279e3096139a668ad991300ba4cd1a083176d 100644 (file)
@@ -150,7 +150,7 @@ void WebResourceLoadScheduler::scheduleLoad(ResourceLoader* resourceLoader, Cach
     }
 #endif
 
-    LOG(NetworkScheduling, "(WebProcess) WebResourceLoadScheduler::scheduleLoad, url '%s' will be scheduled with the NetworkProcess with priority %i", resourceLoader->url().string().utf8().data(), resourceLoader->request().priority());
+    LOG(NetworkScheduling, "(WebProcess) WebResourceLoadScheduler::scheduleLoad, url '%s' will be scheduled with the NetworkProcess with priority %u", resourceLoader->url().string().utf8().data(), resourceLoader->request().priority());
 
     ContentSniffingPolicy contentSniffingPolicy = resourceLoader->shouldSniffContent() ? SniffContent : DoNotSniffContent;
     StoredCredentials allowStoredCredentials = resourceLoader->shouldUseCredentialStorage() ? AllowStoredCredentials : DoNotAllowStoredCredentials;