637002 - ProgressEvent.load fires too late

Status: NEW

(Core :: Networking, defect, P5)

Description

[Simon Speich]

User-Agent:       Mozilla/5.0 (Windows NT 6.1; rv:2.0b12) Gecko/20100101 Firefox/4.0b12
Build Identifier: Mozilla/5.0 (Windows NT 6.1; rv:2.0b12) Gecko/20100101 Firefox/4.0b12

When using the XmlHttpRequest.upload object to monitor upload progress, the ProgressEvent.load only fires, when the whole request is completed, e.g. the server has processed the request and the response is sent back. I my opinion this is wrong (and Google Chrome does it correctly). Instead the event should fire as soon as the browser has sent all the bytes (e.g.finished uploading) and it should not wait for the server's response. You would use the XmlHttpRequest.load event for that.

As a consequence the property ProgressEvent.loaded equals only ProgressEvent.total after the request is completed and not right away when all bytes are sent. This make it impossible to detect the difference. Often there is a big lag of several seconds between the browser having finished uploading (all bytes sent) and the time the server has written the file and sent the response back. In FF there is no way I can inform the user about that.





Reproducible: Always

Steps to Reproduce:
var req = new XMLHttpRequest();
req.upload.addEventListener('progress', function(evt) {
   if (evt.loaded == evt.total) {
      // In Firefox this is never the case, since progress event is not called anymore when all bytes are sent !

      // In Goggle Chrome this works, since it is called one last time

      // Debatable which is the correct behavior, but chrome's is more usable
   }
}, false);

req.upload.addEventListener('load', function(evt) {
   // In FF this event fires to late, e.g. only after the response is sent back.
   // evt.loaded == evt.total == numberOfBytes sent

   // In Chrome this event fires correctly, right after all bytes are sent, but
   // evt.loaded == evt.total == 0 !   
}, false);

req.addEventListener('load', function(evt) {
   // Note: This is the load event on the req and not on the req.upload

   // In FF this event is correctly called when the response is back

   // Chrome fires it to early, right after all bytes are sent. 
}, false);




The docs at W3C on http://www.w3.org/TR/progress-events/#the-progressevent-interface below '4. Suggested ProgressEvent Types' in state, that the load event should be fired: 'After the last progress has been dispatched...'

Comment 1

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

(In reply to comment #0)
> When using the XmlHttpRequest.upload object to monitor upload progress, the
> ProgressEvent.load only fires, when the whole request is completed, e.g. the
> server has processed the request and the response is sent back.
Well, at the point when browser starts to get something back from server, it
knows the data has really been sent. Otherwise the data might be just buffered
somewhere in the network stack.


> Instead the event should
> fire as soon as the browser has sent all the bytes (e.g.finished uploading) and
> it should not wait for the server's response. You would use the
> XmlHttpRequest.load event for that.
upload's load fires way earlier than xhr's load.

Comment 2

[Simon Speich]

> (In reply to comment #0)
> > Instead the event should
> > fire as soon as the browser has sent all the bytes (e.g.finished uploading) and
> > it should not wait for the server's response. You would use the
> > XmlHttpRequest.load event for that.
> upload's load fires way earlier than xhr's load.
No, that's exactly the problem, that it doesn't

Comment 3

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

xhr's load fires after the request has been downloaded.
upload's load fires when the downloading starts.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Yeah, I think this is invalid.  Until we get a response from the server we don't know that that the data has actually been sent (and in fact in some cases we may have to resend it)....

Simon, the upload "load" fires when we get the start of the server's response (the headers).  The download "load" fires once we get all the data.  If you're testing with tiny server responses, you could be getting both in the same packet, of course.

Comment 5

[Simon Speich]

(In reply to comment #4)
> Yeah, I think this is invalid.  Until we get a response from the server we
> don't know that that the data has actually been sent (and in fact in some cases
> we may have to resend it)....
> 
If you leave this as invalid, would it at least be possible (or even necessary) to fire the progress event one last time (e.g. right when finished sending all bytes without waiting for the servers response) and to set evt.loaded == evt.total ?

> Simon, the upload "load" fires when we get the start of the server's response
> (the headers).  The download "load" fires once we get all the data.  If you're
> testing with tiny server responses, you could be getting both in the same
> packet, of course.

Thanks for the hint. I'm using PHP and that buffers the server's output...

Comment 6

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

(In reply to comment #5)
> If you leave this as invalid, would it at least be possible (or even necessary)
> to fire the progress event one last time (e.g. right when finished sending all
> bytes without waiting for the servers response) and to set evt.loaded ==
> evt.total ?
That could violate XMLHttpRequest v2 spec. progress events shouldn't fire
more often than every 50ms.

Comment 7

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

Btw, I just checked, Chrome does occasionally fire "progress" event way too
often.

Comment 9

[Boris Zbarsky [:bzbarsky]]

For what it's worth, I posted http://lists.w3.org/Archives/Public/public-webapps/2011JanMar/0988.html about the fact that Chrome and the spec seem to disagree here.

Comment 10

[Glenn Maynard]

(In reply to comment #6)
> (In reply to comment #5)
> > If you leave this as invalid, would it at least be possible (or even necessary)
> > to fire the progress event one last time (e.g. right when finished sending all
> > bytes without waiting for the servers response) and to set evt.loaded ==
> > evt.total ?
> That could violate XMLHttpRequest v2 spec. progress events shouldn't fire
> more often than every 50ms.

I've posted about this issue separately.  http://lists.w3.org/Archives/Public/public-webapps/2011JanMar/0993.html

Comment 11

[Simon Speich]

Does using FF5's new loadend event fix my problem?
https://developer.mozilla.org/En/XMLHttpRequest/Using_XMLHttpRequest#Detecting_any_load_end_condition

Comment 14

[Boris Zbarsky [:bzbarsky]]

Note that the spec discussion was ... inconclusive.

If we decide we want to change this behavior, we'll need a way for necko to tell us it's done sending the data.

Comment 15

[russell.mcclellan]

I ran into this while developing a website.  I've read http://www.w3.org/TR/XMLHttpRequest/#make-upload-progress-notifications , and it seems unequivocal that the load event should be sent 'irrespective of whether the server has started transmitting a response or the status code of such a response'.  Reads pretty clear to me (although I don't have much training with standards-speak)

Comment 16

[Boris Zbarsky [:bzbarsky]]

We should probably reopen.

I still think the spec is nonsense in the face of a retransmit, but it's hardly the first nonsensical spec out there....

Over to necko for an API that will tell us when to fire this event.

Comment 17

[Patrick McManus [:mcmanus]]

we can easily enough add a notification for when necko is done sending the data.. but we can't easily figure out when the data has arrived at the server though. Basically that would require a trigger on the TCP level ack of the last data byte and that's not-worth-the-trouble for at least two reasons:

1] OS Limitations don't have an event driven architecture for such a thing. To the extent its even possible (which may not be universal) we'd need to basically just continually poll the kernel asking if all the data had been acked yet. Constantly polling the kernel at mumble millisecond granularity is not a kind thing to do to your battery or the event loop.

2] propogation delay of the ack means the timestamp will be off by ~1/2 rtt anyhow.

But, like I said we can certainly notify the rest of gecko when the whole upload has been passed to the next layer. Realistically documents below a certain size will be moved into bufferspace immediately in the operating system. But for large uploads flow control will indeed push back on necko and the event will occur at least in the right general area of time. Note that modern networks need large buffers to work efficiently because they are both high bandwidth and often high delay too.

If the goal is to say "I have a server timeout of 2 seconds and I don't want the upload time of my video to count against that" it should be sufficient.. more granular use cases probably won't work awesomely.

Comment 18

[Boris Zbarsky [:bzbarsky]]

> If the goal is to say

It's not clear what the goal is.  None of the discussion on the list ever mentioned any actual use cases past presenting "upload is done" UI to the user, afaict.

Comment 19

[Patrick McManus [:mcmanus]]

this notion is pretty broken in the face of proxies as well - just another buffer.

Comment 20

[u456582]

Just a note.

If you send a file with XHR 2 with Chromium 33:
- when upload is finished, the 'xhr.upload.load' event is fired
- when processing of your file is finished on the server and the response received by the browser, the 'xhr.onreadystatechange.readyState=4' event is fired

In Firefox 28, events are fired simultaneously.

With Chromium you can write: 0%... 25%.... 50%... 75%... 100% PHP processing now... do other thing.
With Firefox you can only write: 0%... 25%.... 50%... 75%... do other thing.

(percent < 100 are written by 'xhr.upload.progress', percent = 100 and PHP processing is written by 'xhr.upload.load', do other thing is started by 'xhr.onreadystatechange.readyState=4')

See this: http://stackoverflow.com/a/15491086/2980105

Comment 21

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

(In reply to luigifab from comment #20)

> With Chromium you can write: 0%... 25%.... 50%... 75%... 100% PHP processing
> now... do other thing.
> With Firefox you can only write: 0%... 25%.... 50%... 75%... do other thing.
> 
> (percent < 100 are written by 'xhr.upload.progress', percent = 100 and PHP
> processing is written by 'xhr.upload.load', do other thing is started by
> 'xhr.onreadystatechange.readyState=4')
> 

In Chromium case nothing guarantees the data is in the php level when you get
"PHP processing". It can be just buffered somewhere in the network level.

Comment 22

[u456582]

(In reply to Olli Pettay [:smaug] from comment #21)
> In Chromium case nothing guarantees the data is in the php level when you get
> "PHP processing". It can be just buffered somewhere in the network level.

Yeah, but if the upload is ended, we are waiting a response from the server, no?

Comment 23

[Boris Zbarsky [:bzbarsky]]

No, we're waiting on an acknowledgement that the server has received the upload.  If that doesn't come, the upload can actually end up happening again, as I understand the situation.

Comment 24

[u456582]

Hum... so this is the web server (like lighttpd) which is not sending this acknowledgement?

upload start
- user >>>> data sending
upload end
- user <<<< server ack?
file processing by a php script
- user <<<< php response

Comment 25

[Boris Zbarsky [:bzbarsky]]

The problem is that the HTTP protocol does not provide a way for the HTTP server to acknowledge receipt of the request other than by starting the HTTP response.

But yes, the php script could send the response headers immediately and the data once it's done processing if it wanted to.

Comment 26

[u456582]

Okayyyyyyyyyy! So, it's simple.
Let's go for a complete example...

> // YOUR (SIMPLE) JAVASCRIPT FILE
> var form = new FormData(), xhr = new XMLHttpRequest();
> form.append('inputname', document.querySelector('input').files[0]);
> 
> xhr.open('POST', 'http://oneserver/onephpfile', true);
> xhr.setRequestHeader('X-CSRF-Token', 'somestring');
> 
> xhr.onreadystatechange = function () {
> 	if ((xhr.readyState === 4) && (xhr.status === 200))
> 		// do other thing with xhr.responseText.trim()
> };
> 
> xhr.upload.addEventListener('loadstart', showProgressBarFunction, false);
> xhr.upload.addEventListener('progress',  updateProgressBarFunction, false);
> xhr.upload.addEventListener('load',      updateProgressBarFunction, false);
> 
> xhr.send(form);

> // YOUR FIRST (SIMPLE) PHP FILE
> header('Content-Type: text/plain; charset=utf-8');
> header('Cache-Control: no-cache, must-revalidate');
> header('Pragma: no-cache');
> 
> sleep(20);
> echo 'file processing ended';

With this first PHP file, you will see: 10%... 50%... 75%... 'do other thing'
 with Firefox (4/10/28/32) and IE (10/11).

But you we will see: 10%... 50%... 75%... 100%... 'do other thing'
 with Chrome/Chromium (37/33) and Opera (24).

> // YOUR SECOND (SIMPLE) PHP FILE
> header('Content-Type: text/plain; charset=utf-8');
> header('Cache-Control: no-cache, must-revalidate');
> header('Pragma: no-cache');
> 
> header('Content-Encoding: chunked', true);
> header('Connection: Keep-Alive', true);
> ini_set('max_execution_time', 60);
> ini_set('output_buffering', false);
> ini_set('implicit_flush', true);
> ob_implicit_flush(true);
> for ($i = 0; $i < ob_get_level(); $i++)
> 	ob_end_clean();
> echo ' ';
> 
> sleep(20);
> echo 'file processing ended';

With this second PHP file, you will see: 10%... 50%... 75%... 100%... 'do other thing'
 with Chrome/Chromium (37/33), Opera (24), Firefox (4/10/28/32) and IE (10/11)!

I just think that Chrome and Opera displaying 100% before Firefox and Opera, but this is not very important...

Comment 27

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Comment 28

[BugBot [:suhaib / :marco/ :calixte]]

The severity field for this bug is relatively low, S3. However, the bug has 3 duplicates.
:kershaw, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Comment 29

[BugBot (nomail) [:suhaib / :marco/ :calixte]]

The last needinfo from me was triggered in error by recent activity on the bug. I'm clearing the needinfo since this is a very old bug and I don't know if it's still relevant.