[webkit-reviews] review denied: [Bug 13596] Implement .onprogress handler on XMLHttpRequest objects to support progressive download content length information : [Attachment 20708] Updated after XMLHttpRequest changes and IDL introduction

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has denied Julien Chaffraix
<julien.chaffraix at gmail.com>'s request 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 20708: Updated after XMLHttpRequest changes and IDL introduction
http://bugs.webkit.org/attachment.cgi?id=20708&action=edit

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
I think that we need to support adding handlers via addEventListener, not just
.onprogress.

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.

+	 // XmlHttpRequest 2 extension

Should be "XMLHttpRequest".

+unsigned XMLHttpRequestProgressEvent::position()
+{
+    // FIXME: If lengthComputable is false, what should we return?
+    return m_loaded;
+}

What is wrong with returning m_loaded?

+    evt = new XMLHttpRequestProgressEvent("progress", expectedLength &&
m_receivedLength <= expectedLength, m_receivedLength, expectedLength);

Should be progressEvent to avoid constructing an AtomicString from "progress"
again and again.

I think that this is almost ready for landing, but not quite yet.