window.stop() should fire abort events on XMLHttpRequest asynchronously

https://bugs.webkit.org/show_bug.cgi?id=257397 Reviewed by Darin Adler and Youenn Fablet. window.stop() should fire an abort event on XMLHttpRequest asynchronously, to match the behavior of Blink and Gecko. It should schedule a asynchronous task, which should get cancelled if the JavaScript calls `open()` on the XMLHttpRequest again. This is covered by WPT tests. * LayoutTests/imported/w3c/web-platform-tests/xhr/abort-after-stop.window-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/xhr/open-after-stop.window-expected.txt: Rebaseline tests now that more checks are passing. * LayoutTests/imported/w3c/web-platform-tests/xhr/send-authentication-basic-cors-not-enabled-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/xhr/send-authentication-basic-cors-not-enabled.htm: * LayoutTests/imported/w3c/web-platform-tests/xhr/send-authentication-cors-basic-setrequestheader-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/xhr/send-authentication-cors-basic-setrequestheader.htm: * LayoutTests/imported/w3c/web-platform-tests/xhr/send-authentication-cors-setrequestheader-no-cred-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/xhr/send-authentication-cors-setrequestheader-no-cred.htm: * LayoutTests/imported/w3c/web-platform-tests/xhr/send-network-error-sync-events.sub-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/xhr/send-network-error-sync-events.sub.htm: Fix tests to use get_host_info() instead of hardcoding custom domains which don't work with the WebKit layout tests infrastructure. This improves test coverage. I'll upstream those changes. * Source/WebCore/xml/XMLHttpRequest.cpp: (WebCore::XMLHttpRequest::XMLHttpRequest): (WebCore::XMLHttpRequest::open): (WebCore::XMLHttpRequest::abort): (WebCore::XMLHttpRequest::internalAbort): (WebCore::XMLHttpRequest::abortError): (WebCore::XMLHttpRequest::handleCancellation): (WebCore::XMLHttpRequest::didFail): * Source/WebCore/xml/XMLHttpRequest.h: Align our behavior with Blink and Gecko. Canonical link: https://commits.webkit.org/264765@main
WebKit · May 31, 2023 · 02e1d3e · 02e1d3e
1 parent 91f5e58
commit 02e1d3e
Show file tree

Hide file tree

Showing 14 changed files with 38 additions and 28 deletions.
diff --git a/LayoutTests/imported/w3c/web-platform-tests/xhr/abort-after-stop.window-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/xhr/abort-after-stop.window-expected.txt
@@ -1,3 +1,3 @@
 
-FAIL XMLHttpRequest: abort event should fire when stop() method is used assert_equals: expected true but got false
+PASS XMLHttpRequest: abort event should fire when stop() method is used
 
diff --git a/LayoutTests/imported/w3c/web-platform-tests/xhr/open-after-stop.window-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/xhr/open-after-stop.window-expected.txt
@@ -1,3 +1,3 @@
 
-FAIL open() after window.stop() assert_unreached: loadend should not be fired after window.stop() and open() Reached unreachable code
+PASS open() after window.stop()
 
diff --git a/...ported/w3c/web-platform-tests/xhr/send-authentication-basic-cors-not-enabled-expected.txt b/...ported/w3c/web-platform-tests/xhr/send-authentication-basic-cors-not-enabled-expected.txt
@@ -1,4 +1,3 @@
-Blocked access to external URL http://www1.localhost:8800/xhr/resources/auth10/auth.py
 
 PASS XMLHttpRequest: send() - "Basic" authenticated CORS requests with user name and password passed to open() (asserts failure)
 
diff --git a/...tTests/imported/w3c/web-platform-tests/xhr/send-authentication-basic-cors-not-enabled.htm b/...tTests/imported/w3c/web-platform-tests/xhr/send-authentication-basic-cors-not-enabled.htm
@@ -5,6 +5,7 @@
     <script src="/resources/testharness.js"></script>
     <script src="/resources/testharnessreport.js"></script>
     <script src="/common/utils.js"></script>
+    <script src="/common/get-host-info.sub.js"></script>
     <link rel="help" href="https://xhr.spec.whatwg.org/#the-open()-method" data-tested-assertations="following::ol[1]/li[9]/ol[1]/li[1] following::ol[1]/li[9]/ol[1]/li[2]" />
     <link rel="help" href="https://xhr.spec.whatwg.org/#the-send()-method" data-tested-assertations="following::code[contains(@title,'http-authorization')]/.." />
   </head>
@@ -13,10 +14,10 @@
     <script>
       test(function() {
         var client = new XMLHttpRequest(),
-          urlstart = 'www1.'+location.host + location.pathname.replace(/\/[^\/]*$/, '/')
+          urlstart = get_host_info().REMOTE_ORIGIN + location.pathname.replace(/\/[^\/]*$/, '/')
         client.withCredentials = true
         user = token()
-        client.open("GET", location.protocol+'//'+urlstart + "resources/auth10/auth.py", false, user, 'pass')
+        client.open("GET", urlstart + "resources/auth10/auth.py", false, user, 'pass')
         client.setRequestHeader("x-user", user)
         assert_throws_dom("NetworkError", function(){ client.send(null) })
         assert_equals(client.responseText, '')

diff --git a/...d/w3c/web-platform-tests/xhr/send-authentication-cors-basic-setrequestheader-expected.txt b/...d/w3c/web-platform-tests/xhr/send-authentication-cors-basic-setrequestheader-expected.txt
@@ -1,4 +1,3 @@
-Blocked access to external URL http://www1.localhost:8800/xhr/resources/auth2/corsenabled.py
 
-FAIL XMLHttpRequest: send() - "Basic" authenticated CORS request using setRequestHeader() (expects to succeed)  A network error occurred.
+PASS XMLHttpRequest: send() - "Basic" authenticated CORS request using setRequestHeader() (expects to succeed)
 
diff --git a/...s/imported/w3c/web-platform-tests/xhr/send-authentication-cors-basic-setrequestheader.htm b/...s/imported/w3c/web-platform-tests/xhr/send-authentication-cors-basic-setrequestheader.htm
@@ -5,15 +5,16 @@
     <script src="/resources/testharness.js"></script>
     <script src="/resources/testharnessreport.js"></script>
     <script src="/common/utils.js"></script>
+    <script src="/common/get-host-info.sub.js"></script>
   </head>
   <body>
     <div id="log"></div>
     <script>
       async_test(test => {
         var client = new XMLHttpRequest(),
-            urlstart = location.host + location.pathname.replace(/\/[^\/]*$/, '/'),
+            urlstart = get_host_info().REMOTE_ORIGIN + location.pathname.replace(/\/[^\/]*$/, '/'),
             user = token()
-        client.open("GET", location.protocol+'//www1.'+urlstart + "resources/auth2/corsenabled.py", false)
+        client.open("GET", urlstart + "resources/auth2/corsenabled.py", false)
         client.withCredentials = true
         client.setRequestHeader("x-user", user)
         client.setRequestHeader("x-pass", 'pass')

diff --git a/...w3c/web-platform-tests/xhr/send-authentication-cors-setrequestheader-no-cred-expected.txt b/...w3c/web-platform-tests/xhr/send-authentication-cors-setrequestheader-no-cred-expected.txt
@@ -1,6 +1,4 @@
-Blocked access to external URL http://www1.localhost:8800/xhr/resources/auth7/corsenabled.py
-Blocked access to external URL http://www1.localhost:8800/xhr/resources/auth8/corsenabled-no-authorize.py
 
-FAIL CORS request with setRequestHeader auth to URL accepting Authorization header assert_true: responseText should contain the right user and password expected true got false
+PASS CORS request with setRequestHeader auth to URL accepting Authorization header
 PASS CORS request with setRequestHeader auth to URL NOT accepting Authorization header
 
diff --git a/...imported/w3c/web-platform-tests/xhr/send-authentication-cors-setrequestheader-no-cred.htm b/...imported/w3c/web-platform-tests/xhr/send-authentication-cors-setrequestheader-no-cred.htm
@@ -5,6 +5,7 @@
     <script src="/resources/testharness.js"></script>
     <script src="/resources/testharnessreport.js"></script>
     <script src="/common/utils.js"></script>
+    <script src="/common/get-host-info.sub.js"></script>
     <!-- These spec references do not make much sense simply because the spec doesn't say very much about this.. -->
     <link rel="help" href="https://xhr.spec.whatwg.org/#the-setrequestheader()-method" data-tested-assertations="following::ol[1]/li[6]" />
     <link rel="help" href="https://xhr.spec.whatwg.org/#the-send()-method" data-tested-assertations="following::code[contains(@title,'http-authorization')]/.." />
@@ -16,9 +17,9 @@
       var test = async_test(desc)
       test.step(function() {
         var client = new XMLHttpRequest(),
-            urlstart = location.host + location.pathname.replace(/\/[^\/]*$/, '/'),
+            urlstart = get_host_info().REMOTE_ORIGIN + location.pathname.replace(/\/[^\/]*$/, '/'),
             user = token()
-        client.open("GET", location.protocol + "//www1." + urlstart + "resources/" + pathsuffix, false)
+        client.open("GET", urlstart + "resources/" + pathsuffix, false)
         client.setRequestHeader("x-user", user)
         client.setRequestHeader("x-pass", 'pass')
         client.setRequestHeader("Authorization", "Basic " + btoa(user + ":pass"))

diff --git a/...Tests/imported/w3c/web-platform-tests/xhr/send-network-error-sync-events.sub-expected.txt b/...Tests/imported/w3c/web-platform-tests/xhr/send-network-error-sync-events.sub-expected.txt
@@ -1,4 +1,3 @@
-Blocked access to external URL http://nonexistent.localhost:8800/
 
 PASS http URL
 PASS data URL

diff --git a/LayoutTests/imported/w3c/web-platform-tests/xhr/send-network-error-sync-events.sub.htm b/LayoutTests/imported/w3c/web-platform-tests/xhr/send-network-error-sync-events.sub.htm
@@ -17,7 +17,7 @@
         {
             var xhr = new XMLHttpRequest();
 
-            xhr.open("POST", "http://nonexistent.{{host}}:{{ports[http][0]}}", false);
+            xhr.open("POST", "http://{{host}}:1", false); // Bad port.
 
             assert_throws_dom("NetworkError", function()
             {

diff --git a/Source/WebCore/page/LocalDOMWindow.cpp b/Source/WebCore/page/LocalDOMWindow.cpp
@@ -1149,6 +1149,7 @@ void LocalDOMWindow::stop()
     if (!frame)
         return;
 
+    SetForScope isStopping { m_isStopping, true };
     // We must check whether the load is complete asynchronously, because we might still be parsing
     // the document until the callstack unwinds.
     frame->loader().stopForUserCancel(true);

diff --git a/Source/WebCore/page/LocalDOMWindow.h b/Source/WebCore/page/LocalDOMWindow.h
@@ -192,6 +192,7 @@ class LocalDOMWindow final
     void close(Document&);
     void print();
     void stop();
+    bool isStopping() const { return m_isStopping; }
 
     WEBCORE_EXPORT ExceptionOr<RefPtr<WindowProxy>> open(LocalDOMWindow& activeWindow, LocalDOMWindow& firstWindow, const String& urlString, const AtomString& frameName, const String& windowFeaturesString);
 
@@ -489,6 +490,7 @@ class LocalDOMWindow final
 
     bool m_wasWrappedWithoutInitializedSecurityOrigin { false };
     bool m_mayReuseForNavigation { true };
+    bool m_isStopping { false };
 #if ENABLE(USER_MESSAGE_HANDLERS)
     mutable RefPtr<WebKitNamespace> m_webkitNamespace;
 #endif

diff --git a/Source/WebCore/xml/XMLHttpRequest.cpp b/Source/WebCore/xml/XMLHttpRequest.cpp
@@ -114,7 +114,6 @@ XMLHttpRequest::XMLHttpRequest(ScriptExecutionContext& context)
     , m_error(false)
     , m_uploadListenerFlag(false)
     , m_uploadComplete(false)
-    , m_wasAbortedByClient(false)
     , m_responseCacheIsValid(false)
     , m_readyState(static_cast<unsigned>(UNSENT))
     , m_responseType(static_cast<unsigned>(ResponseType::EmptyString))
@@ -135,8 +134,7 @@ XMLHttpRequest::~XMLHttpRequest()
 
 Document* XMLHttpRequest::document() const
 {
-    ASSERT(scriptExecutionContext());
-    return downcast<Document>(scriptExecutionContext());
+    return dynamicDowncast<Document>(scriptExecutionContext());
 }
 
 SecurityOrigin* XMLHttpRequest::securityOrigin() const
@@ -377,7 +375,6 @@ ExceptionOr<void> XMLHttpRequest::open(const String& method, const URL& url, boo
     m_method = normalizeHTTPMethod(method);
     m_error = false;
     m_uploadComplete = false;
-    m_wasAbortedByClient = false;
 
     // clear stuff from possible previous load
     clearResponse();
@@ -682,7 +679,6 @@ void XMLHttpRequest::abort()
 {
     Ref<XMLHttpRequest> protectedThis(*this);
 
-    m_wasAbortedByClient = true;
     if (!internalAbort())
         return;
 
@@ -706,6 +702,7 @@ bool XMLHttpRequest::internalAbort()
     m_receivedLength = 0;
 
     m_decoder = nullptr;
+    m_abortErrorGroup.cancel();
 
     m_timeoutTimer.stop();
 
@@ -769,7 +766,6 @@ void XMLHttpRequest::networkError()
 
 void XMLHttpRequest::abortError()
 {
-    ASSERT(m_wasAbortedByClient);
     genericError();
     dispatchErrorEvents(eventNames().abortEvent);
 }
@@ -889,18 +885,29 @@ String XMLHttpRequest::statusText() const
     return m_response.httpStatusText();
 }
 
-void XMLHttpRequest::didFail(const ResourceError& error)
+void XMLHttpRequest::handleCancellation()
 {
-    Ref protectedThis { *this };
+    m_exceptionCode = AbortError;
+    queueTaskKeepingObjectAlive(*this, TaskSource::Networking, CancellableTask(m_abortErrorGroup, [this] {
+        abortError();
+    }));
+}
 
+void XMLHttpRequest::didFail(const ResourceError& error)
+{
     // If we are already in an error state, for instance we called abort(), bail out early.
     if (m_error)
         return;
 
+    bool wasAbortedByClient = false;
+    if (auto* document = this->document()) {
+        if (auto* window = document->domWindow())
+            wasAbortedByClient = window->isStopping();
+    }
+
     // The XHR specification says we should only fire an abort event if the cancelation was requested by the client.
-    if (m_wasAbortedByClient && error.isCancellation()) {
-        m_exceptionCode = AbortError;
-        abortError();
+    if (wasAbortedByClient && error.isCancellation()) {
+        handleCancellation();
         return;
     }
 

diff --git a/Source/WebCore/xml/XMLHttpRequest.h b/Source/WebCore/xml/XMLHttpRequest.h
@@ -33,6 +33,7 @@
 #include "XMLHttpRequestEventTarget.h"
 #include "XMLHttpRequestProgressEventThrottle.h"
 #include <variant>
+#include <wtf/CancellableTask.h>
 #include <wtf/text/StringBuilder.h>
 
 namespace JSC {
@@ -139,6 +140,7 @@ class XMLHttpRequest final : public ActiveDOMObject, public RefCounted<XMLHttpRe
     explicit XMLHttpRequest(ScriptExecutionContext&);
 
     void updateHasRelevantEventListener();
+    void handleCancellation();
 
     // EventTarget.
     void eventListenersDidChange() final;
@@ -207,7 +209,6 @@ class XMLHttpRequest final : public ActiveDOMObject, public RefCounted<XMLHttpRe
     unsigned m_error : 1;
     unsigned m_uploadListenerFlag : 1;
     unsigned m_uploadComplete : 1;
-    unsigned m_wasAbortedByClient : 1;
     unsigned m_responseCacheIsValid : 1;
     unsigned m_readyState : 3; // State
     unsigned m_responseType : 3; // ResponseType
@@ -254,6 +255,7 @@ class XMLHttpRequest final : public ActiveDOMObject, public RefCounted<XMLHttpRe
     std::optional<ExceptionCode> m_exceptionCode;
     RefPtr<UserGestureToken> m_userGestureToken;
     std::atomic<bool> m_hasRelevantEventListener;
+    TaskCancellationGroup m_abortErrorGroup;
     bool m_wasDidSendDataCalledForTotalBytes { false };
 };