135454 - Should disable Send Unsent Messages if we are currently sending unsent messages

Status: VERIFIED FIXED

(MailNews Core :: Networking: SMTP, defect)

Description

[David :Bienvenu]

spun off from bug 134624 - 

We should not allow Send Unsent Messages if we're already sending unsent
messages. Looking at the code, we could set a flag in the messenger object that
we're doing a send unsent and clear it when the send finishes, probably in the
send later listener, and then disable subsequent send unsent messages requests.

Comment 1

[Jaime Rodriguez, Jr.]

Marking nsbeta1+/ADT2 per ADT triage.

Comment 2

[David :Bienvenu]

Attachment: proposed fix

fix as described above. Unfortunately, ::OnStartSending never gets called, so
we have to set the sendingUnsentMsgs flag ourselves. I tried testing the stop
key and a non-existent server, but the stop key doesn't work, which I'll file
as another bug.

Comment 3

[David :Bienvenu]

JF, can I get a review? thx.

Comment 4

[David :Bienvenu]

Comment on attachment 79479 [details] [diff] [review]
proposed fix

whoops, attaching failed.

Comment 5

[David :Bienvenu]

Attachment: trying again, proposed fix

Comment 6

[(not reading, please use [e-mail] instead)]

Comment on attachment 79480 [details] [diff] [review]
trying again, proposed fix

sr=sspitzer

assuming that it is by design that OnStartSending() isn't getting called, and
not a problem.

ducarroz, is that documented in the code?

Comment 7

[Jean-Francois Ducarroz]

I don't now if it's ok than OnStartSending is not called but that doesn't sound
right! Let me check...

Comment 8

[Jean-Francois Ducarroz]

Correct, we never call NotifyListenerOnStartSending which is suppose to call
OnStartSending and I don't know why. That is true for send and sendlater. I'll
file a bug for that for further investigation. But for now, David patch is fine.

Comment 9

[Jean-Francois Ducarroz]

Comment on attachment 79480 [details] [diff] [review]
trying again, proposed fix

r=ducarroz

Comment 10

[David :Bienvenu]

Attachment: proposed fix using weak references

new fix, using weak references, in case the send later object is never
destroyed, don't want to hold onto the messenger object.

Comment 11

[(not reading, please use [e-mail] instead)]

Comment on attachment 79493 [details] [diff] [review]
proposed fix using weak references

sr=sspitzer

ducarroz, when you log the bug about OnStartSending() not getting called, can
you fix bienvenu's comment
("// this never gets called :-(") to point to that bug?

Comment 12

[David :Bienvenu]

fixed on trunk.

Comment 13

[scottputterman]

adding nsbeta1-.  Let's get this in the next release.

Comment 14

[Sheela Ravindran]

reassign->meehansqa

Comment 15

[Gregg Meehan]

nominating for Buffy

Comment 16

[Gregg Meehan]

Tested using Trunk build 20020905, working correctly. Closing as VERIFIED.