Bug 457342 - "Handle group element count == 0 case in nsAutoSyncManager::DownloadMessagesForOffline" [r=bienvenu,sr=Standard8,a=blocking TB 3b1]
authorEmre Birol <bugmil.ebirol@gmail.com>
Sat, 29 Nov 2008 10:01:29 +0000
changeset 1253 00858c123ca48a62ca80a462c93443d7f4a83492
parent 1252 1c26ea5bb33da1a71e6b9ce12990889db45e9124
child 1254 439ef9103f0e9d1c8b4559519f5a1cd03324303e
push idunknown
push userunknown
push dateunknown
reviewersbienvenu, Standard8, blocking
bugs457342
Bug 457342 - "Handle group element count == 0 case in nsAutoSyncManager::DownloadMessagesForOffline" [r=bienvenu,sr=Standard8,a=blocking TB 3b1]
mailnews/imap/src/nsAutoSyncManager.cpp
mailnews/imap/src/nsAutoSyncManager.h
mailnews/imap/src/nsAutoSyncState.h
--- a/mailnews/imap/src/nsAutoSyncManager.cpp
+++ b/mailnews/imap/src/nsAutoSyncManager.cpp
@@ -521,39 +521,73 @@ NS_IMETHODIMP nsAutoSyncManager::Observe
   nsCOMArray<nsIAutoSyncState> chainedQ;
   nsCOMArray<nsIAutoSyncState> *queue = &mPriorityQ;
   if (mDownloadModel == dmChained) 
   {
     ChainFoldersInQ(mPriorityQ, chainedQ);
     queue = &chainedQ;
   }
   
+  // to store the folders that should be removed from the priority
+  // queue at the end of the iteration.
+  nsCOMArray<nsIAutoSyncState> foldersToBeRemoved;
+  
   // process folders in the priority queue 
   PRInt32 elemCount = queue->Count();
   for (PRInt32 idx = 0; idx < elemCount; idx++)
   {
     nsCOMPtr<nsIAutoSyncState> autoSyncStateObj((*queue)[idx]);
     if (!autoSyncStateObj)
       continue;
     
     PRInt32 state;
     autoSyncStateObj->GetState(&state);
     
     //TODO: Test cached-connection availability in parallel mode
     // and do not exceed (cached-connection count - 1)
     
     if (state != nsAutoSyncState::stReadyToDownload)
       continue;
-    else
+    
+    nsresult rv = DownloadMessagesForOffline(autoSyncStateObj);
+    if (NS_FAILED(rv))
     {
-      if (NS_FAILED(DownloadMessagesForOffline(autoSyncStateObj)))
-        HandleDownloadErrorFor(autoSyncStateObj);
-    }
+      // special case: this folder does not have any message to download
+      // (see bug 457342), remove it explicitly from the queue when iteration
+      // is over.
+      // Note that in normal execution flow, folders are removed from priority
+      // queue only in OnDownloadCompleted when all messages are downloaded
+      // successfully. This is the only place we change this flow.
+      if (NS_ERROR_NOT_AVAILABLE == rv)
+        foldersToBeRemoved.AppendObject(autoSyncStateObj);
+      
+      HandleDownloadErrorFor(autoSyncStateObj, rv);
+    }// endif
   }//endfor
   
+  // remove folders with no pending messages from the priority queue
+  elemCount = foldersToBeRemoved.Count();
+  for (PRInt32 idx = 0; idx < elemCount; idx++)
+  {
+    nsCOMPtr<nsIAutoSyncState> autoSyncStateObj(foldersToBeRemoved[idx]);
+    if (!autoSyncStateObj)
+      continue;
+    
+    nsCOMPtr<nsIMsgFolder> folder;
+    autoSyncStateObj->GetOwnerFolder(getter_AddRefs(folder));
+    if (folder)
+      NOTIFY_LISTENERS(OnDownloadCompleted, (folder));
+
+    autoSyncStateObj->SetState(nsAutoSyncState::stCompletedIdle);
+
+    if (mPriorityQ.RemoveObject(autoSyncStateObj))
+      NOTIFY_LISTENERS(OnFolderRemovedFromQ,
+                      (nsIAutoSyncMgrListener::PriorityQueue, folder));
+  }
+    
   return AutoUpdateFolders();
 }
 
 /**
  * Updates offline imap folders that are not synchronized recently.
  */
 nsresult nsAutoSyncManager::AutoUpdateFolders()
 {
@@ -768,28 +802,39 @@ void nsAutoSyncManager::ScheduleFolderFo
  */
 nsresult nsAutoSyncManager::DownloadMessagesForOffline(nsIAutoSyncState *aAutoSyncStateObj, PRUint32 aSizeLimit)
 {  
   if (!aAutoSyncStateObj)
     return NS_ERROR_INVALID_ARG;
   
   PRInt32 count;
   nsresult rv = aAutoSyncStateObj->GetPendingMessageCount(&count);
-  // TODO: right thing to do here is to return an error to the caller saying that do not
-  // try again. Not sure how to do it using nserror mechanism.
-  // Note that we can't return success in case of 0 == count here since
-  // we only remove the object from the queue in the OnDownloadCompleted method
-  if (NS_FAILED(rv) || !count)
-    return NS_ERROR_FAILURE; 
+  NS_ENSURE_SUCCESS(rv, rv);
+  
+  // special case: no more message to download for this folder:
+  // see HandleDownloadErrorFor for recovery policy
+  if (!count)
+    return NS_ERROR_NOT_AVAILABLE;
  
   nsCOMPtr<nsIMutableArray> messagesToDownload;
   PRUint32 totalSize = 0;
   rv = aAutoSyncStateObj->GetNextGroupOfMessages(mGroupSize, &totalSize, getter_AddRefs(messagesToDownload));
   NS_ENSURE_SUCCESS(rv,rv);
   
+  // there are pending messages but the cumulative size is zero:
+  // treat as special case.
+  // Note that although it shouldn't happen, we know that sometimes
+  // imap servers manifest messages as zero length. By returning
+  // NS_ERROR_NOT_AVAILABLE we cause this folder to be removed from
+  // the priority queue temporarily (until the next idle or next update)
+  // in an effort to prevent it blocking other folders of the same account
+  // being synced.
+  if (!totalSize)
+    return NS_ERROR_NOT_AVAILABLE;
+  
   // ensure that we don't exceed the given size limit for this particular group
   if (aSizeLimit && aSizeLimit < totalSize)
     return NS_ERROR_FAILURE;
   
   PRUint32 length;
   rv = messagesToDownload->GetLength(&length);
   if (NS_SUCCEEDED(rv) && length > 0)
   {
@@ -806,44 +851,63 @@ nsresult nsAutoSyncManager::DownloadMess
 
 /**
  * Assuming that the download operation on the given folder has been failed at least once, 
  * execute these steps:
  *  - put the auto-sync state into ready-to-download mode
  *  - rollback the message offset so we can try the same group again (unless the retry
  *     count is reached to the given limit)
  *  - if parallel model is active, wait to be resumed by the next idle
- *  - if chained model is active, searche the priority queue to find a sibling to continue with.
+ *  - if chained model is active, search the priority queue to find a sibling to continue 
+ *    with.
  */
-nsresult nsAutoSyncManager::HandleDownloadErrorFor(nsIAutoSyncState *aAutoSyncStateObj)
+nsresult nsAutoSyncManager::HandleDownloadErrorFor(nsIAutoSyncState *aAutoSyncStateObj,
+                                                   const nsresult error)
 {
   if (!aAutoSyncStateObj)
     return NS_ERROR_INVALID_ARG;
   
-  // force the auto-sync state to try downloading the same group at least
-  // kGroupRetryCount times before it moves to the next one
-  aAutoSyncStateObj->TryCurrentGroupAgain(kGroupRetryCount);
-  
-  nsCOMPtr<nsIMsgFolder> folder;
-  aAutoSyncStateObj->GetOwnerFolder(getter_AddRefs(folder));
-  if (folder)
-    NOTIFY_LISTENERS(OnDownloadError, (folder));
+  // ensure that an error occured
+  if (NS_SUCCEEDED(error))
+    return NS_OK;
+    
+  // NS_ERROR_NOT_AVAILABLE is a special case/error happens when the queued folder
+  // doesn't have any message to download (see bug 457342). In such case we shouldn't
+  // retry the current message group, nor notify listeners. Simply continuing with the
+  // next sibling in the priority queue would suffice.
+    
+  if (NS_ERROR_NOT_AVAILABLE != error)
+  {
+    // force the auto-sync state to try downloading the same group at least
+    // kGroupRetryCount times before it moves to the next one
+    aAutoSyncStateObj->TryCurrentGroupAgain(kGroupRetryCount);
+    
+    nsCOMPtr<nsIMsgFolder> folder;
+    aAutoSyncStateObj->GetOwnerFolder(getter_AddRefs(folder));
+    if (folder)
+      NOTIFY_LISTENERS(OnDownloadError, (folder));
+  }
   
   // if parallel model, don't do anything else
   
   if (mDownloadModel == dmChained)
   {
     // switch to the next folder in the chain and continue downloading
     nsIAutoSyncState *autoSyncStateObj = aAutoSyncStateObj;
     nsIAutoSyncState *nextAutoSyncStateObj = nsnull;
     while ( (nextAutoSyncStateObj = GetNextSibling(mPriorityQ, autoSyncStateObj)) )
     {
       autoSyncStateObj = nextAutoSyncStateObj;
-      if (NS_SUCCEEDED(DownloadMessagesForOffline(autoSyncStateObj)))
+      nsresult rv = DownloadMessagesForOffline(autoSyncStateObj);
+      if (NS_SUCCEEDED(rv))
         break;
+      else if (rv == NS_ERROR_NOT_AVAILABLE)
+        // next folder in the chain also doesn't have any message to download
+        // switch to next one if any
+        continue;
       else
         autoSyncStateObj->TryCurrentGroupAgain(kGroupRetryCount);
     }
   }
   
   return NS_OK;
 }
 
@@ -1030,17 +1094,17 @@ nsAutoSyncManager::OnDownloadCompleted(n
   {
     // retry the same group kGroupRetryCount times
     // try again if TB still idle, otherwise wait for the next idle time
     autoSyncStateObj->TryCurrentGroupAgain(kGroupRetryCount);
     if (GetIdleState() == idle)
     {
       rv = DownloadMessagesForOffline(autoSyncStateObj);
       if (NS_FAILED(rv))
-        rv = HandleDownloadErrorFor(autoSyncStateObj);
+        rv = HandleDownloadErrorFor(autoSyncStateObj, rv);
     }
     return rv;
   }
       
   // download is successful, reset the retry counter of the folder
   autoSyncStateObj->ResetRetryCounter();
   
   nsCOMPtr<nsIMsgFolder> folder;
@@ -1079,32 +1143,31 @@ nsAutoSyncManager::OnDownloadCompleted(n
   }
   else 
   {
     autoSyncStateObj->SetState(nsAutoSyncState::stCompletedIdle);
     
     nsCOMPtr<nsIMsgFolder> folder;
     nsresult rv = autoSyncStateObj->GetOwnerFolder(getter_AddRefs(folder));
     
-    mPriorityQ.RemoveObject(autoSyncStateObj);
-    if (NS_SUCCEEDED(rv))
+    if (NS_SUCCEEDED(rv) && mPriorityQ.RemoveObject(autoSyncStateObj))
       NOTIFY_LISTENERS(OnFolderRemovedFromQ, (nsIAutoSyncMgrListener::PriorityQueue, folder));
 
     //find the next folder owned by the same server in the queue and continue downloading
     if (mDownloadModel == dmChained)
       nextFolderToDownload = GetHighestPrioSibling(mPriorityQ, autoSyncStateObj);
       
   }//endif
   
   // continue downloading if TB is still in idle state
   if (nextFolderToDownload && GetIdleState() == idle)
   {
     rv = DownloadMessagesForOffline(nextFolderToDownload);
     if (NS_FAILED(rv))
-      rv = HandleDownloadErrorFor(nextFolderToDownload);
+      rv = HandleDownloadErrorFor(nextFolderToDownload, rv);
   }
 
   return rv;
 }
 
 NS_IMETHODIMP nsAutoSyncManager::GetDownloadModel(PRInt32 *aDownloadModel)
 {
   NS_ENSURE_ARG_POINTER(aDownloadModel);
--- a/mailnews/imap/src/nsAutoSyncManager.h
+++ b/mailnews/imap/src/nsAutoSyncManager.h
@@ -48,16 +48,69 @@
 #include "nsIAutoSyncMsgStrategy.h"
 #include "nsIAutoSyncFolderStrategy.h"
 
 class nsImapMailFolder;
 class nsIMsgDBHdr;
 class nsIIdleService;
 class nsIMsgFolder;
 
+/* Auto-Sync
+ *
+ * Background:
+ *  it works only with offline imap folders. "autosync_offline_stores" pref
+ *  enables/disables auto-sync mechanism. Note that setting "autosync_offline_stores"
+ *  to false, or setting folder to not-offline doesn't stop synchronization
+ *  process for already queued folders.
+ *
+ * Auto-Sync policy:
+ *  o It kicks in during system idle time, and tries to download as much messages
+ *    as possible based on given folder and message prioritization strategies/rules.
+ *    Default folder prioritization strategy dictates to sort the folders based on the
+ *    following order:  INBOX > DRAFTS > SUBFOLDERS > TRASH.
+ *    Similarly, default message prioritization strategy dictates to download the most
+ *    recent and smallest message first. Also, by sorting the messages by size in the 
+ *    queue, it tries to maximize the number of messages downloaded.
+ *  o It downloads the messages in groups. Default groups size is defined by |kDefaultGroupSize|. 
+ *  o It downloads the messages larger than the group size one-by-one.
+ *  o If new messages arrive when not idle, it downloads the messages that do fit into
+ *    |kFirstGroupSizeLimit| size limit immediately, without waiting for idle time.
+ *  o If new messages arrive when idle, it downloads all the messages without any restriction.
+ *  o If new messages arrive into a folder while auto-sync is downloading other messages of the
+ *    same folder, it simply puts the new messages into the folder's download queue, and
+ *    re-prioritize the messages. That behavior makes sure that the high priority
+ *    (defined by the message strategy) get downloaded first always.
+ *  o If new messages arrive into a folder while auto-sync is downloading messages of a lower
+ *    priority folder, auto-sync switches the folders in the queue and starts downloading the
+ *    messages of the higher priority folder next time it downloads a message group.
+ *  o Currently there is no way to stop/pause/cancel a message download. The smallest
+ *    granularity is the message group size.
+ *  o Auto-Sync manager periodically (kAutoSyncFreq) checks folder for existing messages
+ *    w/o bodies. It persists the last time the folder is checked in the local database of the
+ *    folder. We call this process 'Discovery'. This process is asynchronous and processes
+ *    |kNumberOfHeadersToProcess| number of headers at each cycle. Since it works on local data,
+ *    it doesn't consume lots of system resources, it does its job fast.
+ *  o Discovery is necessary especially when the user makes a transition from not-offline 
+ *    to offline mode.
+ *  o Update frequency is defined by nsMsgIncomingServer::BiffMinutes.
+ *
+ * Error Handling:
+ *  o if the user moves/deletes/filters all messages of a folder already queued, auto-sync
+ *    deals with that situation by skipping the folder in question, and continuing with the
+ *    next in chain.
+ *  o If the message size is zero, auto-sync ignores the message.
+ *  o If the download of the message group fails for some reason, auto-sync tries to
+ *    download the same group |kGroupRetryCount| times. If it still fails, continues with the
+ *    next group of messages.
+ *
+ * Download Model:
+ *  Parallel model should be used with the imap servers that do not have any session-limit-per-IP
+ *  limit, and when the bandwidth is significantly large.
+ */
+ 
 /**
  * Default strategy implementation to prioritize messages in the download queue.   
  */
 class nsDefaultAutoSyncMsgStrategy : public nsIAutoSyncMsgStrategy
 {
   static const PRUint32 kFirstPassMessageSize = 60U*1024U; // 60K
   
   public:
@@ -119,17 +172,17 @@ class nsAutoSyncManager : public nsIObse
   private:
     ~nsAutoSyncManager();
 
     void SetIdleState(IdleState st);    
     IdleState GetIdleState() const;
     nsresult AutoUpdateFolders(); 
     void ScheduleFolderForOfflineDownload(nsIAutoSyncState *aAutoSyncStateObj);
     nsresult DownloadMessagesForOffline(nsIAutoSyncState *aAutoSyncStateObj, PRUint32 aSizeLimit = 0);
-    nsresult HandleDownloadErrorFor(nsIAutoSyncState *aAutoSyncStateObj);
+    nsresult HandleDownloadErrorFor(nsIAutoSyncState *aAutoSyncStateObj, const nsresult error);
     
     // Helper methods for priority Q operations
     static
     void ChainFoldersInQ(const nsCOMArray<nsIAutoSyncState> &aQueue, 
                           nsCOMArray<nsIAutoSyncState> &aChainedQ);
     static
     nsIAutoSyncState* SearchQForSibling(const nsCOMArray<nsIAutoSyncState> &aQueue, 
                           nsIAutoSyncState *aAutoSyncStateObj, PRInt32 aStartIdx, PRInt32 *aIndex = nsnull);
@@ -186,17 +239,17 @@ change in this queue:
 
 1) nsImapMailFolder::HeaderFetchCompleted: is triggered when TB notices that
 there are pending messages on the server -- via IDLE command from the server, 
 via explicit select from the user, or via automatic Update during idle time. If 
 it turns out that there are pending messages on the server, it adds them into 
 nsAutoSyncState's download queue.
 
 2) nsAutoSyncState::ProcessExistingHeaders: is triggered for every imap folder 
-every hour or so. nsAutoSyncManager uses an internal queue called Discovery 
+every hour or so (see kAutoSyncFreq). nsAutoSyncManager uses an internal queue called Discovery 
 queue to keep track of this task. The purpose of ProcessExistingHeaders() 
 method is to check existing headers of a given folder in batches and discover 
 the messages without bodies, in asynchronous fashion. This process is 
 sequential, one and only one folder at any given time, very similar to 
 indexing. Again, if it turns out that the folder in hand has messages w/o 
 bodies, ProcessExistingHeaders adds them into nsAutoSyncState's download queue.
 
 Any change in nsAutoSyncState's download queue, notifies nsAutoSyncManager and 
@@ -216,12 +269,12 @@ time a batch completed for an imap serve
 So, lets say that updating a sub-folder starts downloading message immediately,
 when an higher priority folder is added into the queue, nsAutoSyncManager
 switches to this higher priority folder instead of processing the next group of
 messages of the lower priority one. Setting group size too high might delay
 this switch at worst. 
 
 And finally, Update queue helps nsAutoSyncManager to keep track of folders 
 waiting to be updated. With the latest change, we update one and only one
-folder at any given time. Frequency of updating is 5 min. We add folders into
-the update queue during idle time, if they are not in mPriorityQ already.
+folder at any given time. Default frequency of updating is 10 min (kDefaultUpdateInterval). 
+We add folders into the update queue during idle time, if they are not in mPriorityQ already.
 
 */
--- a/mailnews/imap/src/nsAutoSyncState.h
+++ b/mailnews/imap/src/nsAutoSyncState.h
@@ -74,19 +74,17 @@ class MsgStrategyComparatorAdaptor
 };
 
 
 /**
  * Facilitates auto-sync capabilities for imap folders.
  */
 class nsAutoSyncState : public nsIAutoSyncState, public nsIUrlListener
 {
-  static const PRInt32 kFirstPassMsgKeyCount = 500;
-  
- public: 
+ public:
   NS_DECL_ISUPPORTS
   NS_DECL_NSIAUTOSYNCSTATE
   NS_DECL_NSIURLLISTENER
   
   nsAutoSyncState(nsImapMailFolder *aOwnerFolder, PRTime aLastSyncTime = 0UL);
   
   /// Called by owner folder when new headers are fetched form the server
   nsresult OnNewHeaderFetchCompleted(const nsTArray<nsMsgKey> &aMsgKeyList);