bug 1242030: DONT-USE-REUSABLE Improve error handling in file/stream I/O. Part 3 (was 2). r=jorgk. revision 2 draft
authorISHIKAWA, Chiaki <ishikawa@yk.rim.or.jp>
Sat, 09 Nov 2019 10:59:42 +0900
changeset 81057 2f506aba41571a3d02a93e33160ef3d8cb4f4088
parent 81056 28ac72095d82940f9091ac285a031b2b1f2df00a
child 81058 d95739b97c93c0f3e61b7fd30452f9d85384ae26
push id9744
push userishikawa@yk.rim.or.jp
push dateSat, 09 Nov 2019 02:01:06 +0000
treeherdertry-comm-central@56c83244ac0e [default view] [failures only]
reviewersjorgk
bugs1242030
bug 1242030: DONT-USE-REUSABLE Improve error handling in file/stream I/O. Part 3 (was 2). r=jorgk. revision 2
mailnews/import/applemail/src/nsAppleMailImport.cpp
mailnews/import/becky/src/nsBeckyMail.cpp
mailnews/import/outlook/src/nsOutlookMail.cpp
--- a/mailnews/import/applemail/src/nsAppleMailImport.cpp
+++ b/mailnews/import/applemail/src/nsAppleMailImport.cpp
@@ -543,35 +543,35 @@ nsAppleMailImportMail::ImportMailbox(nsI
       currentEntry->IsFile(&isFile);
       if (!isFile) continue;
 
       nsAutoString leafName;
       currentEntry->GetLeafName(leafName);
       if (!StringEndsWith(leafName, NS_LITERAL_STRING(".emlx"))) continue;
 
       nsCOMPtr<nsIMsgDBHdr> msgHdr;
-      bool reusable;
-      rv =
-          msgStore->GetNewMsgOutputStream(aDstFolder, getter_AddRefs(msgHdr),
-                                          &reusable, getter_AddRefs(outStream));
+      // bool reusable;
+      rv = msgStore->GetNewMsgOutputStream(aDstFolder, getter_AddRefs(msgHdr),
+                                           getter_AddRefs(outStream));
       if (NS_FAILED(rv)) break;
 
       // add the data to the mbox stream
       if (NS_SUCCEEDED(nsEmlxHelperUtils::AddEmlxMessageToStream(currentEntry,
                                                                  outStream))) {
         mProgress++;
         msgStore->FinishNewMessage(outStream, msgHdr);
       } else {
         msgStore->DiscardNewMessage(outStream, msgHdr);
         break;
       }
-      if (!reusable) outStream->Close();
     }
-    if (outStream) outStream->Close();
+    // FnishNewMessage now closes outStream always
+    outStream = nullptr;
   }
+
   // just indicate that we're done, using the same number that we used to
   // estimate number of messages earlier.
   uint32_t finalSize;
   aMailbox->GetSize(&finalSize);
   mProgress = finalSize;
 
   // report that we successfully imported this mailbox
   ReportStatus(u"ApplemailImportMailboxSuccess", mailboxName, successLog);
--- a/mailnews/import/becky/src/nsBeckyMail.cpp
+++ b/mailnews/import/becky/src/nsBeckyMail.cpp
@@ -446,21 +446,21 @@ NS_IMETHODIMP ImportMessageRunnable::Run
   mResult = mFolder->GetMsgStore(getter_AddRefs(msgStore));
   NS_ENSURE_SUCCESS(mResult, NS_OK);
 
   nsCOMPtr<nsILineInputStream> lineStream;
   mResult = nsBeckyUtils::CreateLineInputStream(mMessageFile,
                                                 getter_AddRefs(lineStream));
   NS_ENSURE_SUCCESS(mResult, NS_OK);
 
-  bool reusable;
+  // bool reusable;
   nsCOMPtr<nsIMsgDBHdr> msgHdr;
   nsCOMPtr<nsIOutputStream> outputStream;
-  mResult = msgStore->GetNewMsgOutputStream(
-      mFolder, getter_AddRefs(msgHdr), &reusable, getter_AddRefs(outputStream));
+  mResult = msgStore->GetNewMsgOutputStream(mFolder, getter_AddRefs(msgHdr),
+                                            getter_AddRefs(outputStream));
   NS_ENSURE_SUCCESS(mResult, NS_OK);
 
   bool inHeader = true;
   bool more = true;
   nsAutoCString headers;
   while (NS_SUCCEEDED(mResult) && more) {
     nsAutoCString line;
     mResult = lineStream->ReadLine(line, &more);
@@ -470,38 +470,41 @@ NS_IMETHODIMP ImportMessageRunnable::Run
       if (IsEndOfHeaders(line)) {
         inHeader = false;
         mResult = WriteHeaders(headers, outputStream);
       } else {
         mResult = HandleHeaderLine(line, headers);
       }
     } else if (IsEndOfMessage(line)) {
       inHeader = true;
+      // FnishNewMessage now closes outputStream always
       mResult = msgStore->FinishNewMessage(outputStream, msgHdr);
-      if (!reusable) outputStream->Close();
+      outputStream = nullptr;
       mResult = msgStore->GetNewMsgOutputStream(mFolder, getter_AddRefs(msgHdr),
-                                                &reusable,
                                                 getter_AddRefs(outputStream));
     } else if (IsBeckyIncludeLine(line)) {
       mResult = WriteAttachmentFile(mMessageFile, line, outputStream);
     } else {
       uint32_t writtenBytes = 0;
       if (StringBeginsWith(line, NS_LITERAL_CSTRING("..")))
         line.Cut(0, 1);
       else if (CheckHeaderKey(line, "From"))
         line.Insert('>', 0);
 
       line.AppendLiteral(MSG_LINEBREAK);
       mResult = outputStream->Write(line.get(), line.Length(), &writtenBytes);
     }
   }
 
   if (outputStream) {
-    if (NS_FAILED(mResult)) msgStore->DiscardNewMessage(outputStream, msgHdr);
-    outputStream->Close();
+    if (NS_FAILED(mResult)) {
+      msgStore->DiscardNewMessage(outputStream, msgHdr);
+    }
+    // DiscardNewMessage now clears outputStream always.
+    outputStream = nullptr;
   }
 
   return NS_OK;
 }
 
 static nsresult ProxyImportMessage(nsIFile *aMessageFile,
                                    nsIMsgFolder *aFolder) {
   RefPtr<ImportMessageRunnable> importMessage =
--- a/mailnews/import/outlook/src/nsOutlookMail.cpp
+++ b/mailnews/import/outlook/src/nsOutlookMail.cpp
@@ -380,20 +380,19 @@ NS_IMETHODIMP ImportMailboxRunnable::Run
   while (!done) {
     if (!contents.GetNext(&cbEid, &lpEid, &oType, &done)) {
       IMPORT_LOG1("*** Error iterating mailbox: %S\n", mName);
       mResult = NS_ERROR_FAILURE;
       return NS_OK;  // Sync runnable must return OK.
     }
 
     nsCOMPtr<nsIMsgDBHdr> msgHdr;
-    bool reusable;
+    // bool reusable;
 
     rv = msgStore->GetNewMsgOutputStream(mDstFolder, getter_AddRefs(msgHdr),
-                                         &reusable,
                                          getter_AddRefs(outputStream));
     if (NS_FAILED(rv)) {
       IMPORT_LOG1("*** Error getting nsIOutputStream of mailbox: %S\n", mName);
       mResult = rv;
       return NS_OK;  // Sync runnable must return OK.
     }
     totalCount = contents.GetCount();
     doneCalc = *mMsgCount;
@@ -418,26 +417,32 @@ NS_IMETHODIMP ImportMailboxRunnable::Run
       nsMsgDeliverMode mode = nsIMsgSend::nsMsgDeliverNow;
       mode = nsIMsgSend::nsMsgSaveAsDraft;
       if (folderName.LowerCaseEqualsLiteral("drafts"))
         mode = nsIMsgSend::nsMsgSaveAsDraft;
 
       rv = ImportMessage(lpMsg, outputStream, mode);
       if (NS_SUCCEEDED(rv)) {  // No errors & really imported
         (*mMsgCount)++;
-        msgStore->FinishNewMessage(outputStream, msgHdr);
+        rv = msgStore->FinishNewMessage(outputStream, msgHdr);
       } else {
         IMPORT_LOG1("*** Error reading message from mailbox: %S\n", mName);
         msgStore->DiscardNewMessage(outputStream, msgHdr);
+        // XXX TODO if this error happens, we might want to
+        // return rv;
+        // But not sure what the error handling framework is.
       }
-      if (!reusable) outputStream->Close();
+      // FinishNewMEssage always close outputStream now.
+      outputStream = nullptr;
     }
   }
-
-  if (outputStream) outputStream->Close();
+  // XXX TODO I am not sure what is the right way to return
+  // error from Runnable.
+  // if (NS_FAILED(rv))  // ImportMessage or FinishMessage
+  //    return rv;
   return NS_OK;
 }
 
 nsresult ProxyImportMailbox(uint32_t *pDoneSoFar, bool *pAbort, int32_t index,
                             const char16_t *pName, nsIMsgFolder *dstFolder,
                             int32_t *pMsgCount, nsOutlookMail *aCaller) {
   RefPtr<ImportMailboxRunnable> importMailbox = new ImportMailboxRunnable(
       pDoneSoFar, pAbort, index, pName, dstFolder, pMsgCount, aCaller);