Closed
Bug 451991
Opened 16 years ago
Closed 16 years ago
nsIProgressEventSink::OnProgress reports wrong values when uploading
Categories
(Core :: Networking, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: smaug, Assigned: smaug)
References
Details
Attachments
(3 files, 1 obsolete file)
1.03 KB,
patch
|
Details | Diff | Splinter Review | |
1.98 KB,
patch
|
Biesinger
:
review+
Biesinger
:
superreview+
|
Details | Diff | Splinter Review |
1.99 KB,
patch
|
Details | Diff | Splinter Review |
Occasionally when creating lots of XMLHttpRequest objects, so that some of them
have to wait others to finish, OnProgress seems to get called with
parameters where aProgress > aProgressMax.
aProgressMax does have the right value (size of headers + size of content), so
it is aProgress which is wrong.
I used this http://mozilla.pettay.fi/xhr_upload/xhr_upload_demo_test.html
to test. It dumps info to console if something goes wrong. Just create 5 XHRs, and
press the button few times.
I verified the problem with some printfs in XHR ::OnProgress.
Assignee | ||
Comment 1•16 years ago
|
||
Assignee | ||
Comment 2•16 years ago
|
||
It seems that nsSocketOutputStream::Write increases mByteCount too much.
mByteCount gets increased up to twice too much?
I'm very unfamiliar with necko code, still trying to find why that happens
and if mByteCount should be reseted somwhere or if nsSocketOutputStream::Write is
called too often...
XHR uploads the right data, so aProgress doesn't reflect the actually amount
of sent data.
Comment 3•16 years ago
|
||
nsSocketOutputStream::Write increases mByteCount by the number it returns for bytes written: the return value of PR_Write. That seems correct, no?
Assignee | ||
Comment 4•16 years ago
|
||
Or perhaps some extra data is sent, but content-length has still the original value. /me continues debugging.
Assignee | ||
Comment 5•16 years ago
|
||
Finally found the problem, sort for. It is in nsHttpConnection::Activate, which
doesn't recreate SocketTransport if there is one already. And that causes
mSocketOut to be reused, but without re-initializing it in any way.
Will try to come up with a patch (first ever in necko).
Assignee: nobody → Olli.Pettay
Assignee | ||
Comment 6•16 years ago
|
||
And for download aProgress is right because it is synthesized in nsHttpChannel::OnDataAvailable
Assignee | ||
Comment 7•16 years ago
|
||
So far I haven't found any better option, but as I said, I'm not familiar with this code.
Attachment #335514 -
Flags: review?(cbiesinger)
Comment 8•16 years ago
|
||
I don't really like messing with the socket transport like that. Can you handle this in the HTTP code instead?
Assignee | ||
Comment 9•16 years ago
|
||
Attachment #335514 -
Attachment is obsolete: true
Attachment #335886 -
Flags: superreview?
Attachment #335886 -
Flags: review?(cbiesinger)
Attachment #335514 -
Flags: review?(cbiesinger)
Assignee | ||
Updated•16 years ago
|
Attachment #335886 -
Flags: superreview? → superreview?(cbiesinger)
Updated•16 years ago
|
Attachment #335886 -
Flags: superreview?(cbiesinger)
Attachment #335886 -
Flags: superreview+
Attachment #335886 -
Flags: review?(cbiesinger)
Attachment #335886 -
Flags: review+
Comment 10•16 years ago
|
||
Comment on attachment 335886 [details] [diff] [review]
update progress value in http code
+ nsCOMPtr<nsISeekableStream> seekable = do_QueryInterface(mRequestStream);
+ if (!seekable) {
I don't see how this QI can fail. Can you replace the if with an assertion?
Assignee | ||
Comment 11•16 years ago
|
||
Assignee | ||
Updated•16 years ago
|
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•