Bug 1174485 - do not try to compact folders that do not exist locally (only on IMAP server). Also allow compacting on platforms without working GetDiskSpaceAvailable(). r=rkent a=jorgk
authoraceman <acelists@atlas.sk>
Fri, 13 May 2016 07:23:19 +0200
changeset 24880 c238dd759028670d36dbf7250f42c292168e1463
parent 24879 83d52c8dc8a6f61833a649ba2dcf8d64cc5d4745
child 24881 b1304880a6006471071c02a7c130fd82c64a4b9c
push id1657
push userclokep@gmail.com
push dateMon, 06 Jun 2016 19:50:21 +0000
treeherdercomm-beta@9fac989284b5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrkent, jorgk
bugs1174485
Bug 1174485 - do not try to compact folders that do not exist locally (only on IMAP server). Also allow compacting on platforms without working GetDiskSpaceAvailable(). r=rkent a=jorgk
mailnews/base/src/nsMsgFolderCompactor.cpp
mailnews/local/src/nsMsgLocalStoreUtils.cpp
--- a/mailnews/base/src/nsMsgFolderCompactor.cpp
+++ b/mailnews/base/src/nsMsgFolderCompactor.cpp
@@ -190,109 +190,123 @@ nsFolderCompactState::Compact(nsIMsgFold
          m_folder = folder;  //will be used to compact
          m_parsingFolder = true;
          rv = localFolder->ParseFolder(m_window, this);
        }
        return rv;
      }
      else
      {
-       bool valid;  
-       rv = db->GetSummaryValid(&valid); 
+       bool valid;
+       rv = db->GetSummaryValid(&valid);
        if (!valid) //we are probably parsing the folder because we selected it.
        {
          folder->NotifyCompactCompleted();
          if (m_compactAll)
            return CompactNextFolder();
          else
            return NS_OK;
        }
      }
    }
    else
    {
      rv = folder->GetMsgDatabase(getter_AddRefs(db));
      NS_ENSURE_SUCCESS(rv, rv);
    }
 
-   rv = folder->GetFilePath(getter_AddRefs(path));
-   NS_ENSURE_SUCCESS(rv, rv);
+  rv = folder->GetFilePath(getter_AddRefs(path));
+  NS_ENSURE_SUCCESS(rv, rv);
 
-   int64_t expunged;
-   folder->GetExpungedBytes(&expunged);
+  do {
+    bool exists = false;
+    rv = path->Exists(&exists);
+    if (!exists) {
+      // No need to compact if the local file does not exist.
+      // Can happen e.g. on IMAP when the folder is not marked for offline use.
+      break;
+    }
 
-   bool abortCompactFolder = false;
-   int64_t diskSize;
-   rv = folder->GetSizeOnDisk(&diskSize);
-   NS_ENSURE_SUCCESS(rv, rv);
+    int64_t expunged = 0;
+    folder->GetExpungedBytes(&expunged);
+    if (expunged == 0) {
+      // No need to compact if nothing would be expunged.
+      break;
+    }
 
-   int64_t diskFree;
-   rv = path->GetDiskSpaceAvailable(&diskFree);
-   NS_ENSURE_SUCCESS(rv, rv);
+    int64_t diskSize;
+    rv = folder->GetSizeOnDisk(&diskSize);
+    NS_ENSURE_SUCCESS(rv, rv);
 
-   // Let's try to not even start compact if there is really low free space.
-   // It may still fail later as we do not know how big exactly the folder DB will
-   // end up being.
-   // The DB already doesn't contain references to messages that are already deleted.
-   // So theoretically it shouldn't shrink with compact. But in practice,
-   // the automatic shrinking of the DB may still have not yet happened.
-   // So we cap the final size at 1KB per message.
-   db->Commit(nsMsgDBCommitType::kCompressCommit);
-
-   int64_t dbSize;
-   rv = db->GetDatabaseSize(&dbSize);
-   NS_ENSURE_SUCCESS(rv, rv);
+    int64_t diskFree;
+    rv = path->GetDiskSpaceAvailable(&diskFree);
+    if (NS_FAILED(rv)) {
+      // If GetDiskSpaceAvailable() failed, better bail out fast.
+      if (rv != NS_ERROR_NOT_IMPLEMENTED)
+        return rv;
+      // Some platforms do not have GetDiskSpaceAvailable implemented.
+      // In that case skip the preventive free space analysis and let it
+      // fail in compact later if space actually wasn't available.
+    } else {
+      // Let's try to not even start compact if there is really low free space.
+      // It may still fail later as we do not know how big exactly the folder DB will
+      // end up being.
+      // The DB already doesn't contain references to messages that are already deleted.
+      // So theoretically it shouldn't shrink with compact. But in practice,
+      // the automatic shrinking of the DB may still have not yet happened.
+      // So we cap the final size at 1KB per message.
+      db->Commit(nsMsgDBCommitType::kCompressCommit);
 
-   int32_t totalMsgs;
-   rv = folder->GetTotalMessages(false, &totalMsgs);
-   NS_ENSURE_SUCCESS(rv, rv);
-   int64_t expectedDBSize = std::min<int64_t>(dbSize, totalMsgs * 1024);
-   if (diskFree < diskSize - expunged + expectedDBSize)
-   {
-     if (!m_alreadyWarnedDiskSpace)
-     {
-       folder->ThrowAlertMsg("compactFolderInsufficientSpace", m_window);
-       m_alreadyWarnedDiskSpace = true;
-     }
-     abortCompactFolder = true;
-   }
+      int64_t dbSize;
+      rv = db->GetDatabaseSize(&dbSize);
+      NS_ENSURE_SUCCESS(rv, rv);
 
-   if (!abortCompactFolder)
-   {
-     rv = folder->GetBaseMessageURI(baseMessageURI);
-     NS_ENSURE_SUCCESS(rv, rv);
+      int32_t totalMsgs;
+      rv = folder->GetTotalMessages(false, &totalMsgs);
+      NS_ENSURE_SUCCESS(rv, rv);
+      int64_t expectedDBSize = std::min<int64_t>(dbSize, ((int64_t)totalMsgs) * 1024);
+      if (diskFree < diskSize - expunged + expectedDBSize)
+      {
+        if (!m_alreadyWarnedDiskSpace)
+        {
+          folder->ThrowAlertMsg("compactFolderInsufficientSpace", m_window);
+          m_alreadyWarnedDiskSpace = true;
+        }
+        break;
+      }
+    }
 
-     rv = Init(folder, baseMessageURI.get(), db, path, m_window);
-     NS_ENSURE_SUCCESS(rv, rv);
+    rv = folder->GetBaseMessageURI(baseMessageURI);
+    NS_ENSURE_SUCCESS(rv, rv);
 
-     bool isLocked;
-     m_folder->GetLocked(&isLocked);
-     if (isLocked)
-     {
-       m_folder->NotifyCompactCompleted();
-       CleanupTempFilesAfterError();
-       m_folder->ThrowAlertMsg("compactFolderDeniedLock", m_window);
-       abortCompactFolder = true;
-     }
-   }
+    rv = Init(folder, baseMessageURI.get(), db, path, m_window);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    bool isLocked = true;
+    m_folder->GetLocked(&isLocked);
+    if (isLocked)
+    {
+      CleanupTempFilesAfterError();
+      m_folder->ThrowAlertMsg("compactFolderDeniedLock", m_window);
+      break;
+    }
 
-   if (!abortCompactFolder)
-   {
-     nsCOMPtr <nsISupports> supports = do_QueryInterface(static_cast<nsIMsgFolderCompactor*>(this));
-     m_folder->AcquireSemaphore(supports);
-     m_totalExpungedBytes += expunged;
-     return StartCompacting();
-   }
-   else
-   {
-     if (m_compactAll)
-       return CompactNextFolder();
-     else
-       return NS_OK;
-   }
+    // If we got here start the real compacting.
+    nsCOMPtr<nsISupports> supports = do_QueryInterface(static_cast<nsIMsgFolderCompactor*>(this));
+    m_folder->AcquireSemaphore(supports);
+    m_totalExpungedBytes += expunged;
+    return StartCompacting();
+
+  } while(false); // block for easy skipping the compaction using 'break'
+
+  folder->NotifyCompactCompleted();
+  if (m_compactAll)
+    return CompactNextFolder();
+  else
+    return NS_OK;
 }
 
 nsresult nsFolderCompactState::ShowStatusMsg(const nsString& aMsg)
 {
   nsCOMPtr <nsIMsgStatusFeedback> statusFeedback;
   if (m_window)
   {
     m_window->GetStatusFeedback(getter_AddRefs(statusFeedback));
--- a/mailnews/local/src/nsMsgLocalStoreUtils.cpp
+++ b/mailnews/local/src/nsMsgLocalStoreUtils.cpp
@@ -305,27 +305,30 @@ nsMsgLocalStoreUtils::DiskSpaceAvailable
     printf("GetDiskSpaceAvailable returned: %lld bytes\n", diskFree);
 #endif
     // When checking for disk space available, take into consideration
     // possible database changes, therefore ask for a little more
     // (EXTRA_SAFETY_SPACE) than what the requested size is. Also, due to disk
     // sector sizes, allocation blocks, etc. The space "available" may be greater
     // than the actual space usable.
     return ((aSpaceRequested + EXTRA_SAFETY_SPACE) < (uint64_t) diskFree);
-  } else {
-    // The call to GetDiskSpaceAvailable FAILED!
+  } else if (rv == NS_ERROR_NOT_IMPLEMENTED) {
+    // The call to GetDiskSpaceAvailable is not implemented!
     // This will happen on certain platforms where GetDiskSpaceAvailable
     // is not implemented. Since people on those platforms still need
     // to download mail, we will simply bypass the disk-space check.
     //
     // We'll leave a debug message to warn people.
 #ifdef DEBUG
-    printf("Call to GetDiskSpaceAvailable FAILED! \n");
+    printf("Call to GetDiskSpaceAvailable FAILED because it is not implemented!\n");
 #endif
     return true;
+  } else {
+    printf("Call to GetDiskSpaceAvailable FAILED!\n");
+    return false;
   }
 }
 
 /**
  * Resets forceReparse in the database.
  *
  * @param aMsgDb The database to reset.
  */