671657 - move/delete of imap message fails, locking UI, but move played back later

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[David :Bienvenu]

Attachment: proposed fix

A user reported the following issue:

1. move an imap message from Inbox to a folder you haven't opened (perhaps ever)
2. Move doesn't happen.
3. Press get new mail
4. UI locks up
5. When you restart, you can see that the move actually happened

From a combination msg copy service + imap offline + imap log, I was able to determine that nsImapMailFolder::CopyMessagesOffline was failing after it created the offline events in the source folder, so we don't do the actual delete. When you press get new mail, we play back the offline events, which does the move and deletes the source header from the db, but doesn't update the view correctly because m_deletingRows is still set, which leaves a garbage row in the view. The xul tree then locks up the UI because GetParentIndex is returning a garbage index, without returning an error, when asked about a row it doesn't know about.

I've managed to recreate this by using the debugger to to simulate failure in ::CopyMessagesOffline, and observing the resulting symptoms. In particular, I simulated a failure in the call to mDatabase->CopyHdrFromExistingHdr by skipping the call and setting rv to 0x80000000.

There are three parts to the fix:

1. Make GetParentIndex return an actual error on invalid index, which stops the xul code from hanging.

2. Notify move/copy error in this case, which makes m_deletingRows get set back to false. This fixes the bogus view line issue.

3. Attempt to playback offline operations when CopyMessagesOffline fails in the pseudo-offline case, in case offline operations were created. This makes the delete actually happen relatively quickly.

The one remaining issue is that we don't select a message after the offline playback delete happens, but that's relatively minor compared to the original bug.

I haven't figured out how to recreate the failure other than using the debugger (not for want of trying) but I think the fixes might help with a variety of issues.

Comment 1

[David :Bienvenu]

http://hg.mozilla.org/comm-central/rev/43bdfad505b3

Comment 2

[David :Bienvenu]

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

I'd like this for 7.0 since it's a general stability fix (though a little trunk baking would be good first)

Comment 3

[David :Bienvenu]

fixed for TB 7 - http://hg.mozilla.org/releases/comm-aurora/rev/58ab4ae9a089