[webkit-reviews] review requested: [Bug 13596] Implement .onprogress handler on XMLHttpRequest objects to support progressive download content length information : [Attachment 20722] Updated with Ap's latest comments

[bugzilla-daemon at webkit.org]

Julien Chaffraix <julien.chaffraix at gmail.com> has asked  for review:
Bug 13596: Implement .onprogress handler on XMLHttpRequest objects to support
progressive download content length information
http://bugs.webkit.org/show_bug.cgi?id=13596

Attachment 20722: Updated with Ap's latest comments
http://bugs.webkit.org/attachment.cgi?id=20722&action=edit

------- Additional Comments from Julien Chaffraix <julien.chaffraix at gmail.com>
> I think that we need to support adding handlers via addEventListener, not
just
> .onprogress.

As mentioned on IRC, we can can register those handlers but they were not
called because of an error on my side (forgotten to call dispatchEvent). Should
be corrected.

> I have noticed that the spec actually describes when the event should be
fired:
>"every 350ms (+-200ms) or for every byte received, whichever is least
> frequent." I think it's OK to land a first version as is, to improve on this
> later.

I had not seen that. 

> +	   // XmlHttpRequest 2 extension
> Should be "XMLHttpRequest".

Corrected.

> What is wrong with returning m_loaded?

I had to check Firefox behaviour first: there was some mention of giving
special value if computableLength was false and I thought it could apply here
too. I have check and we match Firefox so removed the 2 comments.

> Should be progressEvent to avoid constructing an AtomicString from "progress"

> again and again.

Corrected too.

I only included a "proof of concept" test case and some of your comments ask
for more of them. I will bundle more and have them landed later to have a
broader regression test suite (the patch is big enough without them ;)).