Use no-cache fetch mode when loading main documents with location.reload()
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 11 Jan 2018 00:45:49 +0000 (00:45 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 11 Jan 2018 00:45:49 +0000 (00:45 +0000)
https://bugs.webkit.org/show_bug.cgi?id=181285
LayoutTests/imported/w3c:

Patch by Youenn Fablet <youenn@apple.com> on 2018-01-10
Reviewed by Alex Christensen.

* web-platform-tests/service-workers/service-worker/fetch-event.https-expected.txt:

Source/WebCore:

Patch by Youenn Fablet <youenn@apple.com> on 2018-01-10
Reviewed by Alex Christensen.

Covered by rebased tests.

Start to translate cache policy used for navigation as FetchOptions::Cache.
This allows ensuring service workers receive the right cache mode when intercepting navigation loads.
To not change current navigation behavior, ReturnCacheDataElseLoad and ReturnCacheDataDontLoad still trigger default fetch cache mode.

For Reload and ReloadExpiredOnly frame load types, using no-cache mode is more efficient than reload mode,
as a conditional request will be sent if possible. This applies to location.reload which is consistent with other browsers.
Keep reload mode for ReloadFromOrigin.

* loader/DocumentLoader.cpp:
(WebCore::toFetchOptionsCache):
(WebCore::DocumentLoader::loadMainResource):
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::loadFrameRequest):
(WebCore::FrameLoader::loadURL):
(WebCore::FrameLoader::load):
(WebCore::FrameLoader::reload):
(WebCore::FrameLoader::defaultRequestCachingPolicy):
(WebCore::FrameLoader::loadDifferentDocumentItem):
* loader/NavigationScheduler.cpp:

LayoutTests:

<rdar://problem/36356831>

Patch by Youenn Fablet <youenn@apple.com> on 2018-01-10
Reviewed by Alex Christensen.

* http/tests/inspector/network/har/har-page-expected.txt:
* http/tests/inspector/network/har/har-page.html:

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

LayoutTests/ChangeLog
LayoutTests/http/tests/inspector/network/har/har-page-expected.txt
LayoutTests/http/tests/inspector/network/har/har-page.html
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/fetch-event.https-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/loader/DocumentLoader.cpp
Source/WebCore/loader/FrameLoader.cpp
Source/WebCore/loader/NavigationScheduler.cpp

index b8c5800716dd19f34700c075dc95d1da844ed410..8f4e783d27191e0899d6da40e6f360057cc650dc 100644 (file)
@@ -1,3 +1,14 @@
+2018-01-10  Youenn Fablet  <youenn@apple.com>
+
+        Use no-cache fetch mode when loading main documents with location.reload()
+        https://bugs.webkit.org/show_bug.cgi?id=181285
+        <rdar://problem/36356831>
+
+        Reviewed by Alex Christensen.
+
+        * http/tests/inspector/network/har/har-page-expected.txt:
+        * http/tests/inspector/network/har/har-page.html:
+
 2018-01-10  Per Arne Vollan  <pvollan@apple.com>
 
         Mark accessibility/table-header-calculation-for-header-rows.html as failure on Windows.
index 4e164eaf5076a7dcc675b5926e99d1410c45c19c..13908770828cfe836953a85abbc3a9204f5acee9 100644 (file)
@@ -43,10 +43,10 @@ HAR Page Test.
           "cookies": [],
           "headers": "<filtered>",
           "content": {
-            "size": 3042,
+            "size": "<filtered>",
             "compression": 0,
             "mimeType": "text/html",
-            "text": "<filtered text (3042)>"
+            "text": "<filtered text (3171)>"
           },
           "redirectURL": "",
           "headersSize": "<filtered>",
@@ -84,7 +84,7 @@ HAR Page Test.
           "cookies": [],
           "headers": "<filtered>",
           "content": {
-            "size": 0,
+            "size": "<filtered>",
             "compression": 0,
             "mimeType": "application/x-javascript",
             "text": "<filtered text (7039)>"
index 3bb99370971e6d60f5cbda395ca77df044742cbe..52e4da241309c86c6ef809ac70d5d3d479494322 100644 (file)
@@ -42,6 +42,9 @@ function test()
         // Reduce the file contents.
         if (key === "text")
             return "<filtered text (" + value.length + ")>";
+        // Reduce the file size since cache may or may not be used.
+        if (key === "size")
+            return "<filtered>";
 
         // Since cache may or may not be used, timing data may be variable.
         // NOTE: SSL should always be -1 for this test case.
index 88f0f72d2e4fb1513025968a50b2a0c0ac927c3e..e915577296d0a11db9f39ba3743938d77a2aee38 100644 (file)
@@ -1,3 +1,12 @@
+2018-01-10  Youenn Fablet  <youenn@apple.com>
+
+        Use no-cache fetch mode when loading main documents with location.reload()
+        https://bugs.webkit.org/show_bug.cgi?id=181285
+
+        Reviewed by Alex Christensen.
+
+        * web-platform-tests/service-workers/service-worker/fetch-event.https-expected.txt:
+
 2018-01-09  Chris Dumez  <cdumez@apple.com>
 
         We should not return undefined for most properties of a detached Window
index de4173004522026b5dfb4286436755e0ff5d1478..24e5ce4377c550a743fbbe821d65ad791b64a86d 100644 (file)
@@ -1,5 +1,4 @@
 
-
 PASS Service Worker headers in the request of a fetch event 
 PASS Service Worker responds to fetch event with string 
 PASS Service Worker responds to fetch event with blob body 
@@ -12,7 +11,7 @@ PASS Service Worker responds to fetch event with POST form
 PASS Multiple calls of respondWith must throw InvalidStateErrors 
 PASS Service Worker event.respondWith must set the used flag 
 PASS Service Worker should expose FetchEvent URL fragments. 
-FAIL Service Worker responds to fetch event with the correct cache types assert_unreached: unexpected rejection: assert_equals: expected "no-cache" but got "default" Reached unreachable code
+PASS Service Worker responds to fetch event with the correct cache types 
 PASS Service Worker should intercept EventSource 
 PASS Service Worker responds to fetch event with the correct integrity_metadata 
 PASS FetchEvent#body is a string 
index e847ccbbe9127e9134f35b209e7cc107ae710c08..3304196415991c56528902f2d3194fd489aa1c6f 100644 (file)
@@ -1,3 +1,32 @@
+2018-01-10  Youenn Fablet  <youenn@apple.com>
+
+        Use no-cache fetch mode when loading main documents with location.reload()
+        https://bugs.webkit.org/show_bug.cgi?id=181285
+
+        Reviewed by Alex Christensen.
+
+        Covered by rebased tests.
+
+        Start to translate cache policy used for navigation as FetchOptions::Cache.
+        This allows ensuring service workers receive the right cache mode when intercepting navigation loads.
+        To not change current navigation behavior, ReturnCacheDataElseLoad and ReturnCacheDataDontLoad still trigger default fetch cache mode.
+
+        For Reload and ReloadExpiredOnly frame load types, using no-cache mode is more efficient than reload mode,
+        as a conditional request will be sent if possible. This applies to location.reload which is consistent with other browsers.
+        Keep reload mode for ReloadFromOrigin.
+
+        * loader/DocumentLoader.cpp:
+        (WebCore::toFetchOptionsCache):
+        (WebCore::DocumentLoader::loadMainResource):
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::loadFrameRequest):
+        (WebCore::FrameLoader::loadURL):
+        (WebCore::FrameLoader::load):
+        (WebCore::FrameLoader::reload):
+        (WebCore::FrameLoader::defaultRequestCachingPolicy):
+        (WebCore::FrameLoader::loadDifferentDocumentItem):
+        * loader/NavigationScheduler.cpp:
+
 2018-01-10  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r226667 and r226673.
index 298d6507cc15b7be36640dc8626125258aa93bf3..a82c25bc018513f78555b8b29823822962799a5a 100644 (file)
@@ -1607,9 +1607,32 @@ void DocumentLoader::startLoadingMainResource()
     });
 }
 
+static inline FetchOptions::Cache toFetchOptionsCache(ResourceRequestCachePolicy policy)
+{
+    // We are setting FetchOptions::Cache values to keep current behavior consistency.
+    // FIXME: We should merge FetchOptions::Cache with ResourceRequestCachePolicy and merge related class members.
+    switch (policy) {
+    case UseProtocolCachePolicy:
+        return FetchOptions::Cache::Default;
+    case ReloadIgnoringCacheData:
+        return FetchOptions::Cache::Reload;
+    case ReturnCacheDataElseLoad:
+        return FetchOptions::Cache::Default;
+    case ReturnCacheDataDontLoad:
+        return FetchOptions::Cache::Default;
+    case DoNotUseAnyCache:
+        return FetchOptions::Cache::NoStore;
+    case RefreshAnyCacheData:
+        return FetchOptions::Cache::NoCache;
+    }
+    return FetchOptions::Cache::Default;
+}
+
 void DocumentLoader::loadMainResource(ResourceRequest&& request)
 {
-    static NeverDestroyed<ResourceLoaderOptions> mainResourceLoadOptions(SendCallbacks, SniffContent, BufferData, StoredCredentialsPolicy::Use, ClientCredentialPolicy::MayAskClientForCredentials, FetchOptions::Credentials::Include, SkipSecurityCheck, FetchOptions::Mode::Navigate, IncludeCertificateInfo, ContentSecurityPolicyImposition::SkipPolicyCheck, DefersLoadingPolicy::AllowDefersLoading, CachingPolicy::AllowCaching);
+    ResourceLoaderOptions mainResourceLoadOptions { SendCallbacks, SniffContent, BufferData, StoredCredentialsPolicy::Use, ClientCredentialPolicy::MayAskClientForCredentials, FetchOptions::Credentials::Include, SkipSecurityCheck, FetchOptions::Mode::Navigate, IncludeCertificateInfo, ContentSecurityPolicyImposition::SkipPolicyCheck, DefersLoadingPolicy::AllowDefersLoading, CachingPolicy::AllowCaching };
+    mainResourceLoadOptions.cache = toFetchOptionsCache(request.cachePolicy());
+
     CachedResourceRequest mainResourceRequest(ResourceRequest(request), mainResourceLoadOptions);
     if (!m_frame->isMainFrame() && m_frame->document()) {
         // If we are loading the main resource of a subframe, use the cache partition of the main document.
index 4c40576913821267ae12d83cb4c7e30841345b2f..7ecd7c8f5e45a774e27ff2fb3dd32dfb435463d0 100644 (file)
@@ -1193,10 +1193,12 @@ void FrameLoader::loadFrameRequest(FrameLoadRequest&& request, Event* event, For
         referrer = String();
 
     FrameLoadType loadType;
-    if (request.resourceRequest().cachePolicy() == ReloadIgnoringCacheData)
+    if (request.resourceRequest().cachePolicy() == RefreshAnyCacheData)
         loadType = FrameLoadType::Reload;
     else if (request.lockBackForwardList() == LockBackForwardList::Yes)
         loadType = FrameLoadType::RedirectWithLockedBackForwardList;
+    else if (request.resourceRequest().cachePolicy() == ReloadIgnoringCacheData)
+        loadType = FrameLoadType::ReloadFromOrigin;
     else
         loadType = FrameLoadType::Standard;
 
@@ -1281,7 +1283,7 @@ void FrameLoader::loadURL(FrameLoadRequest&& frameLoadRequest, const String& ref
 
     addExtraFieldsToRequest(request, newLoadType, true);
     if (isReload(newLoadType))
-        request.setCachePolicy(ReloadIgnoringCacheData);
+        request.setCachePolicy(RefreshAnyCacheData);
 
     ASSERT(newLoadType != FrameLoadType::Same);
 
@@ -1416,7 +1418,7 @@ void FrameLoader::load(DocumentLoader* newDocumentLoader)
     FrameLoadType type;
 
     if (shouldTreatURLAsSameAsCurrent(newDocumentLoader->originalRequest().url())) {
-        r.setCachePolicy(ReloadIgnoringCacheData);
+        r.setCachePolicy(RefreshAnyCacheData);
         type = FrameLoadType::Same;
     } else if (shouldTreatURLAsSameAsCurrent(newDocumentLoader->unreachableURL()) && m_loadType == FrameLoadType::Reload)
         type = FrameLoadType::Reload;
@@ -1641,8 +1643,7 @@ void FrameLoader::reload(OptionSet<ReloadOption> options)
     
     ResourceRequest& request = loader->request();
 
-    // FIXME: We don't have a mechanism to revalidate the main resource without reloading at the moment.
-    request.setCachePolicy(ReloadIgnoringCacheData);
+    request.setCachePolicy(RefreshAnyCacheData);
 
     // If we're about to re-post, set up action so the application can warn the user.
     if (request.httpMethod() == "POST")
@@ -2614,13 +2615,13 @@ ResourceRequestCachePolicy FrameLoader::defaultRequestCachingPolicy(const Resour
 
     if (isMainResource) {
         if (isReload(loadType) || request.isConditional())
-            return ReloadIgnoringCacheData;
+            return loadType == FrameLoadType::ReloadFromOrigin ? ReloadIgnoringCacheData : RefreshAnyCacheData;
 
         return UseProtocolCachePolicy;
     }
 
     if (request.isConditional())
-        return ReloadIgnoringCacheData;
+        return RefreshAnyCacheData;
 
     if (documentLoader()->isLoadingInAPISense()) {
         // If we inherit cache policy from a main resource, we use the DocumentLoader's
@@ -3478,8 +3479,10 @@ void FrameLoader::loadDifferentDocumentItem(HistoryItem& item, FrameLoadType loa
     } else {
         switch (loadType) {
         case FrameLoadType::Reload:
-        case FrameLoadType::ReloadFromOrigin:
         case FrameLoadType::ReloadExpiredOnly:
+            request.setCachePolicy(RefreshAnyCacheData);
+            break;
+        case FrameLoadType::ReloadFromOrigin:
             request.setCachePolicy(ReloadIgnoringCacheData);
             break;
         case FrameLoadType::Back:
index d73b1b50eb6abb1e3da786e1b3529f25cfbd5d6a..b1f5703d2b396971469a6c87cd36cea3e6d8846d 100644 (file)
@@ -182,7 +182,7 @@ public:
         UserGestureIndicator gestureIndicator { userGestureToForward() };
 
         bool refresh = equalIgnoringFragmentIdentifier(frame.document()->url(), url());
-        ResourceRequest resourceRequest { url(), referrer(), refresh ? ReloadIgnoringCacheData : UseProtocolCachePolicy };
+        ResourceRequest resourceRequest { url(), referrer(), refresh ? RefreshAnyCacheData : UseProtocolCachePolicy };
         FrameLoadRequest frameLoadRequest { initiatingDocument(), *securityOrigin(), resourceRequest, "_self", lockHistory(), lockBackForwardList(), MaybeSendReferrer, AllowNavigationToInvalidURL::No, NewFrameOpenerPolicy::Allow, shouldOpenExternalURLs(), initiatedByMainFrame() };
 
         frame.loader().changeLocation(WTFMove(frameLoadRequest));
@@ -216,7 +216,7 @@ public:
     {
         UserGestureIndicator gestureIndicator { userGestureToForward() };
 
-        ResourceRequest resourceRequest { url(), referrer(), ReloadIgnoringCacheData };
+        ResourceRequest resourceRequest { url(), referrer(), RefreshAnyCacheData };
         FrameLoadRequest frameLoadRequest { initiatingDocument(), *securityOrigin(), resourceRequest, "_self", lockHistory(), lockBackForwardList(), MaybeSendReferrer, AllowNavigationToInvalidURL::Yes, NewFrameOpenerPolicy::Allow, shouldOpenExternalURLs(), initiatedByMainFrame() };
 
         frame.loader().changeLocation(WTFMove(frameLoadRequest));
@@ -319,7 +319,7 @@ public:
         ResourceResponse replacementResponse { m_originDocument.url(), ASCIILiteral("text/plain"), 0, ASCIILiteral("UTF-8") };
         SubstituteData replacementData { SharedBuffer::create(), m_originDocument.url(), replacementResponse, SubstituteData::SessionHistoryVisibility::Hidden };
 
-        ResourceRequest resourceRequest { m_originDocument.url(), emptyString(), ReloadIgnoringCacheData };
+        ResourceRequest resourceRequest { m_originDocument.url(), emptyString(), RefreshAnyCacheData };
         FrameLoadRequest frameLoadRequest { m_originDocument, m_originDocument.securityOrigin(), resourceRequest, { }, lockHistory(), lockBackForwardList(), MaybeSendReferrer, AllowNavigationToInvalidURL::Yes, NewFrameOpenerPolicy::Allow, shouldOpenExternalURLs(), initiatedByMainFrame() };
         frameLoadRequest.setSubstituteData(replacementData);
         frame.loader().load(WTFMove(frameLoadRequest));