88889 - The transaction object for undo/redo should hold a weakptr reference to the folder

Status: VERIFIED FIXED

(MailNews Core :: Backend, defect)

Description

[Navin Gupta]

Comment 1

[Navin Gupta]

Attachment: proposed fix

Comment 2

[Navin Gupta]

So the Undo object has a weakptr to the src and dest folders. Also the 
imap/local folder no longer holds the transaction manager because it 
was causing txnMgr leaks, instead I use the msgWindow object to get 
the txnMgr object.

This patch also includes the fix for one of the cases of empty hdrs on 
undo in bug 86489. Upon deleting a folder it leaks so we try to
get database when we should not, therefore check if the folder exists on
disk. 

Comment 3

[David :Bienvenu]

yuck, nsLocalMailFolder shouldn't have a pointer to a msgWindow - what if you
have multiple message windows open?

Comment 4

[Navin Gupta]

That is nsLocalMailCopyState that I have msgWindow to. The nsLocalMailCopyState
class definition is in nsLocalMailFolder.cpp

Comment 5

[Navin Gupta]

Ignore these changes in nsImapMailFolder. They are not needed. 

+    PRBool exists;
+    rv = pathSpec->Exists(&exists);
+    NS_ENSURE_SUCCESS(rv,rv);
+    if (!exists) return NS_ERROR_NULL_POINTER;  //mDatabase will be null at 
this point.

Comment 6

[David :Bienvenu]

I know that this was messed up before you got here...but
the problem is not what source file it's defined in; it's what object holds onto
one. If an nsLocalMailFolder holds onto an nsIMsgWindow, then you're tying that
folder to just one window, and you're also introducing a great opportunity for
circular references. And I'm worried that people will start using that msgWindow
for other stuff, and it will break the whole model/view distinction that we
should be having. The copy state should belong to the url that's doing the copy.
Plus, unless I'm missing something, the nsLocalMailFolder::m_msgWindow never
gets set to anything.

Comment 7

[David :Bienvenu]

could you make this an nsXPIDLCString while you're at it:

 char *uri = nsnull;
-    rv = m_srcFolder->GetURI(&uri);
+    rv = srcFolder->GetURI(&uri);

and remove the PR_FREEIF?

otherwise, looks good, r=bienvenu

Comment 8

[Navin Gupta]

marking fixed. fix checked in last week

Comment 9

[Navin Gupta]

fixed.

Comment 10

[[e-mail] (gone - use [e-mail] instead)]

Verified FIXED using lxr.mozilla.org:

nsImapMailFolder.cpp 1.413
nsImapMailFolder.h 1.167
nsImapUndoTxn.cpp 1.30
nsImapUndoTxn.h 1.14
nsLocalMailFolder.cpp 1.300
nsLocalMailFolder.h 1.109
nsLocalUndoTxn.cpp 1.24
nsLocalUndoTxn.h 1.9