Skip to content

GitLab

T
tangled
Commit 177405e7 authored by Timothy Gu's avatar Timothy Gu Committed by Commit Bot
Browse files
Options

Fire XHR abort event from a task 

A test for this new behavior is included as
external/wpt/xhr/abort-after-stop.htm.

This also means that XHR abort and readystatechange events from
navigation is no longer fired. Because of that,
external/wpt/xhr/open-url-multi-window-4.htm now times out rather than
fails on an assertion, aligning with Firefox. A TestExpectations entry
has been added with a bug that tracks fixing this test in spec or
upstream in WPT. A few other test expectations were updated as well to
account for this.

http/tests/navigation/reentrant-xhr-onabort-crash-during-commit.html no
longer works as intended, as abort event is no longer fired
synchronously (and thus does not expose the same crash surface). It is
replaced with an EventSource-based test that functions the same way as
XHR before this CL. Note, currently EventSource's error event suffers
from the same problem as XHR's abort event, and the class might undergo
the same change as this in the future. We will look into an alternative
for this test when that change is done.

Bug: 879620, 881180
Change-Id: I5a91047086d06347794656f92511a53c22401b5e
Reviewed-on: https://chromium-review.googlesource.com/1208672Reviewed-by: default avatarPhilip Jägenstedt <foolip@chromium.org>
Reviewed-by: default avatarYutaka Hirano <yhirano@chromium.org>
Commit-Queue: Timothy Gu <timothygu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589634}
parent 34610c1d
Hide whitespace changes
Inline Side-by-side
Showing with 169 additions and 40 deletions
third_party/WebKit/LayoutTests/TestExpectations
View file @ 177405e7
...@@ -3726,6 +3726,11 @@ crbug.com/785940 external/wpt/web-animations/timing-model/timelines/document-tim ...@@ -3726,6 +3726,11 @@ crbug.com/785940 external/wpt/web-animations/timing-model/timelines/document-tim
# Decoding test timeout on Win7. Marked flaky to get imported in case it's flaky timeout everywhere. # Decoding test timeout on Win7. Marked flaky to get imported in case it's flaky timeout everywhere.
crbug.com/862938 external/wpt/encoding/textdecoder-fatal-single-byte.any.worker.html [ Pass Timeout ] crbug.com/862938 external/wpt/encoding/textdecoder-fatal-single-byte.any.worker.html [ Pass Timeout ]
# Unclear if XHR events should still be fired after its frame is discarded.
crbug.com/881180 external/wpt/xhr/open-url-multi-window-4.htm [ Timeout ]
crbug.com/881180 virtual/outofblink-cors/external/wpt/xhr/open-url-multi-window-4.htm [ Timeout ]
crbug.com/881180 virtual/outofblink-cors-ns/external/wpt/xhr/open-url-multi-window-4.htm [ Timeout ]
crbug.com/655458 external/wpt/workers/constructors/SharedWorker/undefined-arguments.html [ Failure ] crbug.com/655458 external/wpt/workers/constructors/SharedWorker/undefined-arguments.html [ Failure ]
crbug.com/655458 external/wpt/workers/interfaces/SharedWorkerGlobalScope/onconnect.html [ Failure ] crbug.com/655458 external/wpt/workers/interfaces/SharedWorkerGlobalScope/onconnect.html [ Failure ]
crbug.com/655458 external/wpt/workers/constructors/SharedWorker/setting-port-members.html [ Failure ] crbug.com/655458 external/wpt/workers/constructors/SharedWorker/setting-port-members.html [ Failure ]
......
third_party/WebKit/LayoutTests/external/wpt/xhr/abort-after-stop.htm
View file @ 177405e7
...@@ -13,7 +13,9 @@ ...@@ -13,7 +13,9 @@
window.onload = test.step_func(function() { window.onload = test.step_func(function() {
var client = new XMLHttpRequest(); var client = new XMLHttpRequest();
var abortFired = false; var abortFired = false;
var sync = true;
client.onabort = test.step_func(function (e) { client.onabort = test.step_func(function (e) {
assert_false(sync);
assert_equals(e.type, 'abort'); assert_equals(e.type, 'abort');
abortFired = true; abortFired = true;
}); });
...@@ -24,6 +26,7 @@ ...@@ -24,6 +26,7 @@
test.done(); test.done();
}, 200); }, 200);
window.stop(); window.stop();
sync = false;
}); });
</script> </script>
</body> </body>
......
third_party/WebKit/LayoutTests/external/wpt/xhr/open-after-abort.htm
View file @ 177405e7
...@@ -9,27 +9,69 @@ ...@@ -9,27 +9,69 @@
<body> <body>
<div id="log"></div> <div id="log"></div>
<script> <script>
var test = async_test() test(function(t) {
test.step(function() {
var client = new XMLHttpRequest(), var client = new XMLHttpRequest(),
result = [], result = [],
expected = [1, 4, 1] // open() -> 1, expected = [
// abort() -> 4, open() -> 1 'readystatechange', 0, 1, // open()
client.onreadystatechange = function() { 'readystatechange', 2, 4, // abort()
test.step(function() { 'abort', 2, 4, // abort()
result.push(client.readyState) 'loadend', 2, 4, // abort()
}) 'readystatechange', 3, 1, // open()
} ]
var state = 0
client.onreadystatechange = t.step_func(function() {
result.push('readystatechange', state, client.readyState)
})
client.onabort = t.step_func(function() {
// abort event must be fired synchronously from abort().
assert_equals(state, 2)
// readystatechange should be fired before abort.
assert_array_equals(result, [
'readystatechange', 0, 1, // open()
'readystatechange', 2, 4, // abort()
])
// readyState should be set to unsent (0) at the very end of abort(),
// after this (and onloadend) is called.
assert_equals(client.readyState, 4)
result.push('abort', state, client.readyState)
})
client.onloadend = t.step_func(function() {
// abort event must be fired synchronously from abort().
assert_equals(state, 2)
// readystatechange should be fired before abort.
assert_array_equals(result, [
'readystatechange', 0, 1, // open()
'readystatechange', 2, 4, // abort()
'abort', 2, 4, // abort()
])
// readyState should be set to unsent (0) at the very end of abort(),
// after this is called.
assert_equals(client.readyState, 4)
result.push('loadend', state, client.readyState)
})
client.open("GET", "resources/well-formed.xml") client.open("GET", "resources/well-formed.xml")
assert_equals(client.readyState, 1) assert_equals(client.readyState, 1)
state = 1
client.send(null) client.send(null)
state = 2
client.abort() client.abort()
assert_equals(client.readyState, 0) assert_equals(client.readyState, 0)
state = 3
client.open("GET", "resources/well-formed.xml") client.open("GET", "resources/well-formed.xml")
assert_equals(client.readyState, 1) assert_equals(client.readyState, 1)
assert_array_equals(result, expected) assert_array_equals(result, expected)
}) })
test.done()
</script> </script>
</body> </body>
</html> </html>
third_party/WebKit/LayoutTests/external/wpt/xhr/open-after-stop.window.js 0 → 100644
View file @ 177405e7
// window.stop() below prevents the load event from firing, so wait until it is
// fired to start the test.
setup({explicit_done: true });
onload = () => {
async_test(function(t) {
const client = new XMLHttpRequest();
const result = [];
const expected = [
'readystatechange', 0, 1, // open()
];
let state = 0;
client.onreadystatechange = t.step_func(() => {
result.push('readystatechange', state, client.readyState);
});
client.onabort = t.unreached_func("abort should not be fired after window.stop() and open()");
client.onloadend = t.unreached_func("loadend should not be fired after window.stop() and open()");
client.open("GET", "resources/well-formed.xml");
assert_equals(client.readyState, 1);
state = 1;
client.send(null);
state = 2;
window.stop();
// Unlike client.abort(), window.stop() does not change readyState
// immediately, rather through a task...
assert_equals(client.readyState, 1);
state = 3;
// ... which is then canceled when we open a new request anyway.
client.open("GET", "resources/well-formed.xml");
assert_equals(client.readyState, 1);
assert_array_equals(result, expected);
// Give the abort and loadend events a chance to fire (erroneously) before
// calling this a success.
t.step_timeout(t.step_func_done(), 1000);
}, "open() after window.stop()");
done();
};
third_party/WebKit/LayoutTests/external/wpt/xhr/open-url-multi-window-4-expected.txt deleted 100644 → 0
View file @ 34610c1d
This is a testharness.js-based test.
FAIL XMLHttpRequest: open() resolving URLs (multi-Window; 4; evil) assert_true: should get an error event expected true got false
Harness: the test ran to completion.
third_party/WebKit/LayoutTests/fast/dom/Document/open-with-pending-load2-expected.txt
View file @ 177405e7
This tests that calling document.open on a document that has a pending load correctly cancels the load and does not crash even if the frame is removed. This tests that calling document.open on a document that has a pending load correctly cancels the load and does not crash even if the frame is removed.
third_party/WebKit/LayoutTests/http/tests/navigation/reentrant-eventsource-onerror-crash-during-commit-expected.txt 0 → 100644
View file @ 177405e7
This test creates a child iframe, which in turn creates an EventSource then reloads. During commit for the reload, the EventSource is cancelled, and the EventSource's onerror handler in turn creates a new EventSource, which is then aborted slightly later during the commit process. This second EventSource's onerror handler detaches the iframe. This will cause the iframe to detach in the middle of commit, and shouldn't crash.
third_party/WebKit/LayoutTests/http/tests/navigation/reentrant-xhr-onabort-crash-during-commit.html → third_party/WebKit/LayoutTests/http/tests/navigation/reentrant-eventsource-onerror-crash-during-commit.html
View file @ 177405e7
<body> <body>
This test creates a child iframe, which in turn creates a slow XHR then reloads. This test creates a child iframe, which in turn creates an EventSource then
During commit for the reload, the slow XHR is cancelled, and the XHR's onabort reloads. During commit for the reload, the EventSource is cancelled, and the
handler in turn creates a new XHR, which is then aborted slightly later during EventSource's onerror handler in turn creates a new EventSource, which is then
the commit process. This second XHR's onabort handler detaches the iframe. This aborted slightly later during the commit process. This second EventSource's
will cause the iframe to detach in the middle of commit, and shouldn't crash. onerror handler detaches the iframe. This will cause the iframe to detach in
the middle of commit, and shouldn't crash.
<script> <script>
if (window.testRunner) { if (window.testRunner) {
testRunner.dumpAsText(); testRunner.dumpAsText();
...@@ -17,5 +18,5 @@ function finish() { ...@@ -17,5 +18,5 @@ function finish() {
}, 0); }, 0);
} }
</script> </script>
<iframe src="resources/reentrant-xhr-onabort-crash-during-commit-iframe.html"></iframe> <iframe src="resources/reentrant-eventsource-onerror-crash-during-commit-iframe.html"></iframe>
</body> </body>
third_party/WebKit/LayoutTests/http/tests/navigation/reentrant-xhr-onabort-crash-during-commit-expected.txt deleted 100644 → 0
View file @ 34610c1d
This test creates a child iframe, which in turn creates a slow XHR then reloads. During commit for the reload, the slow XHR is cancelled, and the XHR's onabort handler in turn creates a new XHR, which is then aborted slightly later during the commit process. This second XHR's onabort handler detaches the iframe. This will cause the iframe to detach in the middle of commit, and shouldn't crash.
third_party/WebKit/LayoutTests/http/tests/navigation/resources/event-stream-with-delay.php 0 → 100644
View file @ 177405e7
<?php
header("Content-Type: text/event-stream");
?>
data: first
<?php
sleep(2)
?>
data: second
third_party/WebKit/LayoutTests/http/tests/navigation/resources/reentrant-eventsource-onerror-crash-during-commit-iframe.html 0 → 100644
View file @ 177405e7
<body>
<script>
var es = new EventSource("event-stream-with-delay.php");
es.onerror = function() {
var es2 = new EventSource("event-stream-with-delay.php");
es2.onerror = function() {
top.finish();
frameElement.remove();
}
}
es.onopen = function() {
location.reload();
}
</script>
</body>
third_party/WebKit/LayoutTests/http/tests/navigation/resources/reentrant-xhr-onabort-crash-during-commit-iframe.html deleted 100644 → 0
View file @ 34610c1d
<body>
<script>
var x = new XMLHttpRequest();
x.onabort = function() {
var x2 = new XMLHttpRequest();
x2.onabort = function() {
top.finish();
frameElement.remove();
}
x2.open("GET", "slow-resource.pl");
x2.send();
}
x.open("GET", "slow-resource.pl");
x.send();
location.reload();
</script>
</body>
third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/frame-load-cancelled-abort-expected.txt
View file @ 177405e7
...@@ -8,7 +8,6 @@ That didFinishedLoading() call should immediately exit and not update the object ...@@ -8,7 +8,6 @@ That didFinishedLoading() call should immediately exit and not update the object
Loading subframe to cancel Loading subframe to cancel
Ready State: 1 Ready State: 1
Body of subframe is loaded. XMLHttpRequest should be in progress. Nuking the iframe Body of subframe is loaded. XMLHttpRequest should be in progress. Nuking the iframe
Ready State: 4
Iframe unloaded Iframe unloaded
Iframe removed Iframe removed
third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/frame-unload-abort-crash-expected.txt
View file @ 177405e7
...@@ -3,5 +3,4 @@ Test for bug 25394: crash in DocumentLoader::addResponse due to bad |this| point ...@@ -3,5 +3,4 @@ Test for bug 25394: crash in DocumentLoader::addResponse due to bad |this| point
You should see a few messages followed by PASSED once. You should see a few messages followed by PASSED once.
Ready State: 1 Ready State: 1
Ready State: 4
PASSED PASSED
third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/navigation-should-abort-expected.txt
View file @ 177405e7
CONSOLE MESSAGE: line 16: PASS: Expected 'abort', got 'abort'.
If this text shows up, you've successfully navigated to 'navigation-target.html'. If this text shows up, you've successfully navigated to 'navigation-target.html'.
third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/navigation-should-abort.html
View file @ 177405e7
...@@ -10,10 +10,10 @@ ...@@ -10,10 +10,10 @@
var req = new XMLHttpRequest(); var req = new XMLHttpRequest();
req.open("GET", "resources/endlessxml.php"); req.open("GET", "resources/endlessxml.php");
req.onerror = function () { req.onerror = function () {
console.log("FAIL: Expected 'abort', got 'error'."); console.log("FAIL: Expected no event after navigation.");
}; };
req.onabort = function () { req.onabort = function () {
console.log("PASS: Expected 'abort', got 'abort'."); console.log("FAIL: Expected no event after navigation.");
}; };
req.send(null); req.send(null);
...@@ -21,6 +21,6 @@ ...@@ -21,6 +21,6 @@
</script> </script>
</head> </head>
<body> <body>
<p>This test requires TestRunner. To execute it manually, open the console, and navigate to <a href="resources/navigation-target.html">resources/navigation-target.html</a>. You should see a "PASS" message in the console.</p> <p>This test requires TestRunner. To execute it manually, open the console, and navigate to <a href="resources/navigation-target.html">resources/navigation-target.html</a>. You should see no log messages in the console.</p>
</body> </body>
</html> </html>
third_party/blink/renderer/core/xmlhttprequest/xml_http_request.cc
View file @ 177405e7
...@@ -1200,6 +1200,11 @@ void XMLHttpRequest::ClearVariablesForLoading() { ...@@ -1200,6 +1200,11 @@ void XMLHttpRequest::ClearVariablesForLoading() {
} }
bool XMLHttpRequest::InternalAbort() { bool XMLHttpRequest::InternalAbort() {
// If there is an existing pending abort event, cancel it. The caller of this
// function is responsible for firing any events on XMLHttpRequest, if
// needed.
pending_abort_event_.Cancel();
// Fast path for repeated internalAbort()s; this // Fast path for repeated internalAbort()s; this
// will happen if an XHR object is notified of context // will happen if an XHR object is notified of context
// destruction followed by finalization. // destruction followed by finalization.
...@@ -1318,8 +1323,11 @@ void XMLHttpRequest::HandleDidCancel() { ...@@ -1318,8 +1323,11 @@ void XMLHttpRequest::HandleDidCancel() {
if (!InternalAbort()) if (!InternalAbort())
return; return;
HandleRequestError(DOMExceptionCode::kAbortError, EventTypeNames::abort, pending_abort_event_ = PostCancellableTask(
received_length, expected_length); *GetExecutionContext()->GetTaskRunner(TaskType::kNetworking), FROM_HERE,
WTF::Bind(&XMLHttpRequest::HandleRequestError, WrapPersistent(this),
DOMExceptionCode::kAbortError, EventTypeNames::abort,
received_length, expected_length));
} }
void XMLHttpRequest::HandleRequestError(DOMExceptionCode exception_code, void XMLHttpRequest::HandleRequestError(DOMExceptionCode exception_code,
......
third_party/blink/renderer/core/xmlhttprequest/xml_http_request.h
View file @ 177405e7
...@@ -41,6 +41,7 @@ ...@@ -41,6 +41,7 @@
#include "third_party/blink/renderer/platform/loader/fetch/resource_response.h" #include "third_party/blink/renderer/platform/loader/fetch/resource_response.h"
#include "third_party/blink/renderer/platform/network/encoded_form_data.h" #include "third_party/blink/renderer/platform/network/encoded_form_data.h"
#include "third_party/blink/renderer/platform/network/http_header_map.h" #include "third_party/blink/renderer/platform/network/http_header_map.h"
#include "third_party/blink/renderer/platform/web_task_runner.h"
#include "third_party/blink/renderer/platform/weborigin/kurl.h" #include "third_party/blink/renderer/platform/weborigin/kurl.h"
#include "third_party/blink/renderer/platform/weborigin/security_origin.h" #include "third_party/blink/renderer/platform/weborigin/security_origin.h"
#include "third_party/blink/renderer/platform/wtf/forward.h" #include "third_party/blink/renderer/platform/wtf/forward.h"
...@@ -314,6 +315,8 @@ class XMLHttpRequest final : public XMLHttpRequestEventTarget, ...@@ -314,6 +315,8 @@ class XMLHttpRequest final : public XMLHttpRequestEventTarget,
TimeDelta timeout_; TimeDelta timeout_;
TraceWrapperMember<Blob> response_blob_; TraceWrapperMember<Blob> response_blob_;
TaskHandle pending_abort_event_;
Member<ThreadableLoader> loader_; Member<ThreadableLoader> loader_;
State state_ = kUnsent; State state_ = kUnsent;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment