Changeset 260622 in webkit

Timestamp:

Apr 23, 2020 11:51:19 PM (4 years ago)

Author:

commit-queue@webkit.org

Message:

Make CachedResourceLoader more conforming to Fetch specification
​https://bugs.webkit.org/show_bug.cgi?id=210925

Patch by Rob Buis <​rbuis@igalia.com> on 2020-04-23
Reviewed by Alex Christensen.

Make CachedResourceLoader more conforming to Fetch specification
by fixing links, re-ordering steps to match main fetch [1] and do
early exit code paths earlier.

• loader/cache/CachedResourceLoader.cpp:

(WebCore::CachedResourceLoader::requestImage): adjust to parameter change.
(WebCore::CachedResourceLoader::canRequest): replace CachedResourceRequest param with ResourceLoaderOptions.
(WebCore::CachedResourceLoader::prepareFetch): fix comment.
(WebCore::CachedResourceLoader::requestResource): re-order.

• loader/cache/CachedResourceLoader.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• loader/cache/CachedResourceLoader.cpp (modified) (6 diffs)
• loader/cache/CachedResourceLoader.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-04-23  Rob Buis  <rbuis@igalia.com>
+
+        Make CachedResourceLoader more conforming to Fetch specification
+        https://bugs.webkit.org/show_bug.cgi?id=210925
+
+        Reviewed by Alex Christensen.
+
+        Make CachedResourceLoader more conforming to Fetch specification
+        by fixing links, re-ordering steps to match main fetch [1] and do
+        early exit code paths earlier.
+
+        * loader/cache/CachedResourceLoader.cpp:
+        (WebCore::CachedResourceLoader::requestImage): adjust to parameter change.
+        (WebCore::CachedResourceLoader::canRequest): replace CachedResourceRequest param with ResourceLoaderOptions.
+        (WebCore::CachedResourceLoader::prepareFetch): fix comment.
+        (WebCore::CachedResourceLoader::requestResource): re-order.
+        * loader/cache/CachedResourceLoader.h:
+
 2020-04-23  Simon Fraser  <simon.fraser@apple.com>
 

trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp

@@ -206 +206 @@
                 request.upgradeInsecureRequestIfNeeded(*document);
             URL requestURL = request.resourceRequest().url();
-            if (requestURL.isValid() && canRequest(CachedResource::Type::ImageResource, requestURL, request, ForPreload::No))
+            if (requestURL.isValid() && canRequest(CachedResource::Type::ImageResource, requestURL, request.options(), ForPreload::No))
                 PingLoader::loadImage(*frame, requestURL);
             return CachedResourceHandle<CachedImage> { };
@@ -498 +498 @@
 }
 
-// Security checks defined in https://fetch.spec.whatwg.org/#main-fetch step 2 and 5.
-bool CachedResourceLoader::canRequest(CachedResource::Type type, const URL& url, const CachedResourceRequest& request, ForPreload forPreload)
-{
-    auto& options = request.options();
-
+// Security checks defined in https://fetch.spec.whatwg.org/#main-fetch step 2 and 4.
+bool CachedResourceLoader::canRequest(CachedResource::Type type, const URL& url, const ResourceLoaderOptions& options, ForPreload forPreload)
+{
     if (document() && !document()->securityOrigin().canDisplay(url)) {
         if (forPreload == ForPreload::No)
@@ -716 +714 @@
 void CachedResourceLoader::prepareFetch(CachedResource::Type type, CachedResourceRequest& request)
 {
-    // Implementing step 1 to 7 of https://fetch.spec.whatwg.org/#fetching
-    auto* document = this->document();
-
-    if (document) {
+    // Implementing step 1.1 to 1.6 of https://fetch.spec.whatwg.org/#concept-fetch.
+    if (auto* document = this->document()) {
         if (!request.origin())
             request.setOrigin(document->securityOrigin());
@@ -806 +802 @@
 ResourceErrorOr<CachedResourceHandle<CachedResource>> CachedResourceLoader::requestResource(CachedResource::Type type, CachedResourceRequest&& request, ForPreload forPreload, ImageLoading imageLoading)
 {
+    URL url = request.resourceRequest().url();
     if (!frame() || !frame()->page()) {
         RELEASE_LOG_IF_ALLOWED("requestResource: failed because no frame or page");
-        return makeUnexpected(ResourceError { errorDomainWebKitInternal, 0, request.resourceRequest().url(), "Invalid loader state"_s });
-    }
+        return makeUnexpected(ResourceError { errorDomainWebKitInternal, 0, url, "Invalid loader state"_s });
+    }
+
     auto& frame = *this->frame();
-    auto& page = *frame.page();
+    if (!url.isValid()) {
+        RELEASE_LOG_IF_ALLOWED_WITH_FRAME("requestResource: URL is invalid", frame);
+        return makeUnexpected(ResourceError { errorDomainWebKitInternal, 0, url, "URL is invalid"_s });
+    }
+
+    LOG(ResourceLoading, "CachedResourceLoader::requestResource '%.255s', 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);
 
     request.setDestinationIfNotSet(destinationForType(type));
@@ -823 +826 @@
     }
 
-    if (Document* document = this->document())
-        request.upgradeInsecureRequestIfNeeded(*document);
-
-    if (InspectorInstrumentation::willInterceptRequest(&frame, request.resourceRequest()))
-        request.setCachingPolicy(CachingPolicy::DisallowCaching);
-
-    URL url = request.resourceRequest().url();
+    prepareFetch(type, request);
+
     if (request.options().destination == FetchOptions::Destination::Document) {
         // FIXME: Identify HSTS cases and avoid adding the header. <https://bugs.webkit.org/show_bug.cgi?id=157885>
@@ -836 +834 @@
     }
 
-    request.updateReferrerPolicy(document() ? document()->referrerPolicy() : ReferrerPolicy::NoReferrerWhenDowngrade);
-
-    LOG(ResourceLoading, "CachedResourceLoader::requestResource '%.255s', 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 (!url.isValid()) {
-        RELEASE_LOG_IF_ALLOWED_WITH_FRAME("requestResource: URL is invalid", frame);
-        return makeUnexpected(ResourceError { errorDomainWebKitInternal, 0, url, "URL is invalid"_s });
-    }
-
-    prepareFetch(type, request);
+    if (Document* document = this->document()) {
+        request.upgradeInsecureRequestIfNeeded(*document);
+        url = request.resourceRequest().url();
+    }
 
     // We are passing url as well as request, as request url may contain a fragment identifier.
-    if (!canRequest(type, url, request, forPreload)) {
+    if (!canRequest(type, url, request.options(), forPreload)) {
         RELEASE_LOG_IF_ALLOWED_WITH_FRAME("requestResource: Not allowed to request resource", frame);
         return makeUnexpected(ResourceError { errorDomainWebKitInternal, 0, url, "Not allowed to request resource"_s, ResourceError::Type::AccessControl });
     }
+
+    request.updateReferrerPolicy(document() ? document()->referrerPolicy() : ReferrerPolicy::NoReferrerWhenDowngrade);
+
+    if (InspectorInstrumentation::willInterceptRequest(&frame, request.resourceRequest()))
+        request.setCachingPolicy(CachingPolicy::DisallowCaching);
+
+    auto& page = *frame.page();
 
 #if ENABLE(CONTENT_EXTENSIONS)

trunk/Source/WebCore/loader/cache/CachedResourceLoader.h

@@ -178 +178 @@
     void updateHTTPRequestHeaders(FrameLoader&, CachedResource::Type, CachedResourceRequest&);
 
-    bool canRequest(CachedResource::Type, const URL&, const CachedResourceRequest&, ForPreload);
+    bool canRequest(CachedResource::Type, const URL&, const ResourceLoaderOptions&, ForPreload);
 
     enum RevalidationPolicy { Use, Revalidate, Reload, Load };