Skip to content

Commit

Permalink
Merge r220731 - XHR should only fire an abort event if the cancellati…
Browse files Browse the repository at this point in the history
…on was requested by the client

https://bugs.webkit.org/show_bug.cgi?id=175546

Reviewed by Youenn Fablet.

LayoutTests/imported/w3c:

Rebaseline several WPT tests now that a different error is sometimes used and that
more checks are passing.

* web-platform-tests/XMLHttpRequest/open-url-multi-window-4-expected.txt:
* web-platform-tests/XMLHttpRequest/send-network-error-sync-events.sub-expected.txt:
* web-platform-tests/cors/allow-headers-expected.txt:
* web-platform-tests/cors/origin-expected.txt:
* web-platform-tests/cors/request-headers-expected.txt:
* web-platform-tests/cors/response-headers-expected.txt:
* web-platform-tests/resource-timing/resource_TAO_match_origin-expected.txt:
* web-platform-tests/resource-timing/resource_TAO_match_wildcard-expected.txt:
* web-platform-tests/resource-timing/resource_TAO_multi-expected.txt:
* web-platform-tests/resource-timing/resource_TAO_null-expected.txt:
* web-platform-tests/resource-timing/resource_TAO_origin-expected.txt:
* web-platform-tests/resource-timing/resource_TAO_origin_uppercase-expected.txt:
* web-platform-tests/resource-timing/resource_TAO_space-expected.txt:
* web-platform-tests/resource-timing/resource_TAO_wildcard-expected.txt:
* web-platform-tests/resource-timing/resource_TAO_zero-expected.txt:

Source/WebCore:

XHR should only fire an abort event if the cancellation was requested by the client, otherwise it should fire an error event.
Blink and Gecko already match the specification.

Specification:
- https://xhr.spec.whatwg.org/#handle-errors
- https://xhr.spec.whatwg.org/#the-abort()-method

Test: http/tests/navigation/page-cache-xhr-in-pagehide.html

* xml/XMLHttpRequest.cpp:
(WebCore::XMLHttpRequest::open):
(WebCore::XMLHttpRequest::abort):
(WebCore::XMLHttpRequest::abortError):
(WebCore::XMLHttpRequest::didFail):
* xml/XMLHttpRequest.h:

LayoutTests:

* http/tests/navigation/page-cache-xhr-expected.txt:
* http/tests/navigation/page-cache-xhr-in-pagehide-expected.txt: Copied from LayoutTests/http/tests/navigation/page-cache-xhr-expected.txt.
* http/tests/navigation/page-cache-xhr-in-pagehide.html: Copied from LayoutTests/http/tests/navigation/page-cache-xhr.html.
* http/tests/navigation/page-cache-xhr.html:
When an XHR is pending and navigating away, we would send an abort event before the navigation and an error event after
restoring from PageCache. This bug was not seen before because the test only checked for error events, not abort ones.
The expected behavior is now that we fire an error event before navigating away, similar to the non-PageCache case.
The only case where the error event should be fired after restoring from PageCache is when an XHR is done in the pagehide
event handler, because it is too late to send the error event to the page before navigating in this case. I added test
coverage for this case.

* http/tests/xmlhttprequest/navigation-should-abort-expected.txt:
* http/tests/xmlhttprequest/navigation-should-abort.html:
This test was expecting an abort event on navigation which is against spec. This test was failing in Blink too.
Update the test to expect an error event instead.
  • Loading branch information
cdumez authored and carlosgcampos committed Aug 17, 2017
1 parent 59da5f0 commit 2e5393d
Show file tree
Hide file tree
Showing 28 changed files with 310 additions and 147 deletions.
23 changes: 23 additions & 0 deletions LayoutTests/ChangeLog
@@ -1,3 +1,26 @@
2017-08-14 Chris Dumez <cdumez@apple.com>

XHR should only fire an abort event if the cancellation was requested by the client
https://bugs.webkit.org/show_bug.cgi?id=175546

Reviewed by Youenn Fablet.

* http/tests/navigation/page-cache-xhr-expected.txt:
* http/tests/navigation/page-cache-xhr-in-pagehide-expected.txt: Copied from LayoutTests/http/tests/navigation/page-cache-xhr-expected.txt.
* http/tests/navigation/page-cache-xhr-in-pagehide.html: Copied from LayoutTests/http/tests/navigation/page-cache-xhr.html.
* http/tests/navigation/page-cache-xhr.html:
When an XHR is pending and navigating away, we would send an abort event before the navigation and an error event after
restoring from PageCache. This bug was not seen before because the test only checked for error events, not abort ones.
The expected behavior is now that we fire an error event before navigating away, similar to the non-PageCache case.
The only case where the error event should be fired after restoring from PageCache is when an XHR is done in the pagehide
event handler, because it is too late to send the error event to the page before navigating in this case. I added test
coverage for this case.

* http/tests/xmlhttprequest/navigation-should-abort-expected.txt:
* http/tests/xmlhttprequest/navigation-should-abort.html:
This test was expecting an abort event on navigation which is against spec. This test was failing in Blink too.
Update the test to expect an error event instead.

2017-08-14 Simon Fraser <simon.fraser@apple.com>

Remove Proximity Events and related code
Expand Down
1 change: 0 additions & 1 deletion LayoutTests/fast/frames/frame-unload-crash-expected.txt
@@ -1,5 +1,4 @@
frame "<!--framePath //<!--frame0-->/<!--frame0-->-->" - has 1 onunload handler(s)
CONSOLE MESSAGE: line 13: XMLHttpRequest cannot load frame-unload-crash-2.html due to access control checks.
This is a test for bug 25136: CRASH in DocumentLoader::removeSubresourceLoader due to null m_frame. If successful, PASS should be printed below.

PASS
Expand Down
4 changes: 2 additions & 2 deletions LayoutTests/http/tests/navigation/page-cache-xhr-expected.txt
Expand Up @@ -4,11 +4,11 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE


pageshow - not from cache
PASS Executed the XHR error handler before entering PageCache
PASS xhr.status is 0
pagehide - entering cache
pageshow - from cache
PASS Page did enter and was restored from the page cache
PASS Executed the XHR error handler after restoring from page cache
PASS xhr.status is 0
PASS successfullyParsed is true

TEST COMPLETE
Expand Down
@@ -0,0 +1,16 @@
CONSOLE MESSAGE: line 35: XMLHttpRequest cannot load http://127.0.0.1:8000/resources/load-and-stall.cgi?name=load-and-stall.cgi&stallFor=3&stallAt=0&mimeType=text/plain due to access control checks.
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 Executed the XHR error handler after restoring from PageCache
PASS xhr.status is 0
PASS successfullyParsed is true

TEST COMPLETE

70 changes: 70 additions & 0 deletions LayoutTests/http/tests/navigation/page-cache-xhr-in-pagehide.html
@@ -0,0 +1,70 @@
<!DOCTYPE html>
<html>
<body>
<script src="/resources/js-test-pre.js"></script>
<script>
description('Tests that a page with a loading XMLHttpRequest goes into the page cache.');
window.jsTestIsAsync = true;

var restoredFromPageCache = false;

if (window.testRunner)
testRunner.overridePreference("WebKitUsesPageCachePreferenceKey", 1);

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();
}
xhr = new XMLHttpRequest();
xhr.onload = xhrLoaded;
xhr.onabort = xhrAbort;
xhr.onerror = xhrError;
// Slow loading XHR (3-second stall).
xhr.open("GET", "/resources/load-and-stall.cgi?name=load-and-stall.cgi&stallFor=3&stallAt=0&mimeType=text/plain", true);
xhr.send();
}, false);

function xhrLoaded()
{
testFailed("The XMLHttpRequest should not haved loaded");
finishJSTest();
}

function xhrAbort() {
testFailed("Executed the XHR abort handler unexpectedly");
finishJSTest();
}

function xhrError() {
if (restoredFromPageCache)
testPassed("Executed the XHR error handler after restoring from PageCache");
else
testFailed("Executed the XHR error handler before entering PageCache");
shouldBe("xhr.status", "0");
finishJSTest();
}

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 = "resources/page-cache-helper.html";
}, 0);
}, false);

</script>
<script src="/resources/js-test-post.js"></script>
</body>
</html>
17 changes: 12 additions & 5 deletions LayoutTests/http/tests/navigation/page-cache-xhr.html
Expand Up @@ -17,6 +17,7 @@
if (event.persisted) {
testPassed("Page did enter and was restored from the page cache");
restoredFromPageCache = true;
setTimeout(finishJSTest, 0);
}
}, false);

Expand All @@ -34,19 +35,25 @@
finishJSTest();
}

function xhrAbort() {
testFailed("Executed the XHR abort handler unexpectedly");
finishJSTest();
}

function xhrError() {
if (restoredFromPageCache)
testPassed("Executed the XHR error handler after restoring from page cache");
else
testFailed("Executed the XHR error handler before restoring from page cache");
if (restoredFromPageCache) {
testFailed("Executed the XHR error handler after restoring from PageCache");
return;
}

testPassed("Executed the XHR error handler before entering PageCache");
shouldBe("xhr.status", "0");
finishJSTest();
}

window.addEventListener('load', function() {
xhr = new XMLHttpRequest();
xhr.onload = xhrLoaded;
xhr.onabort = xhrAbort;
xhr.onerror = xhrError;
// Slow loading XHR (3-second stall).
xhr.open("GET", "/resources/load-and-stall.cgi?name=load-and-stall.cgi&stallFor=3&stallAt=0&mimeType=text/plain", true);
Expand Down
@@ -1,2 +1,2 @@
CONSOLE MESSAGE: line 14: PASS: Expected 'abort', got 'abort'.
CONSOLE MESSAGE: line 11: PASS: Expected 'error', got 'error'.
If this text shows up, you've successfully navigated to 'navigation-target.html'.
Expand Up @@ -8,10 +8,10 @@
var req = new XMLHttpRequest();
req.open("GET", "resources/endlessxml.php");
req.onerror = function () {
console.log("FAIL: Expected 'abort', got 'error'.");
console.log("PASS: Expected 'error', got 'error'.");
};
req.onabort = function () {
console.log("PASS: Expected 'abort', got 'abort'.");
console.log("FAIL: Expected 'error', got 'abort'.");
};
req.send(null);

Expand Down
26 changes: 26 additions & 0 deletions LayoutTests/imported/w3c/ChangeLog
@@ -1,3 +1,29 @@
2017-08-14 Chris Dumez <cdumez@apple.com>

XHR should only fire an abort event if the cancellation was requested by the client
https://bugs.webkit.org/show_bug.cgi?id=175546

Reviewed by Youenn Fablet.

Rebaseline several WPT tests now that a different error is sometimes used and that
more checks are passing.

* web-platform-tests/XMLHttpRequest/open-url-multi-window-4-expected.txt:
* web-platform-tests/XMLHttpRequest/send-network-error-sync-events.sub-expected.txt:
* web-platform-tests/cors/allow-headers-expected.txt:
* web-platform-tests/cors/origin-expected.txt:
* web-platform-tests/cors/request-headers-expected.txt:
* web-platform-tests/cors/response-headers-expected.txt:
* web-platform-tests/resource-timing/resource_TAO_match_origin-expected.txt:
* web-platform-tests/resource-timing/resource_TAO_match_wildcard-expected.txt:
* web-platform-tests/resource-timing/resource_TAO_multi-expected.txt:
* web-platform-tests/resource-timing/resource_TAO_null-expected.txt:
* web-platform-tests/resource-timing/resource_TAO_origin-expected.txt:
* web-platform-tests/resource-timing/resource_TAO_origin_uppercase-expected.txt:
* web-platform-tests/resource-timing/resource_TAO_space-expected.txt:
* web-platform-tests/resource-timing/resource_TAO_wildcard-expected.txt:
* web-platform-tests/resource-timing/resource_TAO_zero-expected.txt:

2017-08-09 Chris Dumez <cdumez@apple.com>

Reinstate active flag for iterators
Expand Down
@@ -1,3 +1,3 @@

FAIL XMLHttpRequest: open() resolving URLs (multi-Window; 4; evil) assert_true: should get an error event expected true got false
PASS XMLHttpRequest: open() resolving URLs (multi-Window; 4; evil)

@@ -1,7 +1,4 @@
Blocked access to external URL http://nonexistent-origin.localhost}:8800/

FAIL XMLHttpRequest: The send() method: Throw a "throw an "NetworkError" exception when Network error happens (synchronous flag is set) assert_throws: function "function ()
{
xhr.send("Test Message");
}" threw object "AbortError: The operation was aborted." that is not a DOMException NetworkError: property "code" is equal to 20, expected 19
PASS XMLHttpRequest: The send() method: Throw a "throw an "NetworkError" exception when Network error happens (synchronous flag is set)

0 comments on commit 2e5393d

Please sign in to comment.