Bug 1428097 - With CONDSTORE, eliminate unneeded flag fetches at startup. r=jorgk
authorGene Smith <gds@chartertn.net>
Mon, 10 Sep 2018 17:01:12 -0400
changeset 33295 028f53e3c744d5f31e5892cf205a427d0044827d
parent 33294 c42d9a24749dc13ff6ccfbd3780828613cd8faf0
child 33296 c2608a685d0082508f8a32c8f3c4f14737285214
push id387
push userclokep@gmail.com
push dateMon, 10 Dec 2018 21:30:47 +0000
reviewersjorgk
bugs1428097
Bug 1428097 - With CONDSTORE, eliminate unneeded flag fetches at startup. r=jorgk
mailnews/imap/src/nsImapFlagAndUidState.cpp
mailnews/imap/src/nsImapFlagAndUidState.h
mailnews/imap/src/nsImapMailFolder.cpp
mailnews/imap/src/nsImapProtocol.cpp
mailnews/imap/src/nsImapServerResponseParser.cpp
mailnews/imap/src/nsImapServerResponseParser.h
--- a/mailnews/imap/src/nsImapFlagAndUidState.cpp
+++ b/mailnews/imap/src/nsImapFlagAndUidState.cpp
@@ -82,16 +82,18 @@ nsImapFlagAndUidState::nsImapFlagAndUidS
     fFlags(numberOfMessages),
     m_customFlagsHash(10),
     m_customAttributesHash(10),
     mLock("nsImapFlagAndUidState.mLock")
 {
   fSupportedUserFlags = 0;
   fNumberDeleted = 0;
   fPartialUIDFetch = true;
+  fStartCapture = false;
+  fNumAdded = 0;
 }
 
 nsImapFlagAndUidState::~nsImapFlagAndUidState()
 {
 }
 
 NS_IMETHODIMP
 nsImapFlagAndUidState::OrSupportedUserFlags(uint16_t flags)
@@ -114,16 +116,18 @@ nsImapFlagAndUidState::GetSupportedUserF
 NS_IMETHODIMP nsImapFlagAndUidState::Reset()
 {
   PR_CEnterMonitor(this);
   fNumberDeleted = 0;
   m_customFlagsHash.Clear();
   fUids.Clear();
   fFlags.Clear();
   fPartialUIDFetch = true;
+  fStartCapture = false;
+  fNumAdded = 0;
   PR_CExitMonitor(this);
   return NS_OK;
 }
 
 
 // Remove (expunge) a message from our array, since now it is gone for good
 
 NS_IMETHODIMP nsImapFlagAndUidState::ExpungeByIndex(uint32_t msgIndex)
@@ -156,16 +160,24 @@ NS_IMETHODIMP nsImapFlagAndUidState::Add
     return NS_ERROR_INVALID_ARG;
   PR_CEnterMonitor(this);
   // make sure there is room for this pair
   if (zeroBasedIndex >= fUids.Length())
   {
     int32_t sizeToGrowBy = zeroBasedIndex - fUids.Length() + 1;
     fUids.InsertElementsAt(fUids.Length(), sizeToGrowBy, 0);
     fFlags.InsertElementsAt(fFlags.Length(), sizeToGrowBy, 0);
+    if (fStartCapture)
+    {
+      // A new partial (CONDSTORE/CHANGEDSINCE) fetch response is occurring
+      // so need to start the count of number of uid/flag combos added.
+      fNumAdded = 0;
+      fStartCapture = false;
+    }
+    fNumAdded++;
   }
 
   fUids[zeroBasedIndex] = uid;
   fFlags[zeroBasedIndex] = flags;
   if (flags & kImapMsgDeletedFlag)
     fNumberDeleted++;
   PR_CExitMonitor(this);
   return NS_OK;
--- a/mailnews/imap/src/nsImapFlagAndUidState.h
+++ b/mailnews/imap/src/nsImapFlagAndUidState.h
@@ -29,25 +29,29 @@ public:
 
     imapMessageFlagsType  GetMessageFlagsFromUID(uint32_t uid, bool *foundIt, int32_t *ndx);
 
     bool         IsLastMessageUnseen(void);
     bool         GetPartialUIDFetch() {return fPartialUIDFetch;}
     void         SetPartialUIDFetch(bool isPartial) {fPartialUIDFetch = isPartial;}
     uint32_t     GetHighestNonDeletedUID();
     uint16_t     GetSupportedUserFlags() { return fSupportedUserFlags; }
+    void         StartCapture() { fStartCapture = true; }
+    uint32_t     GetNumAdded() { return fNumAdded; }
 
 private:
   virtual ~nsImapFlagAndUidState();
 
     nsTArray<nsMsgKey>      fUids;
     nsTArray<imapMessageFlagsType> fFlags;
     // Hash table, mapping uids to extra flags
     nsDataHashtable<nsUint32HashKey, nsCString> m_customFlagsHash;
     // Hash table, mapping UID+customAttributeName to customAttributeValue.
     nsDataHashtable<nsCStringHashKey, nsCString> m_customAttributesHash;
     uint16_t                fSupportedUserFlags;
     int32_t                 fNumberDeleted;
     bool                    fPartialUIDFetch;
+    uint32_t                fNumAdded;
+    bool                    fStartCapture;
     mozilla::Mutex mLock;
 };
 
 #endif
--- a/mailnews/imap/src/nsImapMailFolder.cpp
+++ b/mailnews/imap/src/nsImapMailFolder.cpp
@@ -92,16 +92,17 @@
 #include "nsIStreamListener.h"
 
 static NS_DEFINE_CID(kRDFServiceCID, NS_RDFSERVICE_CID);
 static NS_DEFINE_CID(kParseMailMsgStateCID, NS_PARSEMAILMSGSTATE_CID);
 static NS_DEFINE_CID(kCImapHostSessionList, NS_IIMAPHOSTSESSIONLIST_CID);
 
 extern mozilla::LazyLogModule gAutoSyncLog; // defined in nsAutoSyncManager.cpp
 extern mozilla::LazyLogModule IMAP;         // defined in nsImapProtocol.cpp
+extern mozilla::LazyLogModule IMAP_CS;      // For CONDSTORE, defined in nsImapProtocol.cpp
 
 #define MAILNEWS_CUSTOM_HEADERS "mailnews.customHeaders"
 
 /*
     Copies the contents of srcDir into destDir.
     destDir will be created if it doesn't exist.
 */
 
@@ -2644,16 +2645,19 @@ NS_IMETHODIMP nsImapMailFolder::UpdateIm
   if (mDatabase)
   {
     rv = mDatabase->GetDBFolderInfo(getter_AddRefs(dbFolderInfo));
     if (NS_SUCCEEDED(rv) && dbFolderInfo)
     {
       dbFolderInfo->GetImapUidValidity(&imapUIDValidity);
       uint64_t mailboxHighestModSeq;
       aSpec->GetHighestModSeq(&mailboxHighestModSeq);
+      MOZ_LOG(IMAP_CS, mozilla::LogLevel::Debug,
+              ("UpdateImapMailboxInfo(): Store highest MODSEQ=%" PRIu64 " for folder=%s",
+               mailboxHighestModSeq, m_onlineFolderName.get()));
       char intStrBuf[40];
       PR_snprintf(intStrBuf, sizeof(intStrBuf), "%llu",  mailboxHighestModSeq);
       dbFolderInfo->SetCharProperty(kModSeqPropertyName, nsDependentCString(intStrBuf));
     }
     RefPtr<nsMsgKeyArray> keys = new nsMsgKeyArray;
     if (!keys)
       return NS_ERROR_OUT_OF_MEMORY;
     rv = mDatabase->ListAllKeys(keys);
@@ -2738,16 +2742,19 @@ NS_IMETHODIMP nsImapMailFolder::UpdateIm
       }
     }
     // store the new UIDVALIDITY value
 
     if (NS_SUCCEEDED(rv) && dbFolderInfo)
     {
       dbFolderInfo->SetImapUidValidity(folderValidity);
       // need to forget highest mod seq when uid validity rolls.
+      MOZ_LOG(IMAP_CS, mozilla::LogLevel::Debug,
+              ("UpdateImapMailboxInfo(): UIDVALIDITY changed, reset highest MODSEQ and UID for folder=%s",
+               m_onlineFolderName.get()));
       dbFolderInfo->SetCharProperty(kModSeqPropertyName, EmptyCString());
       dbFolderInfo->SetUint32Property(kHighestRecordedUIDPropertyName, 0);
     }
     // delete all my msgs, the keys are bogus now
     // add every message in this folder
     existingKeys.Clear();
     //      keysToDelete.CopyArray(&existingKeys);
 
@@ -3167,17 +3174,22 @@ nsresult nsImapMailFolder::NormalEndHead
       notifier->NotifyMsgAdded(newMsgHdr);
     // mark the header as not yet reported classified
     OrProcessingFlags(m_curMsgUid, nsMsgProcessingFlags::NotReportedClassified);
   }
   // adjust highestRecordedUID
   if (dbFolderInfo)
   {
     if (m_curMsgUid > highestUID)
+    {
+      MOZ_LOG(IMAP_CS, mozilla::LogLevel::Debug,
+              ("NormalEndHeaderParseStream(): Store new highest UID=%" PRIu32 " for folder=%s",
+               m_curMsgUid, m_onlineFolderName.get()));
       dbFolderInfo->SetUint32Property(kHighestRecordedUIDPropertyName, m_curMsgUid);
+    }
   }
 
   if (m_isGmailServer)
   {
     nsCOMPtr<nsIImapFlagAndUidState> flagState;
     aProtocol->GetFlagAndUidState(getter_AddRefs(flagState));
     nsCString msgIDValue;
     nsCString threadIDValue;
@@ -4928,16 +4940,19 @@ nsImapMailFolder::NotifyMessageFlags(uin
       nsCOMPtr <nsIDBFolderInfo> dbFolderInfo;
       mDatabase->GetDBFolderInfo(getter_AddRefs(dbFolderInfo));
       if (dbFolderInfo)
       {
         if (aHighestModSeq)
         {
           char intStrBuf[40];
           PR_snprintf(intStrBuf, sizeof(intStrBuf), "%llu",  aHighestModSeq);
+          MOZ_LOG(IMAP_CS, mozilla::LogLevel::Debug,
+                  ("NotifyMessageFlags(): Store highest MODSEQ=%" PRIu64 " for folder=%s",
+                   aHighestModSeq, m_onlineFolderName.get()));
           dbFolderInfo->SetCharProperty(kModSeqPropertyName, nsDependentCString(intStrBuf));
         }
         if (msgDeleted)
         {
           uint32_t oldDeletedCount;
           dbFolderInfo->GetUint32Property(kDeletedHdrCountPropertyName, 0, &oldDeletedCount);
           dbFolderInfo->SetUint32Property(kDeletedHdrCountPropertyName, oldDeletedCount + 1);
         }
--- a/mailnews/imap/src/nsImapProtocol.cpp
+++ b/mailnews/imap/src/nsImapProtocol.cpp
@@ -75,16 +75,17 @@
 // imap event sinks
 #include "nsIImapMailFolderSink.h"
 #include "nsIImapServerSink.h"
 #include "nsIImapMessageSink.h"
 
 using namespace mozilla;
 
 LazyLogModule IMAP("IMAP");
+LazyLogModule IMAP_CS("IMAP_CS");
 
 #define ONE_SECOND ((uint32_t)1000)    // one second
 
 #define OUTPUT_BUFFER_SIZE (4096*2) // mscott - i should be able to remove this if I can use nsMsgLineBuffer???
 
 #define IMAP_ENV_HEADERS "From To Cc Bcc Subject Date Message-ID "
 #define IMAP_DB_HEADERS "Priority X-Priority References Newsgroups In-Reply-To Content-Type Reply-To"
 #define IMAP_ENV_AND_DB_HEADERS IMAP_ENV_HEADERS IMAP_DB_HEADERS
@@ -4129,56 +4130,169 @@ void nsImapProtocol::ProcessMailboxUpdat
 
     // make the parser record these flags
     nsCString fetchStr;
     int32_t added = 0, deleted = 0;
 
     m_flagState->GetNumberOfMessages(&added);
     deleted = m_flagState->NumberOfDeletedMessages();
     bool flagStateEmpty = !added;
-    // Figure out if we need to do any kind of sync.
-    bool needFolderSync = (flagStateEmpty || added == deleted) && (!UseCondStore() ||
-      (GetServerStateParser().fHighestModSeq != mFolderLastModSeq) ||
-      (GetShowDeletedMessages() &&
-         GetServerStateParser().NumberOfMessages() != mFolderTotalMsgCount));
+    bool useCS = UseCondStore();
 
     // Figure out if we need to do a full sync (UID Fetch Flags 1:*),
     // a partial sync using CHANGEDSINCE, or a sync from the previous
     // highwater mark.
 
-    // if the folder doesn't know about the highest uid, or the flag state
-    // is empty, and we're not using CondStore, we need a full sync.
-    bool needFullFolderSync = !mFolderHighestUID || (flagStateEmpty && !UseCondStore());
+    // If the folder doesn't know about the highest uid, or the flag state
+    // is empty, and we're not using CondStore, we definitely need a full sync.
+    //
+    // Print to log items affecting needFullFolderSync:
+    MOZ_LOG(IMAP_CS, LogLevel::Debug,
+            ("Do full sync?: mFolderHighestUID=%" PRIu32 ", added=%" PRId32 ", useCS=%s",
+             mFolderHighestUID, added, useCS ? "true" : "false"));
+    bool needFullFolderSync = !mFolderHighestUID || (flagStateEmpty && !useCS);
+    bool needFolderSync = false;
+
+    if (!needFullFolderSync)
+    {
+      // Figure out if we need to do a non-highwater mark sync.
+      // Set needFolderSync true when at least 1 of these 3 cases is true:
+      // 1. Have no uids in flag array or all flag elements are marked deleted AND
+      // not using CONDSTORE.
+      // 2. Have no uids in flag array or all flag elements are marked deleted AND
+      // using "just mark as deleted" and EXISTS response count differs from
+      // stored message count for folder.
+      // 3. Using CONDSTORE and highest MODSEQ response is not equal to stored
+      // mod seq for folder.
+
+      // Print to log items affecting needFolderSync:
+      MOZ_LOG(IMAP_CS, LogLevel::Debug,
+              ("1. Do a sync?: added=%" PRId32 ", deleted=%" PRId32 ", useCS=%s",
+               added, deleted,  useCS ? "true" : "false"));
+      MOZ_LOG(IMAP_CS, LogLevel::Debug,
+              ("2. Do a sync?: ShowDeletedMsgs=%s, exists=%" PRId32 ", mFolderTotalMsgCount=%" PRId32 "",
+               GetShowDeletedMessages() ? "true" : "false", GetServerStateParser().NumberOfMessages(),
+               mFolderTotalMsgCount));
+      MOZ_LOG(IMAP_CS, LogLevel::Debug,
+              ("3. Do a sync?: fHighestModSeq=%" PRIu64 ", mFolderLastModSeq=%" PRIu64 "",
+               GetServerStateParser().fHighestModSeq,  mFolderLastModSeq));
+
+      needFolderSync =
+        (
+          (flagStateEmpty || added == deleted) &&
+          (
+            !useCS
+            ||
+            (GetShowDeletedMessages() &&
+             GetServerStateParser().NumberOfMessages() != mFolderTotalMsgCount)
+          )
+        )
+        ||
+        (useCS && GetServerStateParser().fHighestModSeq != mFolderLastModSeq);
+    }
+    MOZ_LOG(IMAP_CS, LogLevel::Debug, ("needFullFolderSync=%s, needFolderSync=%s",
+            needFullFolderSync ? "true" : "false", needFolderSync ? "true" : "false"));
 
     if (needFullFolderSync || needFolderSync)
     {
       nsCString idsToFetch("1:*");
       char fetchModifier[40] = "";
-      if (!needFullFolderSync && !GetShowDeletedMessages() && UseCondStore())
+      if (!needFullFolderSync && !GetShowDeletedMessages() && useCS)
+      {
+        m_flagState->StartCapture();
+        MOZ_LOG(IMAP_CS, LogLevel::Debug, ("Doing UID fetch 1:* (CHANGEDSINCE %" PRIu64 ")",
+                mFolderLastModSeq));
         PR_snprintf(fetchModifier, sizeof(fetchModifier), " (CHANGEDSINCE %llu)",
                     mFolderLastModSeq);
+      }
       else
         m_flagState->SetPartialUIDFetch(false);
 
       FetchMessage(idsToFetch, kFlags, fetchModifier);
       // lets see if we should expunge during a full sync of flags.
       if (GetServerStateParser().LastCommandSuccessful())
       {
         // if we did a CHANGEDSINCE fetch, do a sanity check on the msg counts
         // to see if some other client may have done an expunge.
         if (m_flagState->GetPartialUIDFetch())
         {
-          if (m_flagState->NumberOfDeletedMessages() +
-              mFolderTotalMsgCount != GetServerStateParser().NumberOfMessages())
+          uint32_t numExists = GetServerStateParser().NumberOfMessages();
+          uint32_t numPrevExists = mFolderTotalMsgCount;
+
+          if (MOZ_LOG_TEST(IMAP_CS, LogLevel::Debug))
+          {
+            int32_t addedByPartialFetch;
+            m_flagState->GetNumberOfMessages(&addedByPartialFetch);
+            MOZ_LOG(IMAP_CS, LogLevel::Debug, ("Sanity, deleted=%" PRId32 ", numPrevExists=%" PRIu32 ", numExists=%" PRIu32 "",
+                    m_flagState->NumberOfDeletedMessages(), numPrevExists,  numExists));
+            MOZ_LOG(IMAP_CS, LogLevel::Debug, ("Sanity, addedByPartialFetch=%" PRId32 "",
+                    addedByPartialFetch));
+          }
+
+          // Determine the number of new UIDs just fetched that are greater than
+          // the saved highest UID for the folder. numToCheck will contain the
+          // number of UIDs just fetched and, of course, not all are new.
+          uint32_t numNewUIDs = 0;
+          uint32_t numToCheck = m_flagState->GetNumAdded();
+          bool flagChangeDetected = false;
+          MOZ_LOG(IMAP_CS, LogLevel::Debug, ("numToCheck=%" PRIu32 "", numToCheck));
+          if (numToCheck && mFolderHighestUID)
           {
-            // sanity check failed - fall back to full flag sync
+            uint32_t uid;
+            int32_t topIndex;
+            m_flagState->GetNumberOfMessages(&topIndex);
+            do {
+              topIndex--;
+              m_flagState->GetUidOfMessage(topIndex, &uid);
+              if (uid && uid != nsMsgKey_None)
+              {
+                if (uid > mFolderHighestUID)
+                {
+                  numNewUIDs++;
+                  MOZ_LOG(IMAP_CS, LogLevel::Debug, ("numNewUIDs=%" PRIu32 ", Added new UID=%" PRIu32 "",
+                       numNewUIDs ,uid));
+                  numToCheck--;
+                }
+                else
+                {
+                  // Just a flag change on an existing UID. No more new UIDs
+                  // will be found. This does not detect an expunged message.
+                  flagChangeDetected = true;
+                  MOZ_LOG(IMAP_CS, LogLevel::Debug, ("Not new uid = %" PRIu32 "", uid));
+                  break;
+                }
+              }
+            } while(numToCheck);
+          }
+
+          // Another client expunged at least one message if the number of new
+          // UIDs is not equal to the observed change in the number of messages
+          // existing in the folder.
+          bool expungeHappened = numNewUIDs != (numExists - numPrevExists);
+          if (expungeHappened)
+          {
+            // Sanity check failed - need full fetch to remove expunged msgs.
+            MOZ_LOG(IMAP_CS, LogLevel::Debug,
+                    ("Other client expunged msgs, do full fetch to remove expunged msgs"));
             m_flagState->Reset();
             m_flagState->SetPartialUIDFetch(false);
             FetchMessage(NS_LITERAL_CSTRING("1:*"), kFlags);
           }
+          else if (numNewUIDs == 0)
+          {
+            // Nothing has been expunged and no new UIDs, so if just a flag
+            // change on existing message(s), avoid unneeded fetch of flags for
+            // messages with UIDs at and above uid (see var uid above) when
+            // "highwater mark" fetch occurs below.
+            if (mFolderHighestUID && flagChangeDetected)
+            {
+              MOZ_LOG(IMAP_CS, LogLevel::Debug, ("Avoid unneeded fetches after just flag changes"));
+              GetServerStateParser().ResetHighestRecordedUID();
+            }
+          }
         }
         int32_t numDeleted = m_flagState->NumberOfDeletedMessages();
         // Don't do expunge when we are lite selecting folder because we
         // could be doing undo.
         // Expunge if we're always expunging, or the number of deleted messages
         // is over the threshold, and we're either always respecting the
         // threshold, or we're expunging based on the delete model, and
         // the delete model is not the imap delete model.
@@ -4187,22 +4301,25 @@ void nsImapProtocol::ProcessMailboxUpdat
              (numDeleted >= gExpungeThreshold &&
               (gExpungeOption == kAutoExpungeOnThreshold ||
                (gExpungeOption == kAutoExpungeDeleteModel && !GetShowDeletedMessages())))))
           Expunge();
       }
     }
     else
     {
+      // Obtain the highest (highwater mark) UID seen since the last UIDVALIDITY
+      // response occurred (associated with the most recent SELECT for the folder).
       uint32_t highestRecordedUID = GetServerStateParser().HighestRecordedUID();
       // if we're using CONDSTORE, and the parser hasn't seen any UIDs, use
-      // the highest UID we've seen from the folder.
-      if (UseCondStore() && !highestRecordedUID)
+      // the highest UID previously seen and saved for the folder instead.
+      if (useCS && !highestRecordedUID)
         highestRecordedUID = mFolderHighestUID;
-
+      MOZ_LOG(IMAP_CS, LogLevel::Debug, ("Check for new messages above UID=%" PRIu32 "",
+              highestRecordedUID));
       AppendUid(fetchStr, highestRecordedUID + 1);
       fetchStr.AppendLiteral(":*");
       FetchMessage(fetchStr, kFlags);      // only new messages please
     }
   }
   else if (GetServerStateParser().LastCommandSuccessful())
   {
     GetServerStateParser().ResetFlagInfo();
--- a/mailnews/imap/src/nsImapServerResponseParser.cpp
+++ b/mailnews/imap/src/nsImapServerResponseParser.cpp
@@ -3271,16 +3271,21 @@ uint32_t nsImapServerResponseParser::Cur
   return fCurrentResponseUID;
 }
 
 uint32_t nsImapServerResponseParser::HighestRecordedUID()
 {
   return fHighestRecordedUID;
 }
 
+void nsImapServerResponseParser::ResetHighestRecordedUID()
+{
+  fHighestRecordedUID = 0;
+}
+
 bool nsImapServerResponseParser::IsNumericString(const char *string)
 {
   int i;
   for(i = 0; i < (int) PL_strlen(string); i++)
   {
     if (! isdigit(string[i]))
     {
       return false;
--- a/mailnews/imap/src/nsImapServerResponseParser.h
+++ b/mailnews/imap/src/nsImapServerResponseParser.h
@@ -60,16 +60,17 @@ public:
   // folder
   bool       CurrentFolderReadOnly();
   int32_t    NumberOfMessages();
   int32_t    NumberOfRecentMessages();
   int32_t    NumberOfUnseenMessages();
   int32_t    FolderUID();
   uint32_t   CurrentResponseUID();
   uint32_t   HighestRecordedUID();
+  void       ResetHighestRecordedUID();
   void       SetCurrentResponseUID(uint32_t uid);
   bool       IsNumericString(const char *string);
   uint32_t   SizeOfMostRecentMessage();
   void       SetTotalDownloadSize(int32_t newSize) { fTotalDownloadSize = newSize; }
 
   nsImapSearchResultIterator *CreateSearchResultIterator();
   void ResetSearchResultSequence() {fSearchResults->ResetSequence();}