Bug 856519 part 1 (nsLocalFolder.cpp)- Multiple messages moved from IMAP (offline-disabled) to maildir only moves last, r=irving, a=rkent
authorR Kent James <kent@caspia.com>
Thu, 08 Jan 2015 09:43:02 -0800
changeset 21611 ee06e1351a335b1add0611b9cb5025c054d235d3
parent 21610 c8113accba21c8d505c529e2e2c334267d09f81c
child 21612 f5f07c62c814bc13840dd43b92cfbabc2f35b28c
push id1305
push usermbanner@mozilla.com
push dateMon, 23 Feb 2015 19:48:12 +0000
treeherdercomm-beta@3ae4f13858fd [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersirving, rkent
bugs856519
Bug 856519 part 1 (nsLocalFolder.cpp)- Multiple messages moved from IMAP (offline-disabled) to maildir only moves last, r=irving, a=rkent
mailnews/local/src/nsLocalMailFolder.cpp
--- a/mailnews/local/src/nsLocalMailFolder.cpp
+++ b/mailnews/local/src/nsLocalMailFolder.cpp
@@ -1999,16 +1999,21 @@ NS_IMETHODIMP nsMsgLocalMailFolder::Begi
 
   nsresult rv;
   if (!mCopyState->m_copyingMultipleMessages)
   {
     rv = InitCopyMsgHdrAndFileStream();
     NS_ENSURE_SUCCESS(rv, rv);
   }
   nsCOMPtr <nsISeekableStream> seekableStream = do_QueryInterface(mCopyState->m_fileStream, &rv);
+
+  //  XXX ToDo: When copying multiple messages from a non-offline-enabled IMAP
+  //  server, this fails. (The copy succeeds because the file stream is created
+  //  subsequently in StartMessage) We should not be warning on an expected error.
+  //  Perhaps there are unexpected consequences of returning early?
   NS_ENSURE_SUCCESS(rv, rv);
   seekableStream->Seek(nsISeekableStream::NS_SEEK_END, 0);
 
   int32_t messageIndex = (mCopyState->m_copyingMultipleMessages) ? mCopyState->m_curCopyIndex - 1 : mCopyState->m_curCopyIndex;
   NS_ASSERTION(!mCopyState->m_copyingMultipleMessages || messageIndex >= 0, "messageIndex invalid");
   // by the time we get here, m_curCopyIndex is 1 relative because WriteStartOfNewMessage increments it
   mCopyState->m_messages->QueryElementAt(messageIndex, NS_GET_IID(nsIMsgDBHdr),
                                   (void **)getter_AddRefs(mCopyState->m_message));
@@ -2192,83 +2197,77 @@ NS_IMETHODIMP nsMsgLocalMailFolder::EndC
   if (!mCopyState)
     return NS_OK;
 
   // we are the destination folder for a move/copy
   nsresult rv = aCopySucceeded ? NS_OK : NS_ERROR_FAILURE;
 
   if (!aCopySucceeded || mCopyState->m_writeFailed)
   {
-    if (mCopyState->m_curDstKey != nsMsgKey_None)
-      mCopyState->m_msgStore->DiscardNewMessage(mCopyState->m_fileStream,
-                                                mCopyState->m_newHdr);
-
     if (mCopyState->m_fileStream)
+    {
+      if (mCopyState->m_curDstKey != nsMsgKey_None)
+        mCopyState->m_msgStore->DiscardNewMessage(mCopyState->m_fileStream,
+                                                  mCopyState->m_newHdr);
       mCopyState->m_fileStream->Close();
+    }
 
     if (!mCopyState->m_isMove)
     {
-      // passing true because the messages that have been successfully 
-      // copied have their corresponding hdrs in place. The message that has 
-      // failed has been truncated so the msf file and berkeley mailbox 
+      // passing true because the messages that have been successfully
+      // copied have their corresponding hdrs in place. The message that has
+      // failed has been truncated so the msf file and berkeley mailbox
       // are in sync.
       (void) OnCopyCompleted(mCopyState->m_srcSupport, true);
       // enable the dest folder
       EnableNotifications(allMessageCountNotifications, true, false /*dbBatching*/); //dest folder doesn't need db batching
     }
     return NS_OK;
   }
 
   bool multipleCopiesFinished = (mCopyState->m_curCopyIndex >= mCopyState->m_totalMsgCount);
 
   nsRefPtr<nsLocalMoveCopyMsgTxn> localUndoTxn = mCopyState->m_undoMsgTxn;
 
-  nsCOMPtr <nsISeekableStream> seekableStream;
   NS_ASSERTION(mCopyState->m_leftOver == 0, "whoops, something wrong with previous copy");
   mCopyState->m_leftOver = 0; // reset to 0.
   // need to reset this in case we're move/copying multiple msgs.
   mCopyState->m_fromLineSeen = false;
 
   // flush the copied message. We need a close at the end to get the
   // file size and time updated correctly.
-  if (mCopyState->m_fileStream)
+  //
+  // These filestream closes are handled inconsistently in the code. In some
+  // cases, this is done in EndMessage, while in others it is done here in
+  // EndCopy. When we do the close in EndMessage, we'll set
+  // mCopyState->m_fileStream to null since it is no longer needed, and detect
+  // here the null stream so we know that we don't have to close it here.
+  //
+  // Similarly, m_parseMsgState->GetNewMsgHdr() returns a null hdr if the hdr
+  // has already been processed by EndMessage so it is not doubly added here.
+
+  nsCOMPtr <nsISeekableStream> seekableStream(do_QueryInterface(mCopyState->m_fileStream));
+  if (seekableStream)
   {
-    seekableStream = do_QueryInterface(mCopyState->m_fileStream);
     if (mCopyState->m_dummyEnvelopeNeeded)
     {
       uint32_t bytesWritten;
       seekableStream->Seek(nsISeekableStream::NS_SEEK_END, 0);
       mCopyState->m_fileStream->Write(MSG_LINEBREAK, MSG_LINEBREAK_LEN, &bytesWritten);
       if (mCopyState->m_parseMsgState)
         mCopyState->m_parseMsgState->ParseAFolderLine(CRLF, MSG_LINEBREAK_LEN);
     }
-    // flush the copied message. We need a close at the end to get the
-    // file size and time updated correctly.
-    seekableStream = do_QueryInterface(mCopyState->m_fileStream);
-    if (mCopyState->m_dummyEnvelopeNeeded)
-    {
-      uint32_t bytesWritten;
-      seekableStream->Seek(nsISeekableStream::NS_SEEK_END, 0);
-      mCopyState->m_fileStream->Write(MSG_LINEBREAK, MSG_LINEBREAK_LEN,
-                                      &bytesWritten);
-      if (mCopyState->m_parseMsgState)
-        mCopyState->m_parseMsgState->ParseAFolderLine(CRLF, MSG_LINEBREAK_LEN);
-    }
-    // flush the copied message. We need a close at the end to get the
-    // file size and time updated correctly.
     rv = mCopyState->m_msgStore->FinishNewMessage(mCopyState->m_fileStream,
-                                             mCopyState->m_newHdr);
+                                                  mCopyState->m_newHdr);
     if (NS_SUCCEEDED(rv) && mCopyState->m_newHdr)
       mCopyState->m_newHdr->GetMessageKey(&mCopyState->m_curDstKey);
     if (multipleCopiesFinished)
       mCopyState->m_fileStream->Close();
     else
       mCopyState->m_fileStream->Flush();
-    mCopyState->m_msgStore->FinishNewMessage(mCopyState->m_fileStream,
-                                             mCopyState->m_newHdr);
   }
   //Copy the header to the new database
   if (mCopyState->m_message)
   {
     //  CopyMessages() goes here, and CopyFileMessages() with metadata to save;
     nsCOMPtr<nsIMsgDBHdr> newHdr;
     if (!mCopyState->m_parseMsgState)
     {
@@ -2575,16 +2574,21 @@ NS_IMETHODIMP nsMsgLocalMailFolder::EndM
   nsCOMPtr <nsISeekableStream> seekableStream = do_QueryInterface(mCopyState->m_fileStream, &rv);
   NS_ENSURE_SUCCESS(rv, rv);
   seekableStream->Seek(nsISeekableStream::NS_SEEK_END, 0);
   uint32_t bytesWritten;
   mCopyState->m_fileStream->Write(MSG_LINEBREAK, MSG_LINEBREAK_LEN, &bytesWritten);
   if (mCopyState->m_parseMsgState)
     mCopyState->m_parseMsgState->ParseAFolderLine(CRLF, MSG_LINEBREAK_LEN);
 
+  rv = mCopyState->m_msgStore->FinishNewMessage(mCopyState->m_fileStream,
+                                                mCopyState->m_newHdr);
+  mCopyState->m_fileStream->Close();
+  mCopyState->m_fileStream = nullptr; // all done with the file stream
+
   // CopyFileMessage() and CopyMessages() from servers other than mailbox
   if (mCopyState->m_parseMsgState)
   {
     nsCOMPtr<nsIMsgDatabase> msgDb;
     nsCOMPtr<nsIMsgDBHdr> newHdr;
 
     mCopyState->m_parseMsgState->FinishHeader();