343107 - STATE_START send after STATE_STOP if download finished early.

Status: RESOLVED FIXED

(Core Graveyard :: File Handling, defect)

Description

[Dorian]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.5) Gecko/20060627 K-Meleon/1.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.5) Gecko/20060627 K-Meleon/1.0

When a download is finished before the user choose the action to do, the nsITransfer object get STATE_STOP before STATE_START.


Reproducible: Always

Comment 1

[Dorian]

Attachment: Patch1

Send STATE_START before calling SetWebProgressListener() since it sends STATE_STOP if the transfert is already finished.

Comment 2

[:Gavin Sharp [email: [e-mail]]]

You need to specifically ask someone for review by putting their email address in the "requestee" field. I recommend cbiesinger@gmx.

Comment 3

[Dorian]

I'm sure I did it the first time, maybe it didn't like it.

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 227536 [details] [diff] [review]
Patch1

OK, upon looking at this in detail I also don't know how this fixes leaks. But it should certainly fix the wrong order, so r=biesi. Get sr from bz or darin.

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

(that comment was in reply to bug 276112 comment 4 and 5)

Comment 6

[Darin Fisher]

Comment on attachment 227536 [details] [diff] [review]
Patch1

perhaps the leak was introduced by firing events in the wrong order?

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

oh, maybe the STATE_STOP handler would drop the reference and thus break the cycle.

Comment 8

[Christian :Biesinger (don't email me, ping me on IRC)]

fixed on trunk
Checking in uriloader/exthandler/nsExternalHelperAppService.cpp;
/cvsroot/mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp,v  <--  nsExternalHelperAppService.cpp
new revision: 1.303; previous revision: 1.302
done

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 227536 [details] [diff] [review]
Patch1

this should be not much risk - it just makes sure to send START notifications before STOP ones.

Comment 10

[Darin Fisher]

Something for FF2 beta2.

Comment 11

[Christian :Biesinger (don't email me, ping me on IRC)]

Fixed on MOZILLA_1_8_BRANCH

Checking in nsExternalHelperAppService.cpp;
/cvsroot/mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp,v  <--  nsExternalHelperAppService.cpp
new revision: 1.290.4.7; previous revision: 1.290.4.6
done