Diff
Modified: trunk/LayoutTests/ChangeLog (289532 => 289533)
--- trunk/LayoutTests/ChangeLog 2022-02-10 15:33:42 UTC (rev 289532)
+++ trunk/LayoutTests/ChangeLog 2022-02-10 15:45:10 UTC (rev 289533)
@@ -1,3 +1,14 @@
+2022-02-10 Youenn Fablet <[email protected]>
+
+ Settling of a fetch promise should be delayed in case page is entering page cache
+ https://bugs.webkit.org/show_bug.cgi?id=236292
+ <rdar://88452971>
+
+ Reviewed by Chris Dumez.
+
+ * http/tests/navigation/fetch-pagecache-expected.txt: Added.
+ * http/tests/navigation/fetch-pagecache.html: Added.
+
2022-02-10 Antti Koivisto <[email protected]>
[:has() pseudo-class] Nullptr crash with non-function :has
Added: trunk/LayoutTests/http/tests/navigation/fetch-pagecache-expected.txt (0 => 289533)
--- trunk/LayoutTests/http/tests/navigation/fetch-pagecache-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/navigation/fetch-pagecache-expected.txt 2022-02-10 15:45:10 UTC (rev 289533)
@@ -0,0 +1,14 @@
+Tests that a page with a loading XMLHttpRequest goes into the page cache.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+pageshow - not from cache
+pagehide - entering cache
+pageshow - from cache
+PASS Page did enter and was restored from the page cache
+PASS fetch promise resolved after page was restored from the page cache
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/http/tests/navigation/fetch-pagecache.html (0 => 289533)
--- trunk/LayoutTests/http/tests/navigation/fetch-pagecache.html (rev 0)
+++ trunk/LayoutTests/http/tests/navigation/fetch-pagecache.html 2022-02-10 15:45:10 UTC (rev 289533)
@@ -0,0 +1,52 @@
+<!doctype html><!-- webkit-test-runner [ dumpJSConsoleLogInStdErr=true UsesBackForwardCache=true ] -->
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<script>
+description('Tests that a page with a loading XMLHttpRequest goes into the page cache.');
+window.jsTestIsAsync = true;
+
+var restoredFromPageCache = false;
+
+function doFetchTest()
+{
+ if (restoredFromPageCache) {
+ testPassed("fetch promise resolved after page was restored from the page cache");
+ finishJSTest();
+ return;
+ }
+ fetch("/resources/load-and-stall.cgi?name=load-and-stall.cgi&stallFor=3&stallAt=0&mimeType=text/plain").then(doFetchTest, doFetchTest);
+}
+
+window.addEventListener("pageshow", function(event) {
+ debug("pageshow - " + (event.persisted ? "" : "not ") + "from cache");
+
+ if (event.persisted) {
+ testPassed("Page did enter and was restored from the page cache");
+ restoredFromPageCache = true;
+ }
+}, false);
+
+window.addEventListener("pagehide", function(event) {
+ debug("pagehide - " + (event.persisted ? "" : "not ") + "entering cache");
+ if (!event.persisted) {
+ testFailed("Page did not enter the page cache.");
+ finishJSTest();
+ }
+ doFetchTest();
+}, false);
+
+window.addEventListener('load', function() {
+ // This needs to happen in a setTimeout because a navigation inside the onload handler would
+ // not create a history entry.
+ setTimeout(function() {
+ // Force a back navigation back to this page.
+ window.location.href = ""
+ }, 0);
+}, false);
+
+</script>
+<script src=""
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (289532 => 289533)
--- trunk/Source/WebCore/ChangeLog 2022-02-10 15:33:42 UTC (rev 289532)
+++ trunk/Source/WebCore/ChangeLog 2022-02-10 15:45:10 UTC (rev 289533)
@@ -1,3 +1,23 @@
+2022-02-10 Youenn Fablet <[email protected]>
+
+ Settling of a fetch promise should be delayed in case page is entering page cache
+ https://bugs.webkit.org/show_bug.cgi?id=236292
+ <rdar://88452971>
+
+ Reviewed by Chris Dumez.
+
+ Make sure to enqueue a task before resolving fetch promise as otherwise, page might continue running _javascript_.
+ Do this for worker as well for good measure.
+ We move signal aborted checks to two clients to handle rejecting fetch promise synchronously.
+
+ Test: http/tests/navigation/fetch-pagecache.html
+
+ * Modules/cache/DOMCache.cpp:
+ * Modules/fetch/FetchResponse.cpp:
+ * Modules/fetch/FetchResponse.h:
+ * Modules/fetch/WindowOrWorkerGlobalScopeFetch.cpp:
+ * workers/service/FetchEvent.cpp:
+
2022-02-10 Chris Dumez <[email protected]>
Fail synchronously when constructing a SharedWorker with an URL that is not same-origin
Modified: trunk/Source/WebCore/Modules/cache/DOMCache.cpp (289532 => 289533)
--- trunk/Source/WebCore/Modules/cache/DOMCache.cpp 2022-02-10 15:33:42 UTC (rev 289532)
+++ trunk/Source/WebCore/Modules/cache/DOMCache.cpp 2022-02-10 15:45:10 UTC (rev 289533)
@@ -259,7 +259,11 @@
for (auto& request : requests) {
auto& requestReference = request.get();
- FetchResponse::fetch(*scriptExecutionContext(), requestReference, [this, request = WTFMove(request), taskHandler](ExceptionOr<FetchResponse&>&& result) mutable {
+ if (requestReference.signal().aborted()) {
+ taskHandler->error(Exception { AbortError, "Request signal is aborted"_s });
+ return;
+ }
+ FetchResponse::fetch(*scriptExecutionContext(), requestReference, [this, request = WTFMove(request), taskHandler](auto&& result) mutable {
if (taskHandler->isDone())
return;
@@ -269,7 +273,8 @@
return;
}
- auto& response = result.releaseReturnValue();
+ auto protectedResponse = result.releaseReturnValue();
+ auto& response = protectedResponse.get();
if (!response.ok()) {
taskHandler->error(Exception { TypeError, "Response is not OK"_s });
@@ -295,7 +300,7 @@
}
size_t recordPosition = taskHandler->addRecord(toConnectionRecord(request.get(), response, nullptr));
- response.consumeBodyReceivedByChunk([taskHandler = WTFMove(taskHandler), recordPosition, data = "" response = Ref { response }] (auto&& result) mutable {
+ response.consumeBodyReceivedByChunk([taskHandler = WTFMove(taskHandler), recordPosition, data = "" response = WTFMove(protectedResponse)] (auto&& result) mutable {
if (taskHandler->isDone())
return;
Modified: trunk/Source/WebCore/Modules/fetch/FetchResponse.cpp (289532 => 289533)
--- trunk/Source/WebCore/Modules/fetch/FetchResponse.cpp 2022-02-10 15:33:42 UTC (rev 289532)
+++ trunk/Source/WebCore/Modules/fetch/FetchResponse.cpp 2022-02-10 15:45:10 UTC (rev 289533)
@@ -237,12 +237,6 @@
void FetchResponse::fetch(ScriptExecutionContext& context, FetchRequest& request, NotificationCallback&& responseCallback, const String& initiator)
{
- if (request.signal().aborted()) {
- responseCallback(Exception { AbortError, "Request signal is aborted"_s });
- // FIXME: Cancel request body if it is a stream.
- return;
- }
-
if (request.isReadableStreamBody()) {
responseCallback(Exception { NotSupportedError, "ReadableStream uploading is not supported"_s });
return;
@@ -355,7 +349,7 @@
m_response.updateContentType();
if (auto responseCallback = WTFMove(m_responseCallback))
- responseCallback(m_response);
+ responseCallback(Ref { m_response });
}
void FetchResponse::BodyLoader::didReceiveData(const SharedBuffer& buffer)
Modified: trunk/Source/WebCore/Modules/fetch/FetchResponse.h (289532 => 289533)
--- trunk/Source/WebCore/Modules/fetch/FetchResponse.h 2022-02-10 15:33:42 UTC (rev 289532)
+++ trunk/Source/WebCore/Modules/fetch/FetchResponse.h 2022-02-10 15:45:10 UTC (rev 289533)
@@ -63,7 +63,7 @@
static Ref<FetchResponse> error(ScriptExecutionContext&);
static ExceptionOr<Ref<FetchResponse>> redirect(ScriptExecutionContext&, const String& url, int status);
- using NotificationCallback = Function<void(ExceptionOr<FetchResponse&>&&)>;
+ using NotificationCallback = Function<void(ExceptionOr<Ref<FetchResponse>>&&)>;
static void fetch(ScriptExecutionContext&, FetchRequest&, NotificationCallback&&, const String& initiator);
void startConsumingStream(unsigned);
Modified: trunk/Source/WebCore/Modules/fetch/WindowOrWorkerGlobalScopeFetch.cpp (289532 => 289533)
--- trunk/Source/WebCore/Modules/fetch/WindowOrWorkerGlobalScopeFetch.cpp 2022-02-10 15:33:42 UTC (rev 289532)
+++ trunk/Source/WebCore/Modules/fetch/WindowOrWorkerGlobalScopeFetch.cpp 2022-02-10 15:45:10 UTC (rev 289533)
@@ -29,6 +29,7 @@
#include "CachedResourceRequestInitiators.h"
#include "DOMWindow.h"
#include "Document.h"
+#include "EventLoop.h"
#include "FetchResponse.h"
#include "JSDOMPromiseDeferred.h"
#include "JSFetchResponse.h"
@@ -39,46 +40,41 @@
using FetchResponsePromise = DOMPromiseDeferred<IDLInterface<FetchResponse>>;
-void WindowOrWorkerGlobalScopeFetch::fetch(DOMWindow& window, FetchRequest::Info&& input, FetchRequest::Init&& init, Ref<DeferredPromise>&& deferred)
+// https://fetch.spec.whatwg.org/#dom-global-fetch
+static void doFetch(ScriptExecutionContext& scope, FetchRequest::Info&& input, FetchRequest::Init&& init, FetchResponsePromise&& promise)
{
- FetchResponsePromise promise = WTFMove(deferred);
-
- auto* document = window.document();
- if (!document) {
- promise.reject(InvalidStateError);
+ auto requestOrException = FetchRequest::create(scope, WTFMove(input), WTFMove(init));
+ if (requestOrException.hasException()) {
+ promise.reject(requestOrException.releaseException());
return;
}
- auto request = FetchRequest::create(*document, WTFMove(input), WTFMove(init));
- if (request.hasException()) {
- promise.reject(request.releaseException());
+ auto request = requestOrException.releaseReturnValue();
+ if (request->signal().aborted()) {
+ promise.reject(Exception { AbortError, "Request signal is aborted"_s });
return;
}
- FetchResponse::fetch(*document, request.releaseReturnValue(), [promise = WTFMove(promise), userGestureToken = UserGestureIndicator::currentUserGesture()](ExceptionOr<FetchResponse&>&& result) mutable {
- if (!userGestureToken || userGestureToken->hasExpired(UserGestureToken::maximumIntervalForUserGestureForwardingForFetch()) || !userGestureToken->processingUserGesture()) {
+ FetchResponse::fetch(scope, request.get(), [promise = WTFMove(promise), scope = Ref { scope }](auto&& result) mutable {
+ scope->eventLoop().queueTask(TaskSource::Networking, [promise = WTFMove(promise), result = WTFMove(result)]() mutable {
promise.settle(WTFMove(result));
- return;
- }
-
- UserGestureIndicator gestureIndicator(userGestureToken, UserGestureToken::GestureScope::MediaOnly, UserGestureToken::IsPropagatedFromFetch::Yes);
- promise.settle(WTFMove(result));
+ });
}, cachedResourceRequestInitiators().fetch);
}
-void WindowOrWorkerGlobalScopeFetch::fetch(WorkerGlobalScope& scope, FetchRequest::Info&& input, FetchRequest::Init&& init, Ref<DeferredPromise>&& deferred)
+void WindowOrWorkerGlobalScopeFetch::fetch(DOMWindow& window, FetchRequest::Info&& input, FetchRequest::Init&& init, Ref<DeferredPromise>&& promise)
{
- FetchResponsePromise promise = WTFMove(deferred);
-
- auto request = FetchRequest::create(scope, WTFMove(input), WTFMove(init));
- if (request.hasException()) {
- promise.reject(request.releaseException());
+ auto* document = window.document();
+ if (!document) {
+ promise->reject(InvalidStateError);
return;
}
+ doFetch(*document, WTFMove(input), WTFMove(init), WTFMove(promise));
+}
- FetchResponse::fetch(scope, request.releaseReturnValue().get(), [promise = WTFMove(promise)](ExceptionOr<FetchResponse&>&& result) mutable {
- promise.settle(WTFMove(result));
- }, cachedResourceRequestInitiators().fetch);
+void WindowOrWorkerGlobalScopeFetch::fetch(WorkerGlobalScope& scope, FetchRequest::Info&& input, FetchRequest::Init&& init, Ref<DeferredPromise>&& promise)
+{
+ doFetch(scope, WTFMove(input), WTFMove(init), WTFMove(promise));
}
}
