Closed Bug 1255597 Opened 4 years ago Closed 4 years ago

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

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
e10s + ---
firefox47 --- fixed
firefox48 --- fixed

People

(Reporter: RyanVM, Assigned: stone, Mentored)

References

(Blocks 1 open bug)

Details

(Whiteboard: [tw-dom])

Attachments

(5 files, 5 obsolete files)

4.09 KB, patch
stone
: review+
Details | Diff | Splinter Review
6.19 KB, patch
stone
: review+
Details | Diff | Splinter Review
7.99 KB, patch
stone
: review+
Details | Diff | Splinter Review
1.47 KB, patch
stone
: review+
Details | Diff | Splinter Review
18.66 KB, patch
stone
: review+
Details | Diff | Splinter Review
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
Flags: needinfo?(mrbkap)
Hey Olli, would you mind taking a look at this? You're the apparent owner of XHR these days.
Flags: needinfo?(mrbkap) → needinfo?(bugs)
Duplicate of this bug: 1241972
I don't expect to have time for this before... say late next week, if even then.
(enough reviews to do and other stuff)
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.
Flags: needinfo?(continuation)
Maybe someone on Hsin-Yi's team is interested.
Flags: needinfo?(htsai)
Whiteboard: [tw-dom]
Hi Stone, would you mind taking a look at?

Kyle, would you be the mentor here?

Thanks!
Flags: needinfo?(htsai) → needinfo?(sshih)
Assignee: nobody → sshih
Flags: needinfo?(sshih)
It looks like somebody else is working on this now. Thanks!
Flags: needinfo?(continuation)
Flags: needinfo?(bugs)
Attached patch Remove redundent trailing spaces (obsolete) — Splinter Review
Attachment #8731524 - Flags: review?(khuey)
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 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.
Attachment #8731526 - Flags: review?(khuey) → review?(jduell.mcbugs)
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/
Attachment #8731525 - Flags: review?(khuey) → review+
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.
Attachment #8731527 - Flags: review?(khuey) → review+
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!
Attachment #8731526 - Flags: review?(jduell.mcbugs) → review+
change 'r?' to 'r='
Attachment #8731524 - Attachment is obsolete: true
Attachment #8732031 - Flags: review+
follow Kyle's comment to delete the failures setting file.
Attachment #8731527 - Attachment is obsolete: true
Attachment #8732036 - Flags: review+
Keywords: checkin-needed
Please request Aurora approval on this as well.
Flags: needinfo?(sshih)
(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.
Flags: needinfo?(sshih) → needinfo?(ryanvm)
We're targeting 47 for shipping e10s, so we want to uplift correctness fixes there as we go.
Flags: needinfo?(ryanvm)
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
Attachment #8735374 - Flags: approval-mozilla-aurora?
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+
Attachment #8735374 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.