part of work to prevent corruption of offline imap stores when compacting offline stores, and miscellaneous cleanup 166617 r/sr=sspitzer

mailnews/base/src/nsMsgFolderCompactor.cpp

@@ -235,10 +235,26 @@ nsFolderCompactState::Compact(nsIMsgFolder *folder, nsIMsgWindow *aMsgWindow)
    NS_ENSURE_SUCCESS(rv,rv);
     
    rv = Init(folder, baseMessageURI, db, pathSpec, m_window);
-   if (NS_SUCCEEDED(rv))
+   NS_ENSURE_SUCCESS(rv,rv);
-      rv = StartCompacting();
 
-   return rv;
+   PRBool isLocked;
+   m_folder->GetLocked(&isLocked);
+   if(!isLocked)
+   {
+     nsCOMPtr <nsISupports> supports = do_QueryInterface(NS_STATIC_CAST(nsIMsgFolderCompactor*, this));
+     m_folder->AcquireSemaphore(supports);
+     return StartCompacting();
+   }
+   else
+   {
+     m_folder->NotifyCompactCompleted();
+     m_folder->ThrowAlertMsg("compactFolderDeniedLock", m_window);
+     CleanupTempFilesAfterError();
+     if (m_compactAll)
+       return CompactNextFolder();
+     else
+       return NS_OK;
+   }
 }
 
 nsresult nsFolderCompactState::ShowStatusMsg(const PRUnichar *aMsg)
@@ -328,21 +344,6 @@ NS_IMETHODIMP nsFolderCompactState::OnStopRunningUrl(nsIURI *url, nsresult statu
 nsresult nsFolderCompactState::StartCompacting()
 {
   nsresult rv = NS_OK;
-  PRBool isLocked;
-  nsCOMPtr <nsISupports> supports = do_QueryInterface(NS_STATIC_CAST(nsIMsgFolderCompactor*, this));
-  m_folder->GetLocked(&isLocked);
-  if(!isLocked)
-    m_folder->AcquireSemaphore(supports);
-  else
-  {
-    m_folder->NotifyCompactCompleted();
-    m_folder->ThrowAlertMsg("compactFolderDeniedLock", m_window);
-    CleanupTempFilesAfterError();
-    if (m_compactAll)
-      return CompactNextFolder();
-    else
-      return rv;
-  }
   if (m_size > 0)
   {
     ShowCompactingStatusMsg();
@@ -645,6 +646,7 @@ nsOfflineStoreCompactState::OnStopRequest(nsIRequest *request, nsISupports *ctxt
   nsCOMPtr<nsIMsgDBHdr> msgHdr;
   nsCOMPtr<nsIMsgDBHdr> newMsgHdr;
   nsCOMPtr <nsIMsgStatusFeedback> statusFeedback;
+  ReleaseFolderLock();
 
   if (NS_FAILED(rv)) goto done;
   uri = do_QueryInterface(ctxt, &rv);
@@ -835,6 +837,7 @@ nsresult nsOfflineStoreCompactState::StartCompacting()
   }
   else
   { // no messages to copy with
+    ReleaseFolderLock();
     FinishCompact();
 //    Release(); // we don't "own" ourselves yet.
   }

mailnews/imap/src/nsImapMailFolder.cpp

@@ -639,31 +639,28 @@ nsImapMailFolder::UpdateFolder(nsIMsgWindow *msgWindow)
   nsresult rv = NS_ERROR_NULL_POINTER;
   PRBool selectFolder = PR_FALSE;
   
-    if (mFlags & MSG_FOLDER_FLAG_INBOX) {
+  if (mFlags & MSG_FOLDER_FLAG_INBOX && !m_filterList) 
-        if (!m_filterList) {
       rv = GetFilterList(msgWindow, getter_AddRefs(m_filterList));
-            // XXX rv ignored
-        }
-    }
   
-    if (m_filterList) {
+  if (m_filterList) 
+  {
     nsCOMPtr<nsIMsgIncomingServer> server;
     rv = GetServer(getter_AddRefs(server));
     NS_ASSERTION(NS_SUCCEEDED(rv), "failed to get server");
     
     PRBool canFileMessagesOnServer = PR_TRUE;
-        if (server) {
+    if (server) 
-            rv = server->GetCanFileMessagesOnServer(
+    {
-                &canFileMessagesOnServer);
+      rv = server->GetCanFileMessagesOnServer(&canFileMessagesOnServer);
       NS_ASSERTION(NS_SUCCEEDED(rv), "failed to determine if we could file messages on this server");
     }
     
     // the mdn filter is for filing return receipts into the sent folder
     // some servers (like AOL mail servers)
     // can't file to the sent folder, so we don't add the filter for those servers
-        if (canFileMessagesOnServer) {
+    if (canFileMessagesOnServer) 
-            rv = server->ConfigureTemporaryReturnReceiptsFilter(
+    {
-                m_filterList);
+      rv = server->ConfigureTemporaryReturnReceiptsFilter(m_filterList);
       NS_ASSERTION(NS_SUCCEEDED(rv), "failed to add MDN filter");
     }
   }
@@ -703,10 +700,8 @@ nsImapMailFolder::UpdateFolder(nsIMsgWindow *msgWindow)
   {
     nsImapOfflineSync *goOnline = new nsImapOfflineSync(msgWindow, this, this);
     if (goOnline)
-      {
       return goOnline->ProcessNextOperation();
   }
-    }
   if (!canOpenThisFolder) 
     selectFolder = PR_FALSE;
   // don't run select if we're already running a url/select...
@@ -718,21 +713,25 @@ nsImapMailFolder::UpdateFolder(nsIMsgWindow *msgWindow)
     if (NS_SUCCEEDED(rv) && pEventQService)
       pEventQService->GetThreadEventQueue(NS_CURRENT_THREAD,
       getter_AddRefs(eventQ));
-            rv = imapService->SelectFolder(eventQ, this, this, msgWindow,
+    rv = imapService->SelectFolder(eventQ, this, this, msgWindow, nsnull);
-                                           nsnull);
+    switch (rv)
-            if ((rv == NS_MSG_ERROR_OFFLINE) ||
-                     (rv == NS_BINDING_ABORTED))
     {
+      case NS_MSG_ERROR_OFFLINE:
+        if (msgWindow)
+          AutoCompact(msgWindow);  
+        // note fall through to next case.
+      case NS_BINDING_ABORTED:
         rv = NS_OK;
         NotifyFolderEvent(mFolderLoadedAtom);
+        break;
+      default:
+        break;
     }
   }
   else if (NS_SUCCEEDED(rv))  // tell the front end that the folder is loaded if we're not going to 
   {                           // actually run a url.
     if (!m_urlRunning)        // if we're already running a url, we'll let that one send the folder loaded
       NotifyFolderEvent(mFolderLoadedAtom);
-            if (msgWindow) // don't do this w/o a msgWindow, since it asserts annoyingly
-                rv = AutoCompact(msgWindow);  
     NS_ENSURE_SUCCESS(rv,rv);
   }
   
@@ -2208,7 +2207,6 @@ NS_IMETHODIMP nsImapMailFolder::GetNewMessages(nsIMsgWindow *aWindow, nsIUrlList
 
     if (imapServer)
     {
-      imapServer->GetDownloadBodiesOnGetNewMail(&m_downloadingFolderForOfflineUse);
       nsCOMPtr<nsIMsgIncomingServer> incomingServer = do_QueryInterface(imapServer, &rv);
       if (incomingServer)
         incomingServer->GetPerformingBiff(&performingBiff);
@@ -3251,8 +3249,6 @@ NS_IMETHODIMP nsImapMailFolder::FolderPrivileges(nsIMsgWindow *window)
     nsCOMPtr<nsIImapService> imapService = do_GetService(NS_IMAPSERVICE_CONTRACTID, &rv);
     NS_ENSURE_SUCCESS(rv,rv);
 
-    // selecting the folder with m_downloadingFolderForOfflineUse true will cause
-    // us to fetch any message bodies we don't have.
     rv = imapService->GetFolderAdminUrl(m_eventQueue, this, window, this, nsnull);
     if (NS_SUCCEEDED(rv))
       m_urlRunning = PR_TRUE;
@@ -3298,8 +3294,6 @@ NS_IMETHODIMP nsImapMailFolder::IssueCommandOnMsgs(const char *command, const ch
   nsCOMPtr<nsIImapService> imapService = do_GetService(NS_IMAPSERVICE_CONTRACTID, &rv);
   NS_ENSURE_SUCCESS(rv,rv);
 
-  // selecting the folder with m_downloadingFolderForOfflineUse true will cause
-  // us to fetch any message bodies we don't have.
   return imapService->IssueCommandOnMsgs(m_eventQueue, this, aWindow, command, uids, url);
 }
 
@@ -3309,8 +3303,6 @@ NS_IMETHODIMP nsImapMailFolder::FetchCustomMsgAttribute(const char *attribute, c
   nsCOMPtr<nsIImapService> imapService = do_GetService(NS_IMAPSERVICE_CONTRACTID, &rv);
   NS_ENSURE_SUCCESS(rv,rv);
 
-  // selecting the folder with m_downloadingFolderForOfflineUse true will cause
-  // us to fetch any message bodies we don't have.
   return imapService->FetchCustomMsgAttribute(m_eventQueue, this, aWindow, attribute, uids, url);
 }
 
@@ -3642,16 +3634,13 @@ NS_IMETHODIMP nsImapMailFolder::DownloadMessagesForOffline(nsISupportsArray *mes
 {
   nsCAutoString messageIds;
   nsMsgKeyArray srcKeyArray;
-#ifdef DEBUG_bienvenu
-//  return DownloadAllForOffline(nsnull, window);
-#endif
   nsresult rv = BuildIdsAndKeyArray(messages, messageIds, srcKeyArray);
   if (NS_FAILED(rv) || messageIds.IsEmpty()) return rv;
 
   nsCOMPtr<nsIImapService> imapService = do_GetService(NS_IMAPSERVICE_CONTRACTID, &rv);
   NS_ENSURE_SUCCESS(rv,rv);
 
-  SetNotifyDownloadedLines(PR_TRUE); // ### TODO need to clear this when we've finished
+  SetNotifyDownloadedLines(PR_TRUE); 
   return imapService->DownloadMessagesForOffline(messageIds.get(), this, nsnull, window);
 }
 
@@ -4263,7 +4252,7 @@ NS_IMETHODIMP nsImapMailFolder::OnStopRequest(nsIRequest *request, nsISupports *
 NS_IMETHODIMP
 nsImapMailFolder::OnStartRunningUrl(nsIURI *aUrl)
 {
-  NS_PRECONDITION(aUrl, "just a sanity check since this is a test program");
+  NS_PRECONDITION(aUrl, "sanity check - need to be be running non-null url");
   m_urlRunning = PR_TRUE;
   return NS_OK;
 }
@@ -4271,11 +4260,14 @@ nsImapMailFolder::OnStartRunningUrl(nsIURI *aUrl)
 NS_IMETHODIMP
 nsImapMailFolder::OnStopRunningUrl(nsIURI *aUrl, nsresult aExitCode)
 {
-  NS_PRECONDITION(aUrl, "just a sanity check since this is a test program");
   nsresult rv = NS_OK;
 
   m_urlRunning = PR_FALSE;
+  if (m_downloadingFolderForOfflineUse)
+  {
+    ReleaseSemaphore(NS_STATIC_CAST(nsIMsgImapMailFolder*, this));
     m_downloadingFolderForOfflineUse = PR_FALSE;
+  }
   nsCOMPtr<nsIMsgMailSession> session = 
            do_GetService(NS_MSGMAILSESSION_CONTRACTID, &rv);
   if (aUrl)
@@ -4869,8 +4861,12 @@ nsImapMailFolder::HeaderFetchCompleted(nsIImapProtocol* aProtocol)
       // for new messages, if the above filter playback actually moves the filtered
       // messages before we get to this code.
       if (autoDownloadNewHeaders)
+      {
+          // acquire semaphore for offline store. If it fails, we won't download for offline use.
+        if (NS_SUCCEEDED(AcquireSemaphore(NS_STATIC_CAST(nsIMsgImapMailFolder*, this))))
           m_downloadingFolderForOfflineUse = PR_TRUE;
       }
+    }
 
     if (m_downloadingFolderForOfflineUse)
     {
@@ -5661,8 +5657,12 @@ nsImapMailFolder::SetUrlState(nsIImapProtocol* aProtocol,
   {
     ProgressStatus(aProtocol, IMAP_DONE, nsnull);
     m_urlRunning = PR_FALSE;
+    if (m_downloadingFolderForOfflineUse)
+    {
+      ReleaseSemaphore(NS_STATIC_CAST(nsIMsgImapMailFolder*, this));
       m_downloadingFolderForOfflineUse = PR_FALSE;
     }
+  }
 
     if (aUrl)
         return aUrl->SetUrlState(isRunning, statusCode);