This should be very easy now. This data is stored on response's body per update to Fetch. Just need to link the bits together.

https://github.com/whatwg/xhr/commit/b9f6bcf9800963eb450b20cc96492a670fb96a99

(In reply to Anne from comment #2)
> https://github.com/whatwg/xhr/commit/b9f6bcf9800963eb450b20cc96492a670fb96a99

This change includes addition of "with 0 and 0" to fire a progress event algorithm steps in "request error steps" (current name). With this, people will see:
- progress ProgressEvent with x and y
- progress ProgressEvent with x' and y
...
- readystatechange event with DONE
- progress ProgressEvent with 0 and 0
- error/abort/timeout ProgressEvent with 0 and 0
- ...

Looking at only the ProgressEvents, it looks good that we're no longer dispatching a misleading progress ProgressEvent with normal non-zero loaded value after setting response to a network error.

But, for example in Blink, we've been using the last loaded/total value for the events since it was not well specified in the spec. Using the last "loaded" value for the error/abort/timeout event allows the user to know around which point the error occurred. Maybe useful for debugging.

So, my gut feeling is we should remove "fire a progress ProgressEvent" step from the algorithm but make "fire a _event_ progress ProgressEvent" and maybe "fire a loadend ProgressEvent" to use the last loaded/total value.

WDYT?

Olli, could you comment on behalf of Gecko regarding comment 3?

(In reply to Takeshi Yoshino from comment #3)
> So, my gut feeling is we should remove "fire a progress ProgressEvent" step
> from the algorithm but make "fire a _event_ progress ProgressEvent" and
> maybe "fire a loadend ProgressEvent" to use the last loaded/total value.
> 
> WDYT?

I don't understand this part.

(In reply to Olli Pettay from comment #5)
> (In reply to Takeshi Yoshino from comment #3)
> > So, my gut feeling is we should remove "fire a progress ProgressEvent" step
> > from the algorithm but make "fire a _event_ progress ProgressEvent" and
> > maybe "fire a loadend ProgressEvent" to use the last loaded/total value.
> > 
> > WDYT?
> 
> I don't understand this part.

Me too after two weeks... I also found a typo. Rewrote:

I suggest that:
- we remove "fire a progress ProgressEvent" step
- have the "fire a _event_ ProgressEvent" step to use the last loaded/total value than 0/0
- have the "fire a loadend ProgressEvent" step to use the last loaded/total value than 0/0

Then:
a) we don't dispatch the misleading progress ProgressEvent (a progress ProgressEvent with positive value looks indicating the loading is still successfully going without any error. we should avoid dispatching such an event)
b) we can know at which point of loading the error occurred. Maybe useful for debugging. As we'll see a ProgressEvent named error/abort/timeout, it's not misleading like (a)

We fire a last event named progress to ensure you can finish animations and such that hang off it. I guess we could opt not to fire one in case of an error scenario though. Seems a bit ugly.

Are you suggesting we store the /transmitted/ and /length/ variables on the XMLHttpRequest object so they can be reused by the final error events?

(In reply to Anne from comment #7)
> We fire a last event named progress to ensure you can finish animations and
> such that hang off it. I guess we could opt not to fire one in case of an
> error scenario though. Seems a bit ugly.

Yes. I'm not talking about successful case. Only an error scenario.

But when I was investigating Bug 26736, I saw this Glenn's post http://lists.w3.org/Archives/Public/public-webapps/2011JanMar/0993.html . I understand the following argument.

> For download progress you can work around this by watching onload and
> treating that as a progress(total, total) event, but that shouldn't be
> required.

Similar logic could be applied to error scenarios. I.e. we could say that we should be able to know some error occurred by looking at only "progress" ProgressEvents. And then, we'll e.g. change the color of the progress bar to red when it sees "loaded" becoming 0 from non-zero or "total" becoming 0 from non-zero.

Do you think this is useful?

Hmm, it also seems a little ugly if we make transmitted and length to the last value before error (for debugging).

> 
> Are you suggesting we store the /transmitted/ and /length/ variables on the
> XMLHttpRequest object so they can be reused by the final error events?

Right. That's what we're doing now in Blink.

Well, that is the reason why I set them all to 0 at that point. But leaving them all the same and freezing seems okay too (if the UI code only looks at progress). I don't feel strongly. 

It's a little uglier to keep state around I think, but I don't mind it.

(In reply to Anne from comment #9)
> Well, that is the reason why I set them all to 0 at that point. But leaving
> them all the same and freezing seems okay too (if the UI code only looks at
> progress). I don't feel strongly. 
> 
> It's a little uglier to keep state around I think, but I don't mind it.

I see. CC-ed Glenn to hear thoughts.

FYI, here's what each browser is doing so far:

I used a script which sends a response using chunked Transfer-Encoding. It flushes 1024 byte every 1 sec. After several chunks, it sends an invalid chunk header (starts with asterisks instead of size of chunk in hex) to induce a network error.

All of IE, Chrome and Gecko keep the "loaded" value.


IE 11.0.9600.17239

readystatechange1 * 2
loadstart(false, 0, 18446744073709552000) * 1
readystatechange2 * 1
readystatechange3 * 1
progress(false, 1024, 18446744073709552000) * 1
readystatechange3 * 1
progress(false, 2048, 18446744073709552000) * 1
readystatechange3 * 1
progress(false, 3072, 18446744073709552000) * 1
readystatechange4 * 1
error(true, 3072, 3072) * 1
loadend(true, 3072, 3072) * 1


39.0.2149.0 canary (64-bit)

Please ignore "(x, y, z)" for readystatechange events. It's due to a bug in Chrome.

readystatechange1(false, 0, 0) * 1
loadstart(false, 0, 0) * 1
readystatechange2(false, 0, 0) * 1
readystatechange3(false, 0, 0) * 1
progress(false, 1024, 0) * 1
readystatechange3(false, 0, 0) * 1
progress(false, 2048, 0) * 1
readystatechange3(false, 0, 0) * 1
progress(false, 3072, 0) * 1
readystatechange4(false, 0, 0) * 1
progress(false, 3072, 0) * 1
error(false, 3072, 0) * 1
loadend(false, 3072, 0) * 1


Firefox Aurora 34.0a2 (2014-09-07)

readystatechange1 * 1
loadstart(false, 0, 0) * 1
readystatechange2 * 1
readystatechange3 * 1
progress(false, 1024, 0) * 1
readystatechange3 * 1
progress(false, 2048, 0) * 1
readystatechange3 * 1
progress(false, 3072, 0) * 1
readystatechange4 * 1
error(false, 3072, 0) * 1
loadend(false, 3072, 0) * 1

For download progress, showing bytes transferred makes sense.  I seem to recall some digital download services showing download speeds in uncompressed bytes, which seemed like a pretty sad lie (look, we're faster than everyone else!), and we'd be making everyone do that if we used bytes after decompression.

I assume you mean "number of compressed bytes, not including protocol overhead" (HTTP chunked headers, SSL packet data, etc.), since otherwise the "total" value would never be known.  I wonder if that's tricky for HTTPS TLS-level compression, where the compressed data size might not be exposed to the upper layer, and if it means that TLS compression would always disable "total".

(In reply to Takeshi Yoshino from comment #8)
> > For download progress you can work around this by watching onload and
> > treating that as a progress(total, total) event, but that shouldn't be
> > required.
> 
> Similar logic could be applied to error scenarios. I.e. we could say that we
> should be able to know some error occurred by looking at only "progress"
> ProgressEvents. And then, we'll e.g. change the color of the progress bar to
> red when it sees "loaded" becoming 0 from non-zero or "total" becoming 0
> from non-zero.

My logic was for using progress events for progress displays, not for figuring out if the transfer finished.  You should use onload, onloadend and onerror for their tasks; don't try to use onprogress to detect if an error occurred.  (What I was describing was making sure progress bars actually fill to 100% when the transfer is complete, and don't hover at 99% because the browser never sent any progress event with loaded == total due to the throttling logic, since developers shouldn't have to take extra steps to avoid that.)

If I start XHR and see "0 bytes, 100 bytes, 200 bytes" and then an error happens, I shouldn't see a progress event saying "0 bytes".  I wouldn't want my progress meters or percent displays to snap back to 0 on error.  (I might want to hide them, but I might also want to leave them, so the user can see how far the transfer went before it failed.)  It's also just weird...

(By the way, you'd have a bug: the progress bar wouldn't become red if an error occurred before loading any bytes, since loaded was never nonzero.)

(In reply to Glenn Maynard from comment #11)

Thanks. So, what do you think about my proposal at comment 6?

1) we remove "fire a progress ProgressEvent" step
2) have the "fire a _event_ ProgressEvent" step to use the last loaded/total value than 0/0
3) have the "fire a loadend ProgressEvent" step to use the last loaded/total value than 0/0

For (1), we could also do "fire a progress ProgressEvent with lengthComputable/loaded/total as of the time error happened" than just removing the step. I.e. we make sure we tell the final position to the user even if there's throttling (like what you suggested for success case). I'm fine with either as this is error case.

For the "error" and "loadend" event, I think providing lengthComputable/loaded/total as of the time error happened would be useful for debugging. But I don't have strong opinion about this.

When lengthComputable is false because chunked Transfer-Encoding is used, currently IE, Chrome and Firefox are doing so for "loaded". IE sets total to the same value as loaded and sets lengthComputable to true. Chrome and Firefox leaves them 0 and false (as on comment 10).

> (By the way, you'd have a bug: the progress bar wouldn't become red if an
> error occurred before loading any bytes, since loaded was never nonzero.)

Right. It's not fully reliable. If lengthComputable is true and total is non-zero, we can look at total going down. Otherwise, there's nothing from which we can detect error only by "progress" ProgressEvent.

Here's the result of event order checking when Content-Length is specified and "Transfer-Encoding: chunked" is not used. I closed the socket after flushing the first 1024 byte.

IE seems to have treated it as successful. loaded became 65536.

Firefox reset lengthComputable to false and total to 0 and dispatched "error" event.


IE 11.0.9600.17239

readystatechange1 * 2
loadstart(false, 0, 18446744073709552000) * 1
readystatechange2 * 1
readystatechange3 * 1
progress(true, 1024, 65536) * 1
progress(true, 65536, 65536) * 1
readystatechange4 * 1
load(true, 65536, 65536) * 1
loadend(true, 65536, 65536) * 1

Chrome 39.0.2149.0 canary (64-bit)

readystatechange1(false, 0, 0) * 1
loadstart(false, 0, 0) * 1
readystatechange2(false, 0, 0) * 1
readystatechange3(false, 0, 0) * 1
progress(true, 1024, 65536) * 1
readystatechange4(false, 0, 0) * 1
progress(true, 1024, 65536) * 1
error(true, 1024, 65536) * 1
loadend(true, 1024, 65536) * 1

Firefox Aurora 34.0a2 (2014-09-07)

readystatechange1 * 1
loadstart(false, 0, 0) * 1
readystatechange2 * 1
readystatechange3 * 1
progress(true, 1024, 65536) * 1
readystatechange4 * 1
error(false, 1024, 0) * 1
loadend(false, 1024, 0) * 1

In the interest of settling long-standing issues related to progress events, I would recommend just changing the web platform tests to match the spec wording as precisely as possible. This will cause some browsers to fail tests they're currently passing for seemingly trivial reasons, but the spec now seems reasonable (and reasonably clear) about which events to fire and which loaded, total, and lengthComputable values to send in those events. For instance:

- no final progress events will be fired in the error case, just the error/loadend events with 0 values (as these events add no real value and are not currently fired by user agents anyhow).

- progress events will set loaded and total to the raw bytes transferred, not the values after decoding (this is info the user cannot compute on their own, whereas they will always know the decoded size upon completion).

- upload.loadstart will set the total and loaded=0 and lengthComputable=true (since the total ought to be known by that point, although some UAs currently fire 0/0/false).

That sounds good to me. If we still need changes here after all, please file an issue at https://github.com/whatwg/xhr/issues/new.