1346767 - XMLHttpRequest: upload listener flag not implemented correctly

Status: RESOLVED FIXED

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

Description

[Anne (:annevk)]

In particular, it seems we bypass it for same-origin requests, which is unlikely to work well, as same-origin can be redirected to cross-origin.

https://github.com/w3c/web-platform-tests/pull/5122

Comment 1

[Andrew Overholt [:overholt]]

Maybe baku knows what's up here?

Comment 2

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

The test case has not landed yet and is still up for review.
https://github.com/w3c/web-platform-tests/pull/5122/commits/1abc11ce5868e29e4416c7e50ae0332d5b657bf9

Comment 3

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

It seems to me an intermittent bug.


Upload events registered too late (resources/redirect.py?code=307&location=http://www1.web-platform.test:8000/XMLHttpRequest/resources/corsenabled.py)	assert_unreached: Reached unreachable code

EventHandlerNonNull*@http://web-platform.test:8000/XMLHttpRequest/event-upload-progress-crossorigin.htm:30:32
Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1405:20
async_test@http://web-platform.test:8000/resources/testharness.js:514:13
@http://web-platform.test:8000/XMLHttpRequest/event-upload-progress-crossorigin.htm:24:3
@http://web-platform.test:8000/XMLHttpRequest/event-upload-progress-crossorigin.htm:23:2

Comment 4

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Original bug: https://github.com/whatwg/xhr/issues/95

Comment 5

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

So that unexpected progress event is from:
http://searchfox.org/mozilla-central/rev/1a0d9545b9805f50a70de703a3c04fc0d22e3839/dom/xhr/XMLHttpRequestMainThread.cpp#3699-3700

Comment 6

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

One of ProgressEventTimer doesn't get cancel, then it fires progress event unexpectedly.
I will find out why progress event timer still exists. We also need to make sure if event listeners are registered after send(), progress event should not fire at all.

Comment 7

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Sorry for late response, now I switch back to this bug. Set ETA.

Comment 8

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Attachment: Bug 1346767 - Check mFlagHadUploadListenersOnSend before sending progress event

Comment 9

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Attachment: Bug 1346767 - Part 1: Check mFlagHadUploadListenersOnSend before sending progress event

Comment 10

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Attachment: Bug 1346767 - Part 2: Make sure xhr.upload event listeners will be ignored if listeners are registered after send()

Comment 11

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

try:
 https://treeherder.mozilla.org/#/jobs?repo=try&revision=4557f3fe1619ab4972f26d5638f14094336ce5c7

Comment 12

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Not sure if this could break other tests compatibility, so run try all.
The risk is that xhr.upload event listeners are registered after xhr.send() in same origin/non-simple, progress events shall not be received, maybe this would break some web sites.

Comment 13

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

(In reply to Shawn Huang [:shawnjohnjr] from comment #11)
> try:
>  https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=4557f3fe1619ab4972f26d5638f14094336ce5c7

There are so many unknown error.
Push to try again:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ae85a618859475927bfb95e1f5d37a8070dea578

Comment 14

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Attachment: Bug 1346767 - Part 2: Make sure xhr.upload event listeners will be ignored if listeners are registered after send(), r=baku

Comment 15

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Attachment: Bug 1346767 - Part 1: Check mFlagHadUploadListenersOnSend before sending progress event, r=baku

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f0817bf2f80b
Part 1: Check mFlagHadUploadListenersOnSend before sending progress event, r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/4377bafde262
Part 2: Make sure xhr.upload event listeners will be ignored if listeners are registered after send(), r=baku

Comment 17

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

https://hg.mozilla.org/mozilla-central/rev/f0817bf2f80b
https://hg.mozilla.org/mozilla-central/rev/4377bafde262

Comment 18

[Chris Mills [:cmills]]

Ah wait, doesn't look like I need to update any docs to account for this.