1255597 - [e10s] Multiple failures in XMLHttpRequest/responsexml-media-type.htm web-platform-test with e10s enabled

Status: RESOLVED FIXED

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

Description

[Ryan VanderMeulen [:RyanVM]]

Blake, I'm ni? you to help find an owner for this, not because I necessarily expect you to personally investigate. These are annotated as failing on e10s across all platforms, but I don't see any bugs on file for investigating why.

https://treeherder.mozilla.org/logviewer.html#?job_id=17851643&repo=try

02:53:49     INFO - TEST-START | /XMLHttpRequest/responsexml-media-type.htm
02:53:49     INFO - TEST-UNEXPECTED-FAIL | /XMLHttpRequest/responsexml-media-type.htm | XMLHttpRequest: responseXML MIME type tests ('', should  parse) - client.responseXML is null
02:53:49     INFO - request/<@http://web-platform.test:8000/XMLHttpRequest/responsexml-media-type.htm:21:13
02:53:49     INFO - Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1382:20
02:53:49     INFO - test@http://web-platform.test:8000/resources/testharness.js:496:9
02:53:49     INFO - request@http://web-platform.test:8000/XMLHttpRequest/responsexml-media-type.htm:14:9
02:53:49     INFO - @http://web-platform.test:8000/XMLHttpRequest/responsexml-media-type.htm:24:7
02:53:49     INFO - TEST-PASS | /XMLHttpRequest/responsexml-media-type.htm | XMLHttpRequest: responseXML MIME type tests ('text/html', should not parse) 
02:53:49     INFO - TEST-UNEXPECTED-FAIL | /XMLHttpRequest/responsexml-media-type.htm | XMLHttpRequest: responseXML MIME type tests ('bogus', should  parse) - client.responseXML is null
02:53:49     INFO - request/<@http://web-platform.test:8000/XMLHttpRequest/responsexml-media-type.htm:21:13
02:53:49     INFO - Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1382:20
02:53:49     INFO - test@http://web-platform.test:8000/resources/testharness.js:496:9
02:53:49     INFO - request@http://web-platform.test:8000/XMLHttpRequest/responsexml-media-type.htm:14:9
02:53:49     INFO - @http://web-platform.test:8000/XMLHttpRequest/responsexml-media-type.htm:26:7
02:53:49     INFO - TEST-UNEXPECTED-FAIL | /XMLHttpRequest/responsexml-media-type.htm | XMLHttpRequest: responseXML MIME type tests ('bogus+xml', should  parse) - client.responseXML is null
02:53:49     INFO - request/<@http://web-platform.test:8000/XMLHttpRequest/responsexml-media-type.htm:21:13
02:53:49     INFO - Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1382:20
02:53:49     INFO - test@http://web-platform.test:8000/resources/testharness.js:496:9
02:53:49     INFO - request@http://web-platform.test:8000/XMLHttpRequest/responsexml-media-type.htm:14:9
02:53:49     INFO - @http://web-platform.test:8000/XMLHttpRequest/responsexml-media-type.htm:27:7
02:53:49     INFO - TEST-PASS | /XMLHttpRequest/responsexml-media-type.htm | XMLHttpRequest: responseXML MIME type tests ('text/plain;+xml', should not parse) 
02:53:49     INFO - TEST-FAIL | /XMLHttpRequest/responsexml-media-type.htm | XMLHttpRequest: responseXML MIME type tests ('text/plainxml', should not parse) - assert_equals: expected null but got Document node with 1 child
02:53:49     INFO - request/<@http://web-platform.test:8000/XMLHttpRequest/responsexml-media-type.htm:19:13
02:53:49     INFO - Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1382:20
02:53:49     INFO - test@http://web-platform.test:8000/resources/testharness.js:496:9
02:53:49     INFO - request@http://web-platform.test:8000/XMLHttpRequest/responsexml-media-type.htm:14:9
02:53:49     INFO - @http://web-platform.test:8000/XMLHttpRequest/responsexml-media-type.htm:29:7
02:53:49     INFO - TEST-PASS | /XMLHttpRequest/responsexml-media-type.htm | XMLHttpRequest: responseXML MIME type tests ('video/x-awesome+xml', should  parse) 
02:53:49     INFO - TEST-PASS | /XMLHttpRequest/responsexml-media-type.htm | XMLHttpRequest: responseXML MIME type tests ('video/x-awesome', should not parse) 
02:53:49     INFO - TEST-PASS | /XMLHttpRequest/responsexml-media-type.htm | XMLHttpRequest: responseXML MIME type tests ('text/xml', should  parse) 
02:53:49     INFO - TEST-UNEXPECTED-FAIL | /XMLHttpRequest/responsexml-media-type.htm | XMLHttpRequest: responseXML MIME type tests ('application', should  parse) - client.responseXML is null
02:53:49     INFO - request/<@http://web-platform.test:8000/XMLHttpRequest/responsexml-media-type.htm:21:13
02:53:49     INFO - Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1382:20
02:53:49     INFO - test@http://web-platform.test:8000/resources/testharness.js:496:9
02:53:49     INFO - request@http://web-platform.test:8000/XMLHttpRequest/responsexml-media-type.htm:14:9
02:53:49     INFO - @http://web-platform.test:8000/XMLHttpRequest/responsexml-media-type.htm:33:7
02:53:49     INFO - TEST-PASS | /XMLHttpRequest/responsexml-media-type.htm | XMLHttpRequest: responseXML MIME type tests ('text/xsl', should not parse) 
02:53:49     INFO - TEST-PASS | /XMLHttpRequest/responsexml-media-type.htm | XMLHttpRequest: responseXML MIME type tests ('text/plain', should not parse) 
02:53:49     INFO - TEST-PASS | /XMLHttpRequest/responsexml-media-type.htm | XMLHttpRequest: responseXML MIME type tests ('application/rdf', should not parse) 
02:53:49     INFO - TEST-PASS | /XMLHttpRequest/responsexml-media-type.htm | XMLHttpRequest: responseXML MIME type tests ('application/xhtml+xml', should  parse) 
02:53:49     INFO - TEST-PASS | /XMLHttpRequest/responsexml-media-type.htm | XMLHttpRequest: responseXML MIME type tests ('image/svg+xml', should  parse) 
02:53:49     INFO - TEST-OK | /XMLHttpRequest/responsexml-media-type.htm | took 283ms

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

Hey Olli, would you mind taking a look at this? You're the apparent owner of XHR these days.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

I don't expect to have time for this before... say late next week, if even then.
(enough reviews to do and other stuff)

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

ni? mccr8 as well in case he either has time or knows someone else who might be willing to take a look at this bug before Olli.

Comment 5

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Maybe someone on Hsin-Yi's team is interested.

Comment 6

[Hsin-Yi Tsai (she/her)[:hsinyi]]

Hi Stone, would you mind taking a look at?

Kyle, would you be the mentor here?

Thanks!

Comment 7

[Andrew McCreight [:mccr8]]

It looks like somebody else is working on this now. Thanks!

Comment 8

[Ming-Chou Shih [:stone]]

Attachment: Remove redundent trailing spaces

Comment 9

[Ming-Chou Shih [:stone]]

Attachment: Follow spec to modify the content-type check conditions which determine parsing XHR body or not

Comment 10

[Ming-Chou Shih [:stone]]

Attachment: Synchronize content-type hint of HttpChannelChild to HttpChannelParent

Comment 11

[Ming-Chou Shih [:stone]]

Attachment: Remove 'expected fail' settings and expect testcases in responsexml-media-type.htm should be passed

Comment 12

[Ming-Chou Shih [:stone]]

Those test cases failed on e10s but not on non-e10s are caused by the content-type hint of HttpChannelChild didn't synchronize with HttpChannelParent on e10s.

The test case failed on both e10s and non-e10s is due to the content-type checking mismatch XMLHttpRequest spec, which determines parsing response body or not.

Comment 13

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 8731526 [details] [diff] [review]
Synchronize content-type hint of HttpChannelChild to HttpChannelParent

Review of attachment 8731526 [details] [diff] [review]:
-----------------------------------------------------------------

Looks reasonable to me, but I don't own this code, so a Necko peer should sign off.

Comment 14

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 8731525 [details] [diff] [review]
Follow spec to modify the content-type check conditions which determine parsing XHR body or not

Review of attachment 8731525 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/nsXMLHttpRequest.cpp
@@ +1990,5 @@
>        }
> +    } else if (!(type.EqualsLiteral("text/xml") ||
> +                 type.EqualsLiteral("application/xml") ||
> +                 type.RFind("+xml", true, -1, 4) != kNotFound)) {
> +      // Follow https://www.w3.org/TR/XMLHttpRequest/

The canonical version of this spec now lives at https://xhr.spec.whatwg.org/

Comment 15

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 8731527 [details] [diff] [review]
Remove 'expected fail' settings and expect testcases in responsexml-media-type.htm should be passed

Review of attachment 8731527 [details] [diff] [review]:
-----------------------------------------------------------------

::: testing/web-platform/meta/XMLHttpRequest/responsexml-media-type.htm.ini
@@ +1,2 @@
>  [responsexml-media-type.htm]
>    type: testharness

You can just delete the file entirely now that we have no failures to mark.

Comment 16

[Jason Duell]

Comment on attachment 8731526 [details] [diff] [review]
Synchronize content-type hint of HttpChannelChild to HttpChannelParent

Review of attachment 8731526 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good.  Thanks for the patch!

Comment 17

[Ming-Chou Shih [:stone]]

Attachment: Part1: Remove redundent trailing spaces. r=khuey

change 'r?' to 'r='

Comment 18

[Ming-Chou Shih [:stone]]

Attachment: Part2: Follow spec to modify the content-type check conditions which determine parsing XHR body or not. r=khuey

change 'r?' to 'r='

Comment 19

[Ming-Chou Shih [:stone]]

Attachment: Part3: Synchronize content-type hint of HttpChannelChild to HttpChannelParent. r=jduell

change reviewer

Comment 20

[Ming-Chou Shih [:stone]]

Attachment: Part4: Remove 'expected fail' settings and expect testcases in responsexml-media-type.htm should be passed. r=khuey

follow Kyle's comment to delete the failures setting file.

Comment 21

[Ming-Chou Shih [:stone]]

Attachment: Part2: Follow spec to modify the content-type check conditions which determine parsing XHR body or not. r=khuey

Update comments.

Comment 22

[Ming-Chou Shih [:stone]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5bdcd3d3698c

Comment 23

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/752de758530e
https://hg.mozilla.org/integration/mozilla-inbound/rev/fd2df0fdbc40
https://hg.mozilla.org/integration/mozilla-inbound/rev/093561da226a
https://hg.mozilla.org/integration/mozilla-inbound/rev/77191462f644

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/752de758530e
https://hg.mozilla.org/mozilla-central/rev/fd2df0fdbc40
https://hg.mozilla.org/mozilla-central/rev/093561da226a
https://hg.mozilla.org/mozilla-central/rev/77191462f644

Comment 25

[Ryan VanderMeulen [:RyanVM]]

Please request Aurora approval on this as well.

Comment 26

[Ming-Chou Shih [:stone]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #25)
> Please request Aurora approval on this as well.

Hi Ryan,
Before requesting the approval for Aurora, may I have more background about why this patch is required for Aurora branch. This helps me to fill the reason in the approval form. Thanks.

Comment 27

[Ryan VanderMeulen [:RyanVM]]

We're targeting 47 for shipping e10s, so we want to uplift correctness fixes there as we go.

Comment 28

[Ming-Chou Shih [:stone]]

Attachment: Modify the content-type check which determine parsing XHR body. Sync content-type hint to HttpChannelParent. r=khuey,jduell

Merged 4 patches to 1 for Aurora.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5294181738cc&selectedJob=18675812

Comment 29

[Ming-Chou Shih [:stone]]

Comment on attachment 8735374 [details] [diff] [review]
Modify the content-type check which determine parsing XHR body. Sync content-type hint to HttpChannelParent. r=khuey,jduell

Approval Request Comment
[Feature/regressing bug #]: NA
[User impact if declined]: XHR responseXML isn't parsed when MIME type is empty or bogus on e10s.
[Describe test coverage new/current, TreeHerder]: XMLHttpRequest/responsexml-media-type.htm
[Risks and why]: NA
[String/UUID change made/needed]: NA

Comment 30

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8735374 [details] [diff] [review]
Modify the content-type check which determine parsing XHR body. Sync content-type hint to HttpChannelParent. r=khuey,jduell

e10s specific, Aurora47+

Comment 31

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/e50bd4f62a1f