From 970f6f664476d626ccae7e9cb303ee214fee9dd0 Mon Sep 17 00:00:00 2001 From: "scott%scott-macgregor.org" Date: Tue, 17 Jul 2007 00:04:04 +0000 Subject: [PATCH] Bug #388353 --> displaying a mail message leaks a jsContext. mailnews urls should use weak references for the msg window and the status feedback object sr=bienvenu git-svn-id: svn://10.0.0.236/trunk@230081 18797224-902f-48f8-a5cc-f745e15eee43 --- .../base/public/nsIMsgMailNewsUrl.idl | 110 +++++++++--------- mozilla/mailnews/base/src/nsMsgProgress.cpp | 1 + mozilla/mailnews/base/src/nsMsgProgress.h | 3 +- .../mailnews/base/util/nsMsgMailNewsUrl.cpp | 101 +++++----------- mozilla/mailnews/base/util/nsMsgMailNewsUrl.h | 5 +- mozilla/mailnews/imap/src/nsImapUrl.cpp | 25 ++-- mozilla/mailnews/local/src/nsMailboxUrl.cpp | 11 +- 7 files changed, 111 insertions(+), 145 deletions(-) diff --git a/mozilla/mailnews/base/public/nsIMsgMailNewsUrl.idl b/mozilla/mailnews/base/public/nsIMsgMailNewsUrl.idl index 5be99f0e8b3..c1dcdd2a064 100644 --- a/mozilla/mailnews/base/public/nsIMsgMailNewsUrl.idl +++ b/mozilla/mailnews/base/public/nsIMsgMailNewsUrl.idl @@ -53,66 +53,63 @@ interface nsIMsgFolder; interface nsIMsgHeaderSink; interface nsIMsgDBHdr; -[scriptable, uuid(0f9fb55c-f9ac-4501-af93-d4a7f23f82fb)] +[scriptable, uuid(2FB327C2-00A3-4e3f-8CD6-93BED40BD621)] interface nsIMsgMailNewsUrl : nsIURL { - /////////////////////////////////////////////////////////////////////////////// - // Eventually we'd like to push this type of functionality up into nsIURI. - // The idea is to allow the "application" (the part of the code which wants to - // run a url in order to perform some action) to register itself as a listener - // on url. As a url listener, the app will be informed when the url begins to run - // and when the url is finished. - //////////////////////////////////////////////////////////////////////////////// - void RegisterListener (in nsIUrlListener aUrlListener); - void UnRegisterListener (in nsIUrlListener aUrlListener); + /////////////////////////////////////////////////////////////////////////////// + // Eventually we'd like to push this type of functionality up into nsIURI. + // The idea is to allow the "application" (the part of the code which wants to + // run a url in order to perform some action) to register itself as a listener + // on url. As a url listener, the app will be informed when the url begins to run + // and when the url is finished. + //////////////////////////////////////////////////////////////////////////////// + void RegisterListener (in nsIUrlListener aUrlListener); + void UnRegisterListener (in nsIUrlListener aUrlListener); - readonly attribute nsIURI baseURI; - // Eventually this will be removed and replaced with calls to nsIMsgStatusFeedback - attribute string errorMessage; + readonly attribute nsIURI baseURI; - // if you really want to know what the current state of the url is (running or not - // running) you should look into becoming a urlListener... - void SetUrlState(in boolean runningUrl, in nsresult aStatusCode); - void GetUrlState(out boolean runningUrl); + // if you really want to know what the current state of the url is (running or not + // running) you should look into becoming a urlListener... + void SetUrlState(in boolean runningUrl, in nsresult aStatusCode); + void GetUrlState(out boolean runningUrl); - readonly attribute nsIMsgIncomingServer server; - attribute nsIMsgFolder folder; + readonly attribute nsIMsgIncomingServer server; + attribute nsIMsgFolder folder; - // the ownership model for msg feedback - attribute nsIMsgStatusFeedback statusFeedback; + attribute nsIMsgStatusFeedback statusFeedback; - attribute nsIMsgWindow msgWindow; + attribute nsIMsgWindow msgWindow; - // current mime headers if reading message - attribute nsIMimeHeaders mimeHeaders; + // current mime headers if reading message + attribute nsIMimeHeaders mimeHeaders; - // the load group is computed from the msgWindow - readonly attribute nsILoadGroup loadGroup; + // the load group is computed from the msgWindow + readonly attribute nsILoadGroup loadGroup; - // search session, if we're running a search. - attribute nsIMsgSearchSession searchSession; - attribute boolean updatingFolder; - attribute boolean addToMemoryCache; - attribute boolean msgIsInLocalCache; - attribute boolean suppressErrorMsgs; // used to avoid displaying biff error messages + // search session, if we're running a search. + attribute nsIMsgSearchSession searchSession; + attribute boolean updatingFolder; + attribute boolean addToMemoryCache; + attribute boolean msgIsInLocalCache; + attribute boolean suppressErrorMsgs; // used to avoid displaying biff error messages - attribute nsICacheEntryDescriptor memCacheEntry; - // if we want mime to cache images fetched by this message, set this cache session. - attribute nsICacheSession imageCacheSession; - // hold onto this cache entry until url goes away. This is used by mime to keep - // images cached until the url is deleted so we don't get entries evicted - // before we've used them. We need to be sure this doesn't introduce - // circular references or the url will never get freed. - void cacheCacheEntry(in nsICacheEntryDescriptor cacheEntry); - void removeCacheEntry(in nsICacheEntryDescriptor cacheEntry); - const unsigned long eCopy = 0; - const unsigned long eMove = 1; - const unsigned long eDisplay = 2; - boolean IsUrlType(in unsigned long type); - nsIStreamListener getSaveAsListener(in boolean addDummyEnvelope, in nsIFile aFile); + attribute nsICacheEntryDescriptor memCacheEntry; + // if we want mime to cache images fetched by this message, set this cache session. + attribute nsICacheSession imageCacheSession; + // hold onto this cache entry until url goes away. This is used by mime to keep + // images cached until the url is deleted so we don't get entries evicted + // before we've used them. We need to be sure this doesn't introduce + // circular references or the url will never get freed. + void cacheCacheEntry(in nsICacheEntryDescriptor cacheEntry); + void removeCacheEntry(in nsICacheEntryDescriptor cacheEntry); + const unsigned long eCopy = 0; + const unsigned long eMove = 1; + const unsigned long eDisplay = 2; + boolean IsUrlType(in unsigned long type); + nsIStreamListener getSaveAsListener(in boolean addDummyEnvelope, in nsIFile aFile); - // typically the header sink is tied to the nsIMsgWindow, but in certain circumstances, a consumer - // may chose to provide its own header sink for this url - attribute nsIMsgHeaderSink msgHeaderSink; + // typically the header sink is tied to the nsIMsgWindow, but in certain circumstances, a consumer + // may chose to provide its own header sink for this url + attribute nsIMsgHeaderSink msgHeaderSink; }; ////////////////////////////////////////////////////////////////////////////////// @@ -132,12 +129,11 @@ interface nsIMsgMessageUrl : nsISupports { attribute string uri; // used by imap, pop and nntp in order to implement save message to disk attribute nsIFile messageFile; - attribute boolean AddDummyEnvelope; + attribute boolean AddDummyEnvelope; attribute boolean canonicalLineEnding; attribute string originalSpec; // a message db header for that message. readonly attribute nsIMsgDBHdr messageHeader; - }; ////////////////////////////////////////////////////////////////////////////////// @@ -149,10 +145,10 @@ interface nsIMsgMessageUrl : nsISupports { [scriptable, uuid(D71E0785-2862-11d4-98C1-001083010E9B)] interface nsIMsgI18NUrl : nsISupports { - // the charset associated with a folder for this url..... - readonly attribute string folderCharset; - readonly attribute boolean folderCharsetOverride; - // the charsetOverRide is a charset the user may have specified via the menu for - // a particular message - attribute string charsetOverRide; + // the charset associated with a folder for this url..... + readonly attribute string folderCharset; + readonly attribute boolean folderCharsetOverride; + // the charsetOverRide is a charset the user may have specified via the menu for + // a particular message + attribute string charsetOverRide; }; diff --git a/mozilla/mailnews/base/src/nsMsgProgress.cpp b/mozilla/mailnews/base/src/nsMsgProgress.cpp index 23dbe9a007a..e010fc8d090 100644 --- a/mozilla/mailnews/base/src/nsMsgProgress.cpp +++ b/mozilla/mailnews/base/src/nsMsgProgress.cpp @@ -55,6 +55,7 @@ NS_INTERFACE_MAP_BEGIN(nsMsgProgress) NS_INTERFACE_MAP_ENTRY(nsIMsgStatusFeedback) NS_INTERFACE_MAP_ENTRY(nsIWebProgressListener) NS_INTERFACE_MAP_ENTRY(nsIProgressEventSink) + NS_INTERFACE_MAP_ENTRY(nsISupportsWeakReference) NS_INTERFACE_MAP_END_THREADSAFE diff --git a/mozilla/mailnews/base/src/nsMsgProgress.h b/mozilla/mailnews/base/src/nsMsgProgress.h index 7f374c8c4d0..a16214af17f 100644 --- a/mozilla/mailnews/base/src/nsMsgProgress.h +++ b/mozilla/mailnews/base/src/nsMsgProgress.h @@ -48,7 +48,8 @@ #include "nsIStringBundle.h" #include "nsWeakReference.h" -class nsMsgProgress : public nsIMsgProgress, public nsIMsgStatusFeedback, public nsIProgressEventSink +class nsMsgProgress : public nsIMsgProgress, public nsIMsgStatusFeedback, public nsIProgressEventSink, + public nsSupportsWeakReference { public: NS_DECL_ISUPPORTS diff --git a/mozilla/mailnews/base/util/nsMsgMailNewsUrl.cpp b/mozilla/mailnews/base/util/nsMsgMailNewsUrl.cpp index d9bba1cac69..410a8e27a3c 100644 --- a/mozilla/mailnews/base/util/nsMsgMailNewsUrl.cpp +++ b/mozilla/mailnews/base/util/nsMsgMailNewsUrl.cpp @@ -148,18 +148,6 @@ nsresult nsMsgMailNewsUrl::UnRegisterListener (nsIUrlListener * aUrlListener) return NS_OK; } -nsresult nsMsgMailNewsUrl::SetErrorMessage (const char * errorMessage) -{ - // functionality has been moved to nsIMsgStatusFeedback - return NS_ERROR_NOT_IMPLEMENTED; -} - -nsresult nsMsgMailNewsUrl::GetErrorMessage (char ** errorMessage) -{ - // functionality has been moved to nsIMsgStatusFeedback - return NS_ERROR_NOT_IMPLEMENTED; -} - NS_IMETHODIMP nsMsgMailNewsUrl::GetServer(nsIMsgIncomingServer ** aIncomingServer) { // mscott --> we could cache a copy of the server here....but if we did, we run @@ -208,54 +196,49 @@ NS_IMETHODIMP nsMsgMailNewsUrl::GetServer(nsIMsgIncomingServer ** aIncomingServe return rv; } -NS_IMETHODIMP nsMsgMailNewsUrl::SetStatusFeedback(nsIMsgStatusFeedback *aMsgFeedback) -{ - if (aMsgFeedback) - m_statusFeedback = do_QueryInterface(aMsgFeedback); - return NS_OK; -} - NS_IMETHODIMP nsMsgMailNewsUrl::GetMsgWindow(nsIMsgWindow **aMsgWindow) { NS_ENSURE_ARG_POINTER(aMsgWindow); + *aMsgWindow = nsnull; // note: it is okay to return a null msg window and not return an error // it's possible the url really doesn't have msg window - - *aMsgWindow = m_msgWindow; - NS_IF_ADDREF(*aMsgWindow); - + nsCOMPtr msgWindow(do_QueryReferent(m_msgWindowWeak)); + msgWindow.swap(*aMsgWindow); return NS_OK; } NS_IMETHODIMP nsMsgMailNewsUrl::SetMsgWindow(nsIMsgWindow *aMsgWindow) { if (aMsgWindow) - m_msgWindow = do_QueryInterface(aMsgWindow); + m_msgWindowWeak = do_GetWeakReference(aMsgWindow); return NS_OK; } NS_IMETHODIMP nsMsgMailNewsUrl::GetStatusFeedback(nsIMsgStatusFeedback **aMsgFeedback) { - nsresult rv = NS_OK; // note: it is okay to return a null status feedback and not return an error // it's possible the url really doesn't have status feedback - if (!m_statusFeedback) + *aMsgFeedback = nsnull; + if (!m_statusFeedbackWeak) { - - if(m_msgWindow) - { - m_msgWindow->GetStatusFeedback(getter_AddRefs(m_statusFeedback)); - } - } - if (aMsgFeedback) - { - *aMsgFeedback = m_statusFeedback; - NS_IF_ADDREF(*aMsgFeedback); + nsCOMPtr msgWindow(do_QueryReferent(m_msgWindowWeak)); + if (msgWindow) + msgWindow->GetStatusFeedback(aMsgFeedback); } else - rv = NS_ERROR_NULL_POINTER; - return rv; + { + nsCOMPtr statusFeedback(do_QueryReferent(m_statusFeedbackWeak)); + statusFeedback.swap(*aMsgFeedback); + } + return *aMsgFeedback ? NS_OK : NS_ERROR_NULL_POINTER; +} + +NS_IMETHODIMP nsMsgMailNewsUrl::SetStatusFeedback(nsIMsgStatusFeedback *aMsgFeedback) +{ + if (aMsgFeedback) + m_statusFeedbackWeak = do_GetWeakReference(aMsgFeedback); + return NS_OK; } NS_IMETHODIMP nsMsgMailNewsUrl::GetLoadGroup(nsILoadGroup **aLoadGroup) @@ -265,37 +248,14 @@ NS_IMETHODIMP nsMsgMailNewsUrl::GetLoadGroup(nsILoadGroup **aLoadGroup) // it's possible the url really doesn't have load group if (!m_loadGroup) { - if (m_msgWindow) + nsCOMPtr msgWindow(do_QueryReferent(m_msgWindowWeak)); + if (msgWindow) { - // XXXbz This is really weird... why are we getting some - // random loadgroup we're not really a part of? - nsCOMPtr docShell; - m_msgWindow->GetRootDocShell(getter_AddRefs(docShell)); - -#if 0 // since we're not going through the doc loader for most mail/news urls, - //, this code isn't useful - // but I can imagine it could be useful at some point. - - // load group needs status feedback set, since it's - // not the main window load group. - nsCOMPtr statusFeedback; - m_msgWindow->GetStatusFeedback(getter_AddRefs(statusFeedback)); - - if (statusFeedback) - { - nsCOMPtr webProgress(do_GetInterface(docShell)); - nsCOMPtr webProgressListener(do_QueryInterface(statusFeedback)); - - // register our status feedback object - if (statusFeedback && docShell) - { - webProgressListener = do_QueryInterface(statusFeedback); - webProgress->AddProgressListener(webProgressListener, - nsIWebProgress::NOTIFY_ALL); - } - } -#endif - m_loadGroup = do_GetInterface(docShell); + // XXXbz This is really weird... why are we getting some + // random loadgroup we're not really a part of? + nsCOMPtr docShell; + msgWindow->GetRootDocShell(getter_AddRefs(docShell)); + m_loadGroup = do_GetInterface(docShell); } } @@ -580,11 +540,12 @@ NS_IMETHODIMP nsMsgMailNewsUrl::Clone(nsIURI **_retval) NS_ENSURE_SUCCESS(rv, rv); // add the msg window to the cloned url - if (m_msgWindow) + nsCOMPtr msgWindow(do_QueryReferent(m_msgWindowWeak)); + if (msgWindow) { nsCOMPtr msgMailNewsUrl = do_QueryInterface(*_retval, &rv); NS_ENSURE_SUCCESS(rv, rv); - msgMailNewsUrl->SetMsgWindow(m_msgWindow); + msgMailNewsUrl->SetMsgWindow(msgWindow); } return rv; diff --git a/mozilla/mailnews/base/util/nsMsgMailNewsUrl.h b/mozilla/mailnews/base/util/nsMsgMailNewsUrl.h index faaa7f3929f..fc632893ad4 100644 --- a/mozilla/mailnews/base/util/nsMsgMailNewsUrl.h +++ b/mozilla/mailnews/base/util/nsMsgMailNewsUrl.h @@ -55,6 +55,7 @@ #include "nsICacheSession.h" #include "nsISupportsArray.h" #include "nsIMimeMiscStatus.h" +#include "nsWeakReference.h" /////////////////////////////////////////////////////////////////////////////////// // Okay, I found that all of the mail and news url interfaces needed to support @@ -84,8 +85,8 @@ protected: virtual const char * GetUserName() = 0; nsCOMPtr m_baseURL; - nsCOMPtr m_statusFeedback; - nsCOMPtr m_msgWindow; + nsWeakPtr m_statusFeedbackWeak; + nsWeakPtr m_msgWindowWeak; nsCOMPtr m_loadGroup; nsCOMPtr mMimeHeaders; nsCOMPtr m_searchSession; diff --git a/mozilla/mailnews/imap/src/nsImapUrl.cpp b/mozilla/mailnews/imap/src/nsImapUrl.cpp index bc038c08680..7c839f77776 100644 --- a/mozilla/mailnews/imap/src/nsImapUrl.cpp +++ b/mozilla/mailnews/imap/src/nsImapUrl.cpp @@ -121,26 +121,29 @@ NS_IMETHODIMP nsImapUrl::SetMsgWindow(nsIMsgWindow *aMsgWindow) { if (aMsgWindow) { - m_msgWindow = do_QueryInterface(aMsgWindow); + nsMsgMailNewsUrl::SetMsgWindow(aMsgWindow); if (m_mockChannel) { m_mockChannel->SetURI(this); nsCOMPtr msgDocShell; - m_msgWindow->GetRootDocShell(getter_AddRefs(msgDocShell)); - if (msgDocShell) { - nsCOMPtr prevEventSink; - m_mockChannel->GetProgressEventSink(getter_AddRefs(prevEventSink)); - nsCOMPtr docIR(do_QueryInterface(msgDocShell)); - m_mockChannel->SetNotificationCallbacks(docIR); - // we want to use our existing event sink. - if (prevEventSink) - m_mockChannel->SetProgressEventSink(prevEventSink); + nsCOMPtr msgWindow(do_QueryReferent(m_msgWindowWeak)); + if (msgWindow) + { + msgWindow->GetRootDocShell(getter_AddRefs(msgDocShell)); + if (msgDocShell) { + nsCOMPtr prevEventSink; + m_mockChannel->GetProgressEventSink(getter_AddRefs(prevEventSink)); + nsCOMPtr docIR(do_QueryInterface(msgDocShell)); + m_mockChannel->SetNotificationCallbacks(docIR); + // we want to use our existing event sink. + if (prevEventSink) + m_mockChannel->SetProgressEventSink(prevEventSink); + } } } } return NS_OK; } - nsImapUrl::~nsImapUrl() { PR_FREEIF(m_listOfMessageIds); diff --git a/mozilla/mailnews/local/src/nsMailboxUrl.cpp b/mozilla/mailnews/local/src/nsMailboxUrl.cpp index f0a7f5a520a..f01c6b135be 100644 --- a/mozilla/mailnews/local/src/nsMailboxUrl.cpp +++ b/mozilla/mailnews/local/src/nsMailboxUrl.cpp @@ -228,17 +228,20 @@ nsresult nsMailboxUrl::GetMsgHdrForKey(nsMsgKey msgKey, nsIMsgDBHdr ** aMsgHdr) rv = mailDB->GetMsgHdrForKey(msgKey, aMsgHdr); else { - if (!m_msgWindow) + nsCOMPtr msgWindow(do_QueryReferent(m_msgWindowWeak)); + if (!msgWindow) { nsCOMPtr mailSession = do_GetService(NS_MSGMAILSESSION_CONTRACTID, &rv); NS_ENSURE_SUCCESS(rv, rv); - mailSession->GetTopmostMsgWindow(getter_AddRefs(m_msgWindow)); + mailSession->GetTopmostMsgWindow(getter_AddRefs(msgWindow)); + SetMsgWindow(msgWindow); } + // maybe this is .eml file we're trying to read. See if we can get a header from the header sink. - if (m_msgWindow) + if (msgWindow) { nsCOMPtr headerSink; - m_msgWindow->GetMsgHeaderSink(getter_AddRefs(headerSink)); + msgWindow->GetMsgHeaderSink(getter_AddRefs(headerSink)); if (headerSink) return headerSink->GetDummyMsgHeader(aMsgHdr); }