Closed Bug 473740 Opened 11 years ago Closed 11 years ago

Tidy up send later service and drop the nsIMessenger middle-man

Categories

(MailNews Core :: Networking: SMTP, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.0b2

People

(Reporter: standard8, Assigned: standard8)

References

Details

(Keywords: memory-footprint)

Attachments

(1 file, 1 obsolete file)

Attached patch The fix (obsolete) — Splinter Review
There's a few problems with the send later service that I want to address before I start on using it for background sending. Here's some of them:

- It is inconsistently used as an instance and a service.
- nsIMessenger acts as a middle-man, supposedly to tell the message window if the send unsent messages action is in progress or not. Unfortunately this means the notification won't work cross mail windows, and just increases code size.
- The onStartSending notification isn't sent.
- The interfaces aren't documented.
- We're initialising nsCOMPtr<>s when we don't need to.

There looks to be more problems as well, but we'll leave those to another patch.

For the nsIMessenger issue, I've moved the status into nsIMsgSendLater and hence the UI goes directly to that now.

There's some XXX comments in the code for things I need to look at and address later.
Attachment #357127 - Flags: superreview?(bienvenu)
Attachment #357127 - Flags: review?(neil)
Comment on attachment 357127 [details] [diff] [review]
The fix

>nsMsgSendLater.cpp(159) : error C2065: 'NS_ERROR_QUEUED_DELIVERY_FAILED' : undeclared identifier
>nsMsgSendLater.cpp(1055) : error C2065: 'NS_MSG_ERROR_WRITING_FILE' : undeclared identifier
Attachment #357127 - Flags: review?(neil) → review-
(also some whitespace errors)
Hmmm, I've gotten into the habit of using my unsent folder as a "work-in-progress" sort of archive. 
I suppose I should clean that up before my store of 3k+ messages suddenly gets sent unintentionally in the background.
(In reply to comment #3)
> Hmmm, I've gotten into the habit of using my unsent folder as a
> "work-in-progress" sort of archive. 
> I suppose I should clean that up before my store of 3k+ messages suddenly gets
> sent unintentionally in the background.

Joe, this bug won't do that. This bug is just tidying things up and sorting out some bugs. You want bug 440794. Most people use drafts for work-in-progress items ;-)
Attached patch The fix v2Splinter Review
Fix build bustage and whitespace issues, also improved some of the comments.
Attachment #357127 - Attachment is obsolete: true
Attachment #357480 - Flags: superreview?(bienvenu)
Attachment #357480 - Flags: review?(neil)
Attachment #357127 - Flags: superreview?(bienvenu)
Attachment #357480 - Flags: review?(neil) → review+
Attachment #357480 - Flags: superreview?(bienvenu) → superreview+
Comment on attachment 357480 [details] [diff] [review]
The fix v2

would be nice to get rid of kMsgSendLaterCID and use NS_MSGSENDLATER_CONTRACTID instead.
Checked in: http://hg.mozilla.org/comm-central/rev/8cedb9f7ad8c

(In reply to comment #6)
> (From update of attachment 357480 [details] [diff] [review])
> would be nice to get rid of kMsgSendLaterCID and use NS_MSGSENDLATER_CONTRACTID
> instead.

I forgot to do that on checkin, however I'm touching the service more yet so I'll try and remember to do it on the next patch.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b3
Product: Core → MailNews Core
Target Milestone: mozilla1.9.1b3 → Thunderbird 3.0b2
You need to log in before you can comment on or make changes to this bug.