This is an archived snapshot of W3C's public bugzilla bug tracker, decommissioned in April 2019. Please see the home page for more details.

Bug 26917 - For any chunk, progress event should be fired at least in 50ms since the arrival of the chunk
Summary: For any chunk, progress event should be fired at least in 50ms since the arri...
Status: RESOLVED WORKSFORME
Alias: None
Product: WebAppsWG
Classification: Unclassified
Component: XHR (show other bugs)
Version: unspecified
Hardware: PC Linux
: P2 normal
Target Milestone: ---
Assignee: Anne
QA Contact: public-webapps-bugzilla
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-09-29 07:24 UTC by Takeshi Yoshino
Modified: 2016-08-12 09:13 UTC (History)
3 users (show)

See Also:

Attachments

Description Takeshi Yoshino 2014-09-29 07:24:46 UTC 
https://github.com/whatwg/fetch/commit/d7e357d83380b50886832316c84ae125193b58b4
https://github.com/whatwg/xhr/commit/0e3f85a60cd2938059d7594ba9242209aa13415d

Now the throttle (interval 50ms) is back to XHR spec again.

The new text says that "process response body" call made in 50ms since the last "process response body" call will be no-op. Right?

Please imagine situation where two chunks come at time n and n + (1 ms), and then the connection becomes idle. Based on the new text, we fire progress event for the first chunk, but do nothing for the second since we'll enter idle state.

Blink fires a progress event after 50ms passes since the last progress event firing if any chunk has arrived after the chunk which triggered the last progress event. This ensures that for any chunk at least one progress event will be fired for the chunk at least in 50ms since the chunk arrives.

I think this behavior is reasonable. What do you think?
Comment 1 Anne 2014-09-29 08:33:51 UTC 
Agreed. I was hoping to avoid more complicated text, but it seems like I should add some.
Comment 2 Anne 2014-09-29 09:22:21 UTC 
I'm trying to think what a good strategy is here. Perhaps we should go with your idea of storing the progress on the XMLHttpRequest object. And then queue a task every 50ms to dispatch the events from.
Comment 3 Anne 2014-10-14 14:09:44 UTC 
Takeshi, would comment 2 work?
Comment 4 Takeshi Yoshino 2014-10-15 03:18:34 UTC 
Is your plan to invoke the task every 50ms regardless of the time chunks arrive?

It should also work but a little different from ours. Is it for algorithm simplicity or any other benefit in your mind?

When a new chunk arrives, we check whether there's any task queued to aggregate progress events. If there is, we just update the progress data stored on the XHR object. If not, we fire a progress event and queue the task. The task is scheduled to be run after 50ms. It checks if there's any progress data stored on the XHR. If not, do nothing. If there is, fire a progress event from the stored data and queue the task again.

0ms chunkA
10ms chunkB
20ms chunkC
120ms chunkD

Then, we fire a progress event for chunkA at 0ms and one for chunkC at 50ms, and one for chunkD at 120ms.
Comment 5 Anne 2014-10-15 11:55:51 UTC 
It seems your approach does not accomplish having an event every 50ms (but does accomplish the "least frequent" wording from the spec before). That also seems fine to me.
Comment 6 Takeshi Yoshino 2014-10-16 02:55:28 UTC 
(In reply to Anne from comment #5)
> It seems your approach does not accomplish having an event every 50ms (but
> does accomplish the "least frequent" wording from the spec before). That
> also seems fine to me.

I think it's fine, too. If one needs to do something based on the progress every 50ms, he/she could save the progress from the progress event to somewhere and invoke a repeated task using a timer where we do something by looking at the saved progress.

Otherwise, less frequency would be nice basically.
Comment 7 Takeshi Yoshino 2014-10-16 03:14:58 UTC 
(In reply to Takeshi Yoshino from comment #6)
> (In reply to Anne from comment #5)
> > It seems your approach does not accomplish having an event every 50ms (but
> > does accomplish the "least frequent" wording from the spec before). That
> > also seems fine to me.
> 
> I think it's fine, too. If one needs to do something based on the progress
> every 50ms, he/she could save the progress from the progress event to
> somewhere and invoke a repeated task using a timer where we do something by
> looking at the saved progress.
> 
> Otherwise, less frequency would be nice basically.

By otherwise, I meant "if one doesn't need every-50ms dispatching".
Comment 8 Anne 2014-10-16 07:13:49 UTC 
I'm open to revisiting the 50ms rule. We could also just dispatch an event for each chunk. I'm not sure if that is typically rarer? Or we could make it more up to the user agent.
Comment 9 Anne 2016-08-12 09:13:05 UTC 
If you still want to change this, let's discuss it on GitHub.