Title: [206900] trunk/Source/WebCore
Revision
206900
Author
commit-qu...@webkit.org
Date
2016-10-06 23:00:10 -0700 (Thu, 06 Oct 2016)

Log Message

CachedResourceRequest should not need to store defer and preload options
https://bugs.webkit.org/show_bug.cgi?id=163004

Patch by Youenn Fablet <you...@apple.com> on 2016-10-06
Reviewed by Alex Christensen.

No change of behavior.

Removing CachedResourceRequest defer and preload fields.
These fields are computed inside CachedResourceLoader instead.

Updated setting of priority from CachedResourceRequest to CachedResource.
Priority is set for any new resource (this covers all cases where no cached resource can be reused from the memory cache).
Priority is set for a cached resource if the request is not a preload request.

* loader/LinkLoader.cpp:
(WebCore::LinkLoader::preloadIfNeeded):
* loader/cache/CachedResource.cpp:
(WebCore::CachedResource::CachedResource):
* loader/cache/CachedResourceLoader.cpp:
(WebCore::CachedResourceLoader::requestImage):
(WebCore::CachedResourceLoader::canRequest):
(WebCore::CachedResourceLoader::requestResource):
(WebCore::CachedResourceLoader::determineRevalidationPolicy):
(WebCore::CachedResourceLoader::requestPreload):
* loader/cache/CachedResourceLoader.h:
* loader/cache/CachedResourceRequest.cpp:
(WebCore::CachedResourceRequest::CachedResourceRequest):
* loader/cache/CachedResourceRequest.h:
(WebCore::CachedResourceRequest::priority):
(WebCore::CachedResourceRequest::forPreload): Deleted.
(WebCore::CachedResourceRequest::setForPreload): Deleted.
(WebCore::CachedResourceRequest::defer): Deleted.
(WebCore::CachedResourceRequest::setDefer): Deleted.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (206899 => 206900)


--- trunk/Source/WebCore/ChangeLog	2016-10-07 05:07:13 UTC (rev 206899)
+++ trunk/Source/WebCore/ChangeLog	2016-10-07 06:00:10 UTC (rev 206900)
@@ -1,3 +1,39 @@
+2016-10-06  Youenn Fablet  <you...@apple.com>
+
+        CachedResourceRequest should not need to store defer and preload options
+        https://bugs.webkit.org/show_bug.cgi?id=163004
+
+        Reviewed by Alex Christensen.
+
+        No change of behavior.
+
+        Removing CachedResourceRequest defer and preload fields.
+        These fields are computed inside CachedResourceLoader instead.
+
+        Updated setting of priority from CachedResourceRequest to CachedResource.
+        Priority is set for any new resource (this covers all cases where no cached resource can be reused from the memory cache).
+        Priority is set for a cached resource if the request is not a preload request.
+
+        * loader/LinkLoader.cpp:
+        (WebCore::LinkLoader::preloadIfNeeded):
+        * loader/cache/CachedResource.cpp:
+        (WebCore::CachedResource::CachedResource):
+        * loader/cache/CachedResourceLoader.cpp:
+        (WebCore::CachedResourceLoader::requestImage):
+        (WebCore::CachedResourceLoader::canRequest):
+        (WebCore::CachedResourceLoader::requestResource):
+        (WebCore::CachedResourceLoader::determineRevalidationPolicy):
+        (WebCore::CachedResourceLoader::requestPreload):
+        * loader/cache/CachedResourceLoader.h:
+        * loader/cache/CachedResourceRequest.cpp:
+        (WebCore::CachedResourceRequest::CachedResourceRequest):
+        * loader/cache/CachedResourceRequest.h:
+        (WebCore::CachedResourceRequest::priority):
+        (WebCore::CachedResourceRequest::forPreload): Deleted.
+        (WebCore::CachedResourceRequest::setForPreload): Deleted.
+        (WebCore::CachedResourceRequest::defer): Deleted.
+        (WebCore::CachedResourceRequest::setDefer): Deleted.
+
 2016-10-06  Myles C. Maxfield  <mmaxfi...@apple.com>
 
         Variation fonts don't affect glyph advances

Modified: trunk/Source/WebCore/loader/LinkLoader.cpp (206899 => 206900)


--- trunk/Source/WebCore/loader/LinkLoader.cpp	2016-10-07 05:07:13 UTC (rev 206899)
+++ trunk/Source/WebCore/loader/LinkLoader.cpp	2016-10-07 06:00:10 UTC (rev 206900)
@@ -162,7 +162,6 @@
     linkRequest.setInitiator("link");
 
     linkRequest.setAsPotentiallyCrossOrigin(crossOriginMode, document);
-    linkRequest.setForPreload(true);
     CachedResourceHandle<CachedResource> cachedLinkResource = document.cachedResourceLoader().preload(type.value(), WTFMove(linkRequest), CachedResourceLoader::ExplicitPreload);
 
     if (cachedLinkResource)

Modified: trunk/Source/WebCore/loader/cache/CachedResource.cpp (206899 => 206900)


--- trunk/Source/WebCore/loader/cache/CachedResource.cpp	2016-10-07 05:07:13 UTC (rev 206899)
+++ trunk/Source/WebCore/loader/cache/CachedResource.cpp	2016-10-07 06:00:10 UTC (rev 206900)
@@ -125,6 +125,8 @@
     , m_type(type)
 {
     ASSERT(sessionID.isValid());
+
+    setLoadPriority(request.priority());
     finishRequestInitialization();
 
     // FIXME: We should have a better way of checking for Navigation loads, maybe FetchMode::Options::Navigate.

Modified: trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp (206899 => 206900)


--- trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp	2016-10-07 05:07:13 UTC (rev 206899)
+++ trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp	2016-10-07 06:00:10 UTC (rev 206900)
@@ -183,14 +183,14 @@
             if (Document* document = frame->document())
                 document->contentSecurityPolicy()->upgradeInsecureRequestIfNeeded(request.mutableResourceRequest(), ContentSecurityPolicy::InsecureRequestType::Load);
             URL requestURL = request.resourceRequest().url();
-            if (requestURL.isValid() && canRequest(CachedResource::ImageResource, requestURL, request))
+            if (requestURL.isValid() && canRequest(CachedResource::ImageResource, requestURL, request, ForPreload::No))
                 PingLoader::loadImage(*frame, requestURL);
             return nullptr;
         }
     }
 
-    request.setDefer(clientDefersImage(request.resourceRequest().url()) ? CachedResourceRequest::DeferredByClient : CachedResourceRequest::NoDefer);
-    return downcast<CachedImage>(requestResource(CachedResource::ImageResource, WTFMove(request)).get());
+    auto defer = clientDefersImage(request.resourceRequest().url()) ? DeferOption::DeferredByClient : DeferOption::NoDefer;
+    return downcast<CachedImage>(requestResource(CachedResource::ImageResource, WTFMove(request), ForPreload::No, defer).get());
 }
 
 CachedResourceHandle<CachedFont> CachedResourceLoader::requestFont(CachedResourceRequest&& request, bool isSVG)
@@ -437,12 +437,12 @@
     return url.protocolIsData() && options.sameOriginDataURLFlag == SameOriginDataURLFlag::Set;
 }
 
-bool CachedResourceLoader::canRequest(CachedResource::Type type, const URL& url, const CachedResourceRequest& request)
+bool CachedResourceLoader::canRequest(CachedResource::Type type, const URL& url, const CachedResourceRequest& request, ForPreload forPreload)
 {
     auto& options = request.options();
 
     if (document() && !document()->securityOrigin()->canDisplay(url)) {
-        if (!request.forPreload())
+        if (forPreload == ForPreload::No)
             FrameLoader::reportLocalLoadFailed(frame(), url.stringCenterEllipsizedToLength());
         LOG(ResourceLoading, "CachedResourceLoader::requestResource URL was not allowed by SecurityOrigin::canDisplay");
         return false;
@@ -634,7 +634,7 @@
     // FIXME: Decide whether to support client hints
 }
 
-CachedResourceHandle<CachedResource> CachedResourceLoader::requestResource(CachedResource::Type type, CachedResourceRequest&& request)
+CachedResourceHandle<CachedResource> CachedResourceLoader::requestResource(CachedResource::Type type, CachedResourceRequest&& request, ForPreload forPreload, DeferOption defer)
 {
     if (Document* document = this->document())
         document->contentSecurityPolicy()->upgradeInsecureRequestIfNeeded(request.mutableResourceRequest(), ContentSecurityPolicy::InsecureRequestType::Load);
@@ -641,7 +641,7 @@
 
     URL url = ""
 
-    LOG(ResourceLoading, "CachedResourceLoader::requestResource '%s', charset '%s', priority=%d, forPreload=%u", url.stringCenterEllipsizedToLength().latin1().data(), request.charset().latin1().data(), request.priority() ? static_cast<int>(request.priority().value()) : -1, request.forPreload());
+    LOG(ResourceLoading, "CachedResourceLoader::requestResource '%s', charset '%s', priority=%d, forPreload=%u", url.stringCenterEllipsizedToLength().latin1().data(), request.charset().latin1().data(), request.priority() ? static_cast<int>(request.priority().value()) : -1, forPreload == ForPreload::Yes);
 
     // If only the fragment identifiers differ, it is the same resource.
     url = ""
@@ -654,7 +654,7 @@
     prepareFetch(type, request);
 
     // We are passing url as well as request, as request url may contain a fragment identifier.
-    if (!canRequest(type, url, request)) {
+    if (!canRequest(type, url, request, forPreload)) {
         RELEASE_LOG_IF_ALLOWED("requestResource: Not allowed to request resource (frame = %p)", frame());
         return nullptr;
     }
@@ -712,13 +712,7 @@
 
     logMemoryCacheResourceRequest(frame(), resource ? DiagnosticLoggingKeys::inMemoryCacheKey() : DiagnosticLoggingKeys::notInMemoryCacheKey());
 
-    // These 3 fields will be used below after request is moved.
-    // FIXME: We can rearrange the code to not require storing all 3 fields.
-    auto forPreload = request.forPreload();
-    auto defer = request.defer();
-    auto priority = request.priority();
-
-    RevalidationPolicy policy = determineRevalidationPolicy(type, request, resource.get());
+    RevalidationPolicy policy = determineRevalidationPolicy(type, request, resource.get(), forPreload, defer);
     switch (policy) {
     case Reload:
         memoryCache.remove(*resource);
@@ -753,6 +747,8 @@
                 m_resourceTimingInfo.addResourceTiming(resource.get(), *document(), loadTiming);
             }
 #endif
+            if (forPreload == ForPreload::No)
+                resource->setLoadPriority(request.priority());
         }
         break;
     }
@@ -760,15 +756,12 @@
     if (!resource)
         return nullptr;
 
-    if (!forPreload || policy != Use)
-        resource->setLoadPriority(priority);
-
-    if (!forPreload && resource->loader() && resource->resourceRequest().ignoreForRequestCount()) {
+    if (forPreload == ForPreload::No && resource->loader() && resource->resourceRequest().ignoreForRequestCount()) {
         resource->resourceRequest().setIgnoreForRequestCount(false);
         incrementRequestCount(*resource);
     }
 
-    if ((policy != Use || resource->stillNeedsLoad()) && CachedResourceRequest::NoDefer == defer) {
+    if ((policy != Use || resource->stillNeedsLoad()) && defer == DeferOption::NoDefer) {
         resource->load(*this);
 
         // We don't support immediate loads, but we do support immediate failure.
@@ -868,7 +861,7 @@
     }
 }
 
-CachedResourceLoader::RevalidationPolicy CachedResourceLoader::determineRevalidationPolicy(CachedResource::Type type, CachedResourceRequest& cachedResourceRequest, CachedResource* existingResource) const
+CachedResourceLoader::RevalidationPolicy CachedResourceLoader::determineRevalidationPolicy(CachedResource::Type type, CachedResourceRequest& cachedResourceRequest, CachedResource* existingResource, ForPreload forPreload, DeferOption defer) const
 {
     auto& request = cachedResourceRequest.resourceRequest();
 
@@ -876,7 +869,7 @@
         return Load;
 
     // We already have a preload going for this URL.
-    if (cachedResourceRequest.forPreload() && existingResource->isPreloaded())
+    if (forPreload == ForPreload::Yes && existingResource->isPreloaded())
         return Use;
 
     // If the same URL has been loaded as a different type, we need to reload.
@@ -905,9 +898,8 @@
     // Conditional requests should have failed canReuse check.
     ASSERT(!request.isConditional());
 
-    // Do not load from cache if images are not enabled. The load for this image will be blocked
-    // in CachedImage::load.
-    if (cachedResourceRequest.defer() == CachedResourceRequest::DeferredByClient)
+    // Do not load from cache if images are not enabled. The load for this image will be blocked in CachedImage::load.
+    if (defer == DeferOption::DeferredByClient)
         return Reload;
 
     // Don't reload resources while pasting.
@@ -1224,9 +1216,8 @@
 {
     if (request.charset().isEmpty() && (type == CachedResource::Script || type == CachedResource::CSSStyleSheet))
         request.setCharset(m_document->charset());
-    request.setForPreload(true);
 
-    CachedResourceHandle<CachedResource> resource = requestResource(type, WTFMove(request));
+    CachedResourceHandle<CachedResource> resource = requestResource(type, WTFMove(request), ForPreload::Yes);
     if (!resource || (m_preloads && m_preloads->contains(resource.get())))
         return nullptr;
     // Fonts need special treatment since just creating the resource doesn't trigger a load.

Modified: trunk/Source/WebCore/loader/cache/CachedResourceLoader.h (206899 => 206900)


--- trunk/Source/WebCore/loader/cache/CachedResourceLoader.h	2016-10-07 05:07:13 UTC (rev 206899)
+++ trunk/Source/WebCore/loader/cache/CachedResourceLoader.h	2016-10-07 06:00:10 UTC (rev 206900)
@@ -136,7 +136,6 @@
     void checkForPendingPreloads();
     void printPreloadStats();
 
-    bool canRequest(CachedResource::Type, const URL&, const CachedResourceRequest&);
     bool canRequestAfterRedirection(CachedResource::Type, const URL&, const ResourceLoaderOptions&);
 
     static const ResourceLoaderOptions& defaultCachedResourceOptions();
@@ -152,14 +151,19 @@
 private:
     explicit CachedResourceLoader(DocumentLoader*);
 
-    CachedResourceHandle<CachedResource> requestResource(CachedResource::Type, CachedResourceRequest&&);
+    enum class ForPreload { Yes, No };
+    enum class DeferOption { NoDefer, DeferredByClient };
+
+    CachedResourceHandle<CachedResource> requestResource(CachedResource::Type, CachedResourceRequest&&, ForPreload = ForPreload::No, DeferOption = DeferOption::NoDefer);
     void prepareFetch(CachedResource::Type, CachedResourceRequest&);
     CachedResourceHandle<CachedResource> revalidateResource(CachedResourceRequest&&, CachedResource&);
     CachedResourceHandle<CachedResource> loadResource(CachedResource::Type, CachedResourceRequest&&);
     CachedResourceHandle<CachedResource> requestPreload(CachedResource::Type, CachedResourceRequest&&);
 
+    bool canRequest(CachedResource::Type, const URL&, const CachedResourceRequest&, ForPreload);
+
     enum RevalidationPolicy { Use, Revalidate, Reload, Load };
-    RevalidationPolicy determineRevalidationPolicy(CachedResource::Type, CachedResourceRequest&, CachedResource* existingResource) const;
+    RevalidationPolicy determineRevalidationPolicy(CachedResource::Type, CachedResourceRequest&, CachedResource* existingResource, ForPreload, DeferOption) const;
 
     bool shouldUpdateCachedResourceWithCurrentRequest(const CachedResource&, const CachedResourceRequest&);
     CachedResourceHandle<CachedResource> updateCachedResourceWithCurrentRequest(const CachedResource&, CachedResourceRequest&&);

Modified: trunk/Source/WebCore/loader/cache/CachedResourceRequest.cpp (206899 => 206900)


--- trunk/Source/WebCore/loader/cache/CachedResourceRequest.cpp	2016-10-07 05:07:13 UTC (rev 206899)
+++ trunk/Source/WebCore/loader/cache/CachedResourceRequest.cpp	2016-10-07 06:00:10 UTC (rev 206900)
@@ -39,8 +39,6 @@
     , m_charset(charset)
     , m_options(CachedResourceLoader::defaultCachedResourceOptions())
     , m_priority(priority)
-    , m_forPreload(false)
-    , m_defer(NoDefer)
 {
 }
 
@@ -48,8 +46,6 @@
     : m_resourceRequest(WTFMove(resourceRequest))
     , m_options(options)
     , m_priority(priority)
-    , m_forPreload(false)
-    , m_defer(NoDefer)
 {
 }
 

Modified: trunk/Source/WebCore/loader/cache/CachedResourceRequest.h (206899 => 206900)


--- trunk/Source/WebCore/loader/cache/CachedResourceRequest.h	2016-10-07 05:07:13 UTC (rev 206899)
+++ trunk/Source/WebCore/loader/cache/CachedResourceRequest.h	2016-10-07 06:00:10 UTC (rev 206900)
@@ -40,8 +40,6 @@
 
 class CachedResourceRequest {
 public:
-    enum DeferOption { NoDefer, DeferredByClient };
-
     explicit CachedResourceRequest(const ResourceRequest&, const String& charset = String(), Optional<ResourceLoadPriority> = Nullopt);
     CachedResourceRequest(ResourceRequest&&, const ResourceLoaderOptions&, Optional<ResourceLoadPriority> = Nullopt);
 
@@ -52,10 +50,6 @@
     const ResourceLoaderOptions& options() const { return m_options; }
     void setOptions(const ResourceLoaderOptions& options) { m_options = options; }
     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; }
-    void setDefer(DeferOption defer) { m_defer = defer; }
     void setInitiator(PassRefPtr<Element>);
     void setInitiator(const AtomicString& name);
     const AtomicString& initiatorName() const;
@@ -72,8 +66,6 @@
     String m_charset;
     ResourceLoaderOptions m_options;
     Optional<ResourceLoadPriority> m_priority;
-    bool m_forPreload;
-    DeferOption m_defer;
     RefPtr<Element> m_initiatorElement;
     AtomicString m_initiatorName;
     RefPtr<SecurityOrigin> m_origin;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to