Closed Bug 435241 Opened 12 years ago Closed 12 years ago

Remove NS_NewPipe usage from mailnews

Categories

(MailNews Core :: Backend, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.9

People

(Reporter: prasad, Assigned: prasad)

References

()

Details

Attachments

(6 files, 1 obsolete file)

Mailnews should not use NS_NewPipe, instead it should directly create the pipes using nsIPipe interface.  This is required for mailnews to link to frozen linkage.
Patch for mailnews/base/util.
Attachment #322151 - Flags: superreview?(dmose)
Attachment #322151 - Flags: review?(dmose)
Comment on attachment 322151 [details] [diff] [review]
Removes NS_NewPipe from mailnews/base/util

r+sr=dmose if you get replace the temporaries since each one causes an extra AddRef/Release pair.
Attachment #322151 - Flags: superreview?(dmose)
Attachment #322151 - Flags: superreview+
Attachment #322151 - Flags: review?(dmose)
Attachment #322151 - Flags: review+
uses raw pointers to get the streams.
fix for mailnews/base/util.
Attachment #322151 - Attachment is obsolete: true
Attachment #322156 - Flags: superreview?(dmose)
Attachment #322156 - Flags: review?(dmose)
Comment on attachment 322156 [details] [diff] [review]
[checked in] Removes NS_NewPipe from mailnews/base/util - v2

r+sr=dmose
Attachment #322156 - Flags: superreview?(dmose)
Attachment #322156 - Flags: superreview+
Attachment #322156 - Flags: review?(dmose)
Attachment #322156 - Flags: review+
Keywords: checkin-needed
Fixes mailnews/addrbook.
Again a tiny patch :)
Attachment #322163 - Flags: superreview?(dmose)
Attachment #322163 - Flags: review?(dmose)
Comment on attachment 322163 [details] [diff] [review]
[checked in] Removes NS_NewPipe from mailnews/addrbook

r+sr=dmose
Attachment #322163 - Flags: superreview?(dmose)
Attachment #322163 - Flags: superreview+
Attachment #322163 - Flags: review?(dmose)
Attachment #322163 - Flags: review+
Removes NS_NewPipe from mailnews/mime.

1.I removed the macros for segment size and buffer length because they are not used anywhere else and the buffer length has to anyway change to segment count when using nsIPipe directly.

2.In this case chaning nsI(Input|Output)Stream to nsIAsync(Input|Output)Stream directly in the class declaration seems to be of extremely low risk.  Actually, the mInputStream does not even seem to be required because it is a private variable and is used only in one function in the implementation of this class.
Attachment #322233 - Flags: superreview?(dmose)
Attachment #322233 - Flags: review?(dmose)
In comment (2) above, the extremely low risk can be read as 'no risk'. About mInputStream, i meant that it can be moved directly to the implementation of nsStreamConverter::Init instead of having a class variable.
Comment on attachment 322156 [details] [diff] [review]
[checked in] Removes NS_NewPipe from mailnews/base/util - v2

Checking in mailnews/base/util/nsMsgProtocol.cpp;
/cvsroot/mozilla/mailnews/base/util/nsMsgProtocol.cpp,v  <--  nsMsgProtocol.cpp
new revision: 1.154; previous revision: 1.153
done
Attachment #322156 - Attachment description: Removes NS_NewPipe from mailnews/base/util - v2 → [checked in] Removes NS_NewPipe from mailnews/base/util - v2
Comment on attachment 322163 [details] [diff] [review]
[checked in] Removes NS_NewPipe from mailnews/addrbook

Checking in mailnews/addrbook/src/nsAddbookProtocolHandler.cpp;
/cvsroot/mozilla/mailnews/addrbook/src/nsAddbookProtocolHandler.cpp,v  <--  nsAddbookProtocolHandler.cpp
new revision: 1.60; previous revision: 1.59
done
Attachment #322163 - Attachment description: Removes NS_NewPipe from mailnews/addrbook → [checked in] Removes NS_NewPipe from mailnews/addrbook
Patch for mailnews/imap:

Moved nsI(Output|Input)Stream to nsIAsync(Input|Output)Stream in the class declaration.  All the places where these are used in the implementations should be fine with this change.
Attachment #322264 - Flags: superreview?(dmose)
Attachment #322264 - Flags: review?(dmose)
Comment on attachment 322233 [details] [diff] [review]
[checked in] Removes NS_NewPipe from mailnews/mime

r+sr=dmose
Attachment #322233 - Flags: superreview?(dmose)
Attachment #322233 - Flags: superreview+
Attachment #322233 - Flags: review?(dmose)
Attachment #322233 - Flags: review+
Comment on attachment 322264 [details] [diff] [review]
[checked in] Removes NS_NewPipe from mailnews/imap

r+sr=dmose
Attachment #322264 - Flags: superreview?(dmose)
Attachment #322264 - Flags: superreview+
Attachment #322264 - Flags: review?(dmose)
Attachment #322264 - Flags: review+
Keywords: checkin-needed
Attachment #322276 - Flags: superreview?(dmose)
Attachment #322276 - Flags: review?(dmose)
Attachment #322277 - Flags: superreview?(dmose)
Attachment #322277 - Flags: review?(dmose)
Comment on attachment 322276 [details] [diff] [review]
[checked in] Patch for mailnews/news

r+sr=dmose
Attachment #322276 - Flags: superreview?(dmose)
Attachment #322276 - Flags: superreview+
Attachment #322276 - Flags: review?(dmose)
Attachment #322276 - Flags: review+
Comment on attachment 322277 [details] [diff] [review]
[checked in] Patch for mailnews/compose

r+sr=dmose
Attachment #322277 - Flags: superreview?(dmose)
Attachment #322277 - Flags: superreview+
Attachment #322277 - Flags: review?(dmose)
Attachment #322277 - Flags: review+
Attachment #322233 - Attachment description: Removes NS_NewPipe from mailnews/mime → [checked in] Removes NS_NewPipe from mailnews/mime
Comment on attachment 322264 [details] [diff] [review]
[checked in] Removes NS_NewPipe from mailnews/imap

Checking in mailnews/mime/src/nsStreamConverter.cpp;
/cvsroot/mozilla/mailnews/mime/src/nsStreamConverter.cpp,v  <--  nsStreamConverter.cpp
new revision: 1.137; previous revision: 1.136
done
Checking in mailnews/mime/src/nsStreamConverter.h;
/cvsroot/mozilla/mailnews/mime/src/nsStreamConverter.h,v  <--  nsStreamConverter.h
new revision: 1.28; previous revision: 1.27
done
Checking in mailnews/imap/src/nsImapProtocol.cpp;
/cvsroot/mozilla/mailnews/imap/src/nsImapProtocol.cpp,v  <--  nsImapProtocol.cpp
new revision: 1.683; previous revision: 1.682
done
Checking in mailnews/imap/src/nsImapProtocol.h;
/cvsroot/mozilla/mailnews/imap/src/nsImapProtocol.h,v  <--  nsImapProtocol.h
new revision: 1.212; previous revision: 1.211
done
Attachment #322264 - Attachment description: Removes NS_NewPipe from mailnews/imap → [checked in] Removes NS_NewPipe from mailnews/imap
Comment on attachment 322276 [details] [diff] [review]
[checked in] Patch for mailnews/news

Checking in mailnews/news/src/nsNNTPProtocol.cpp;
/cvsroot/mozilla/mailnews/news/src/nsNNTPProtocol.cpp,v  <--  nsNNTPProtocol.cpp
new revision: 1.400; previous revision: 1.399
done
Checking in mailnews/news/src/nsNNTPProtocol.h;
/cvsroot/mozilla/mailnews/news/src/nsNNTPProtocol.h,v  <--  nsNNTPProtocol.h
new revision: 1.107; previous revision: 1.106
done
Attachment #322276 - Attachment description: Patch for mailnews/news → [checked in] Patch for mailnews/news
Comment on attachment 322277 [details] [diff] [review]
[checked in] Patch for mailnews/compose

Checking in mailnews/compose/src/nsSmtpService.cpp;
/cvsroot/mozilla/mailnews/compose/src/nsSmtpService.cpp,v  <--  nsSmtpService.cpp
new revision: 1.152; previous revision: 1.151
done
Attachment #322277 - Attachment description: Patch for mailnews/compose → [checked in] Patch for mailnews/compose
All NS_NewPipe instances from mailnews are now gone :)
resolution => Fixed
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.