Closed Bug 1362354 Opened 7 years ago Closed 6 years ago

Failure in testing/web-platform/tests/XMLHttpRequest/abort-after-stop.htm

Categories

(Core :: DOM: Core & HTML, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: shawnjohnjr, Assigned: twisniewski)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 4 obsolete files)

XMLHttpRequest: abort event should fire when stop() method is used

assert_equals: expected true but got false
Assignee: nobody → shuang
I've fixed the original issue, so right now abort event will be issued.
But I can still see testharness timeout. I guess that it's because window.stop() cause test.done() failure.

Hi James,
Do you think it's normal after calling window.stop(), test.done()[1] can't end the test case?
I even tried to move test.done() into client.onabort. Timeout also happened with Chrome[2].



[1] https://github.com/w3c/web-platform-tests/blob/master/XMLHttpRequest/abort-after-stop.htm#L24
[2] https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/TestExpectations?type=cs&q=abort-after-stop.htm&l=2205
Flags: needinfo?(james)
Attachment #8890294 - Flags: review?(amarchesini) → review+
Pushed by shuang@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6ed95396bdb6
Send abort event if window.stop() is called, r=baku
Backed out for failing web-platform-test /html/browsers/browsing-the-web/navigating-across-documents/010.html:

https://hg.mozilla.org/integration/mozilla-inbound/rev/0739667c02050d60fec1f965307a8a85f8c40b8b

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=6ed95396bdb623610e88b4d59ed267d513ae2886&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=118440054&repo=mozilla-inbound

[task 2017-07-27T10:28:32.186301Z] 10:28:32     INFO - TEST-START | /html/browsers/browsing-the-web/navigating-across-documents/010.html
[task 2017-07-27T10:28:32.242834Z] 10:28:32     INFO - PID 3801 | 1501151312233	Marionette	DEBUG	Register listener.js for window 2147483709
[task 2017-07-27T10:28:42.323396Z] 10:28:42     INFO - 
[task 2017-07-27T10:28:42.323678Z] 10:28:42     INFO - TEST-UNEXPECTED-FAIL | /html/browsers/browsing-the-web/navigating-across-documents/010.html | Link with onclick form submit to javascript url with delayed document.write and href navigation  - assert_equals: expected "href" but got "write"
[task 2017-07-27T10:28:42.324129Z] 10:28:42     INFO - onmessage<@http://web-platform.test:8000/html/browsers/browsing-the-web/navigating-across-documents/010.html:14:5
[task 2017-07-27T10:28:42.324581Z] 10:28:42     INFO - Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1412:20
[task 2017-07-27T10:28:42.325065Z] 10:28:42     INFO - Test.prototype.step_func/<@http://web-platform.test:8000/resources/testharness.js:1436:20
[task 2017-07-27T10:28:42.325580Z] 10:28:42     INFO - EventHandlerNonNull*@http://web-platform.test:8000/html/browsers/browsing-the-web/navigating-across-documents/010.html:12:13
[task 2017-07-27T10:28:42.326179Z] 10:28:42     INFO - TEST-UNEXPECTED-TIMEOUT | /html/browsers/browsing-the-web/navigating-across-documents/010.html | expected OK
Flags: needinfo?(shuang)
I'm sorry that I did not notice this test case fail. I will dig into it.
Flags: needinfo?(shuang)
Flags: needinfo?(james)
Without my patch, running 010.html, I always saw js error.

0:09.82 PROCESS_OUTPUT: ProcessReader (pid:29244) "JavaScript error: javascript:(function()%20{var%20x%20=%20new%20XMLHttpRequest();%20x.open('GET',%20'blank.html?pipe=trickle(d2)',%20false);%20x.send();%20document.write('<script>parent.postMessage("write",%20"*")</script>');%20return%20'<script>parent.postMessage("click",%20"*")</script>'})(), line 1: NetworkError: A network error occurred."

I wonder how is this can pass the test case.
Hmm. Since js error, then href shall be always expected.
(In reply to Shawn Huang [:shawnjohnjr] from comment #9)
> Without my patch, running 010.html, I always saw js error.
> 
> 0:09.82 PROCESS_OUTPUT: ProcessReader (pid:29244) "JavaScript error:
> javascript:(function()%20{var%20x%20=%20new%20XMLHttpRequest();%20x.
> open('GET',%20'blank.html?pipe=trickle(d2)',%20false);%20x.send();
> %20document.write('<script>parent.postMessage("write",%20"*")</script>');
> %20return%20'<script>parent.postMessage("click",%20"*")</script>'})(), line
> 1: NetworkError: A network error occurred."
> 
> I wonder how is this can pass the test case.

Oh. Even without my patch, originally since open() is sync version which is already deprecated, then in send() function, code returns network error due to mChannel is null, it throws network error. This leads document.write() won't be executed. So the test case doesn't actually work properly. Verdict can pass just because javascript error.

Hmm. I honestly think this test case is weired and now I'm now sure how to fix this test case from xhr.
With my patch, xhr sends abort event, so the script will be executed anyway, then document.write happens earlier than postMessage in href.html. I wondered why this test uses xhr sync to delay 2 seconds.

Considering xhr sync support also deprecated in Chrome. I think it's not necessary to use xhr sync to delay.
Priority: -- → P2
Blocks: xhr
Assignee: shuang → nobody
Comment on attachment 8997495 [details]
Bug 1362354 - Handle XHRs with aborted bindings as aborted rather than network errors.

https://reviewboard.mozilla.org/r/261240/#review268710
Attachment #8997495 - Flags: review?(amarchesini) → review+
Keywords: checkin-needed
Pushed by aiakab@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8ee7533d1e8e
Handle XHRs with aborted bindings as aborted rather than network errors. r=baku
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/8ee7533d1e8e
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Assignee: nobody → twisniewski
Depends on: 1508377
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.