[Webkit-unassigned] [Bug 13596] Implement .onprogress handler on XMLHttpRequest objects to support progressive download content length information

[bugzilla-daemon at webkit.org]

http://bugs.webkit.org/show_bug.cgi?id=13596





------- Comment #4 from julien.chaffraix at gmail.com  2008-04-01 04:58 PDT -------
(In reply to comment #3)
> (From update of attachment 20152 [edit])
> +        Our event does not match Firefox interface but matches the XHR2
> standard.
> 
> I think we'd need to have more clarity on why these are different. Is Firefox
> going to change to match the draft spec?
> 

As discussed on IRC, I have asked for a clarification on the public-webapi ML
(will post a link to the archive here when I have it).

> --- a/WebCore/WebCore.order
> +++ b/WebCore/WebCore.order
> 
> I do not think order files are to be manually edited.
> 
A dumb mistake, will be removed.

> +    m_receivedLength = 0;
> 
> It might make sense to consider how this will work with multipart responses -
> although it's not strictly necessary for this patch, given that we don't
> support those anyway yet.
> 

I have not though about multipart support, but I think this issue can be
postponed until we support it (will add a FIXME to indicate that multipart
support should be careful with that).

> +    // FIXME: We are called too often (3-4 times more that firefox) so we have
> a huge performance hit
> 
> Is this something that needs to be addressed at CFNetwork level? Can we make a
> workaround (I assume the hit is caused by the actual event handlers, so perhaps
> we could throttle event firing within WebCore)? We can't take a performance hit
> without at least having a clear plan on how to remedy it. But also, the scope
> of the problem is not quite obvious - what are the scenarios where this would
> be noticeable?
> 

Yes, it will be ! (my test case was loading a big file (~3/4Mb) and showing the
download progress, as we get called every 0.01% or so the perform hit was huge)

As you said the issue is the event handler. FYI, my testing were done on
gtk/curl so I think the issue should be solved in WebCore. I thought I would
work on it on a separate bug (to do maybe more perf testing). If you insist, I
can include the fix with this patch as is quite simple (just adding a byte
counter and firing the event when it is above a threshold).

> Also, "Firefox" with capital "F", and we usually put a dot at the end of a full
> sentence like this.
> 
> +    if (!expectedLength || m_receivedLength > expectedLength)
> +        evt = new ProgressEvent("progress", false, m_receivedLength,
> expectedLength);
> +    else
> +        evt = new ProgressEvent("progress", true, m_receivedLength,
> expectedLength);
> 
> This would look better with a local variable for "!expectedLength ||
> m_receivedLength > expectedLength" - no need for "if".
> 
> +    void updateAndDispatchOnProgress(int len);
> 
> In didReceiveData(), len is an int because it can be -1, indicating a
> null-terminated string. But in updateAndDispatchOnProgress(), the length is
> always known, so it would be better to use unsigned.
> 

I will address those in the next iteration.


-- 
Configure bugmail: http://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.