fixes for hierarchy delimiter handling, multiple fetches of imap messages to retrieve inline attachments, crash in morkRowObject::CloseRowObject r=mscott 52260, 59376 58691

mailnews/imap/public/nsIImapMessageSink.idl

@@ -22,6 +22,7 @@
 
 #include "nsISupports.idl"
 #include "MailNewsTypes2.idl"
+#include "nsIImapUrl.idl"
 
 interface nsIFileSpec;
 
@@ -50,4 +51,5 @@ interface nsIImapMessageSink : nsISupports {
 
     void GetMessageSizeFromDB(in string id, in boolean idIsUid, out unsigned long size);
 
+    void SetContentModified(in nsIImapUrl aImapUrl, in nsImapContentModifiedType modified);
 };

mailnews/imap/src/nsImapIncomingServer.cpp

@@ -463,6 +463,7 @@ nsImapIncomingServer::LoadNextQueuedUrl(PRBool *aResult)
         aSupport(getter_AddRefs(m_urlQueue->ElementAt(0)));
     nsCOMPtr<nsIImapUrl>
         aImapUrl(do_QueryInterface(aSupport, &rv));
+    nsCOMPtr<nsIMsgMailNewsUrl> aMailNewsUrl(do_QueryInterface(aSupport, &rv));
 
     if (aImapUrl)
     {
@@ -479,22 +480,10 @@ nsImapIncomingServer::LoadNextQueuedUrl(PRBool *aResult)
 
           mockChannel->Close(); // try closing it to get channel listener nulled out.
 
-          nsCOMPtr<nsINetDataCacheManager> cacheManager = do_GetService(NS_NETWORK_CACHE_MANAGER_CONTRACTID, &res);
+          if (aMailNewsUrl)
-          if (NS_SUCCEEDED(res) && cacheManager)
           {
             nsCOMPtr<nsICachedNetData>  cacheEntry;
-            // Retrieve an existing cache entry or create a new one if none exists for the
+            res = aMailNewsUrl->GetMemCacheEntry(getter_AddRefs(cacheEntry));
-            // given URL.
-            nsXPIDLCString urlCString; 
-            // eventually we are going to want to use the url spec - the query/ref part 'cause that doesn't
-            // distinguish urls.......
-            url->GetSpec(getter_Copies(urlCString));
-            // for now, truncate of the query part so we don't duplicate urls in the cache...
-            char * anchor = PL_strrchr(urlCString, '?');
-            if (anchor)
-              *anchor = '\0';
-            res = cacheManager->GetCachedNetData(urlCString, 0, 0, nsINetDataCacheManager::BYPASS_PERSISTENT_CACHE,
-                                                getter_AddRefs(cacheEntry));
             if (NS_SUCCEEDED(res) && cacheEntry)
               cacheEntry->Delete();
           }
@@ -917,7 +906,7 @@ NS_IMETHODIMP nsImapIncomingServer::PossibleImapMailbox(const char *folderPath,
 	}
 
     nsCAutoString dupFolderPath(folderPath);
-    if (dupFolderPath.Last() == '/')
+    if (dupFolderPath.Last() == hierarchyDelimiter)
     {
         dupFolderPath.SetLength(dupFolderPath.Length()-1);
         // *** this is what we did in 4.x in order to list uw folder only
@@ -934,7 +923,7 @@ NS_IMETHODIMP nsImapIncomingServer::PossibleImapMailbox(const char *folderPath,
 
     uri.Assign(serverUri);
 
-    PRInt32 leafPos = folderName.RFindChar('/');
+    PRInt32 leafPos = folderName.RFindChar(hierarchyDelimiter);
 
     nsCAutoString parentName(folderName);
 	nsCAutoString parentUri(uri);

mailnews/imap/src/nsImapMailFolder.cpp

@@ -326,13 +326,16 @@ nsresult nsImapMailFolder::CreateSubFolders(nsFileSpec &path)
 
           // take the full unicode folder name and find the unicode leaf name.
           currentFolderNameStr.Assign(unicodeName);
-          PRInt32 leafPos = currentFolderNameStr.RFindChar('/');
+
+          PRUnichar delimiter = 0;
+          GetHierarchyDelimiter(&delimiter);
+          PRInt32 leafPos = currentFolderNameStr.RFindChar(delimiter);
           if (leafPos > 0)
             currentFolderNameStr.Cut(0, leafPos + 1);
 
           // take the utf7 full online name, and determine the utf7 leaf name
           utf7LeafName.AssignWithConversion(onlineFullUtf7Name);
-          leafPos = utf7LeafName.RFindChar('/');
+          leafPos = utf7LeafName.RFindChar(delimiter);
           if (leafPos > 0)
             utf7LeafName.Cut(0, leafPos + 1);
         }
@@ -567,7 +570,7 @@ NS_IMETHODIMP nsImapMailFolder::CreateClientSubfolderInfo(const char *folderName
     nsAutoString leafName; leafName.AssignWithConversion(folderName);
     nsAutoString folderNameStr;
     nsAutoString parentName = leafName;
-    PRInt32 folderStart = leafName.FindChar('/');
+    PRInt32 folderStart = leafName.FindChar(hierarchyDelimiter);
     if (folderStart > 0)
     {
         NS_WITH_SERVICE(nsIRDFService, rdf, kRDFServiceCID, &rv);
@@ -626,11 +629,11 @@ NS_IMETHODIMP nsImapMailFolder::CreateClientSubfolderInfo(const char *folderName
       //need to set the folder name
       nsCOMPtr <nsIDBFolderInfo> folderInfo;
       rv = unusedDB->GetDBFolderInfo(getter_AddRefs(folderInfo));
-      if(NS_SUCCEEDED(rv))
+//      if(NS_SUCCEEDED(rv))
-      {
+//      {
         // ### DMB used to be leafNameFromUser?
-        folderInfo->SetMailboxName(&folderNameStr);
+//        folderInfo->SetMailboxName(&folderNameStr);
-      }
+//      }
 
       //Now let's create the actual new folder
       rv = AddSubfolderWithPath(&folderNameStr, dbFileSpec, getter_AddRefs(child));
@@ -646,7 +649,15 @@ NS_IMETHODIMP nsImapMailFolder::CreateClientSubfolderInfo(const char *folderName
         onlineName.AppendWithConversion(folderNameStr);
         imapFolder->SetVerifiedAsOnlineFolder(PR_TRUE);
         imapFolder->SetOnlineName(onlineName.GetBuffer());
-                imapFolder->SetHierarchyDelimiter(hierarchyDelimiter);
+        imapFolder->SetHierarchyDelimiter(hierarchyDelimiter);
+        // store the online name as the mailbox name in the db folder info
+        // I don't think anyone uses the mailbox name, so we'll use it
+        // to restore the online name when blowing away an imap db.
+        if (folderInfo)
+        {
+          nsAutoString unicodeOnlineName; unicodeOnlineName.AssignWithConversion(onlineName);
+          folderInfo->SetMailboxName(&unicodeOnlineName);
+        }
       }
 
             unusedDB->SetSummaryValid(PR_TRUE);
@@ -785,6 +796,7 @@ NS_IMETHODIMP nsImapMailFolder::GetHierarchyDelimiter(PRUnichar *aHierarchyDelim
 {
   if (!aHierarchyDelimiter)
     return NS_ERROR_NULL_POINTER;
+  ReadDBFolderInfo(PR_FALSE); // update cache first.
   *aHierarchyDelimiter = m_hierarchyDelimiter;
   return NS_OK;
 }
@@ -1366,7 +1378,7 @@ NS_IMETHODIMP nsImapMailFolder::SetOnlineName(const char * aOnlineFolderName)
   nsCOMPtr<nsIDBFolderInfo> folderInfo;
   m_onlineFolderName = aOnlineFolderName;
   rv = GetDBFolderInfoAndDB(getter_AddRefs(folderInfo), getter_AddRefs(db));
-  if(NS_SUCCEEDED(rv))
+  if(NS_SUCCEEDED(rv) && folderInfo)
   {
     nsAutoString onlineName; onlineName.AssignWithConversion(aOnlineFolderName);
     rv = folderInfo->SetProperty("onlineName", &onlineName);
@@ -1397,6 +1409,7 @@ nsImapMailFolder::GetDBFolderInfoAndDB(nsIDBFolderInfo **folderInfo, nsIMsgDatab
   nsresult openErr=NS_ERROR_UNEXPECTED;
   if(!db || !folderInfo)
   return NS_ERROR_NULL_POINTER; //ducarroz: should we use NS_ERROR_INVALID_ARG?
+  nsresult rv;
 
   openErr = GetDatabase(nsnull);
 
@@ -1414,19 +1427,22 @@ nsImapMailFolder::GetDBFolderInfoAndDB(nsIDBFolderInfo **folderInfo, nsIMsgDatab
           m_onlineFolderName.Assign(onlineName);
         else
         {
-          char *uri = nsnull;
+          nsAutoString autoOnlineName; 
-          nsresult rv = GetURI(&uri);
+          // autoOnlineName.AssignWithConversion(name);
-          if (NS_FAILED(rv)) return rv;
+          (*folderInfo)->GetMailboxName(&autoOnlineName);
-          char * hostname = nsnull;
+          if (autoOnlineName.Length() == 0)
-          rv = GetHostname(&hostname);
+          {
-          if (NS_FAILED(rv)) return rv;
+            nsXPIDLCString uri;
-          nsXPIDLCString name;
+            rv = GetURI(getter_Copies(uri));
-          rv = nsImapURI2FullName(kImapRootURI, hostname, uri, getter_Copies(name));
+            if (NS_FAILED(rv)) return rv;
-          m_onlineFolderName.Assign(name);
+            nsXPIDLCString hostname;
-          nsAutoString autoOnlineName; autoOnlineName.AssignWithConversion(name);
+            rv = GetHostname(getter_Copies(hostname));
+            if (NS_FAILED(rv)) return rv;
+            nsXPIDLCString name;
+            rv = nsImapURI2FullName(kImapRootURI, hostname, uri, getter_Copies(name));
+            m_onlineFolderName.Assign(name);
+          }
           rv = (*folderInfo)->SetProperty("onlineName", &autoOnlineName);
-          PR_FREEIF(uri);
-          PR_FREEIF(hostname);
         }
       }
     }
@@ -1855,6 +1871,7 @@ NS_IMETHODIMP nsImapMailFolder::UpdateImapMailboxInfo(
       if (mDatabase)
       {
         dbFolderInfo = null_nsCOMPtr();
+        NotifyStoreClosedAllHeaders();
         mDatabase->ForceClosed();
       }
       mDatabase = null_nsCOMPtr();
@@ -3289,6 +3306,12 @@ nsImapMailFolder::GetMessageSizeFromDB(const char *id, PRBool idIsUid, PRUint32
     return rv;
 }
 
+NS_IMETHODIMP
+nsImapMailFolder::SetContentModified(nsIImapUrl *aImapUrl, nsImapContentModifiedType modified)
+{
+  return aImapUrl->SetContentModified(modified);
+}
+
 NS_IMETHODIMP
 nsImapMailFolder::OnStartRunningUrl(nsIURI *aUrl)
 {

mailnews/imap/src/nsImapProtocol.cpp

@@ -59,8 +59,6 @@
 #include "nsIDNSService.h"
 
 // for the memory cache...
-#include "nsINetDataCacheManager.h"
-#include "nsINetDataCache.h"
 #include "nsICachedNetData.h"
 
 #include "nsCOMPtr.h"
@@ -3527,8 +3525,8 @@ PRBool  nsImapProtocol::GetActive()
 
 void nsImapProtocol::SetContentModified(IMAP_ContentModifiedType modified)
 {
-  if (m_runningUrl)
+  if (m_runningUrl && m_imapMessageSink)
-    m_runningUrl->SetContentModified(modified);
+    m_imapMessageSink->SetContentModified(m_runningUrl, modified);
 }
 
 
@@ -6809,38 +6807,23 @@ NS_IMETHODIMP nsImapMockChannel::AsyncRead(nsIStreamListener *listener, nsISuppo
   // look to see if this url should be added to the memory cache..
   PRBool useMemoryCache = PR_FALSE;
   mailnewsUrl->GetAddToMemoryCache(&useMemoryCache);
-  nsCOMPtr<nsINetDataCacheManager> cacheManager = do_GetService(NS_NETWORK_CACHE_MANAGER_CONTRACTID, &rv);
+  rv = mailnewsUrl->GetMemCacheEntry(getter_AddRefs(cacheEntry));
-  if (NS_SUCCEEDED(rv) && cacheManager)
+  if (NS_SUCCEEDED(rv) && cacheEntry)
   {
-    // Retrieve an existing cache entry or create a new one if none exists for the
+    PRBool updateInProgress;
-    // given URL.
+    cacheEntry->GetPartialFlag(&partialFlag);
-    nsXPIDLCString urlCString; 
+    cacheEntry->GetUpdateInProgress(&updateInProgress);        
-    // eventually we are going to want to use the url spec - the query/ref part 'cause that doesn't
+    cacheEntry->GetStoredContentLength(&contentLength);
-    // distinguish urls.......
+    // only try to update the cache entry if it isn't being used.
-    m_url->GetSpec(getter_Copies(urlCString));
+    // We always want to try to write to the cache entry if we can
-    // for now, truncate of the query part so we don't duplicate urls in the cache...
+    if (!updateInProgress)
-    char * anchor = PL_strrchr(urlCString, '?');
-    if (anchor)
-      *anchor = '\0';
-    rv = cacheManager->GetCachedNetData(urlCString, 0, 0, nsINetDataCacheManager::BYPASS_PERSISTENT_CACHE,
-                                        getter_AddRefs(cacheEntry));
-    if (NS_SUCCEEDED(rv) && cacheEntry)
     {
-      PRBool updateInProgress;
+      // now we need to figure out if the entry is new / or partially unfinished...
-      cacheEntry->GetPartialFlag(&partialFlag);
+      // this determines if we are going to USE the cache entry for reading the data
-      cacheEntry->GetUpdateInProgress(&updateInProgress);        
+      // vs. if we need to write data into the cache entry...
-      cacheEntry->GetStoredContentLength(&contentLength);
+      if (!contentLength || partialFlag)
-      // only try to update the cache entry if it isn't being used.
+        // we're going to fill up this cache entry, 
-      // and we want to write to the memory cache.
+        rv = cacheEntry->InterceptAsyncRead(listener, 0, getter_AddRefs(m_channelListener));
-      if (!updateInProgress && useMemoryCache)
-      {
-        // now we need to figure out if the entry is new / or partially unfinished...
-        // this determines if we are going to USE the cache entry for reading the data
-        // vs. if we need to write data into the cache entry...
-        if (!contentLength || partialFlag)
-          // we're going to fill up this cache entry, 
-          rv = cacheEntry->InterceptAsyncRead(listener, 0, getter_AddRefs(m_channelListener));
-      }
     }
   }
 
@@ -6866,30 +6849,38 @@ NS_IMETHODIMP nsImapMockChannel::AsyncRead(nsIStreamListener *listener, nsISuppo
   // to really load the msg with a protocol connection...
   if (cacheEntry && contentLength > 0 && !partialFlag)
   {
-    nsCOMPtr<nsIChannel> cacheChannel;
+    PRUint32 annotationLength = 0;
-    rv = cacheEntry->NewChannel(m_loadGroup, getter_AddRefs(cacheChannel));
+    char *annotation = nsnull;
-    if (NS_SUCCEEDED(rv))
+
+    rv = cacheEntry->GetAnnotation("ContentModified", &annotationLength, &annotation);
+    if (NS_SUCCEEDED(rv) && annotationLength == nsCRT::strlen("Not Modified") 
+      && annotation && !nsCRT::strncmp(annotation, "Not Modified", annotationLength))
     {
-      // turn around and make our ref on m_url an owning ref...and force the url to remove
+      nsCOMPtr<nsIChannel> cacheChannel;
-      // its reference on the mock channel...this is a complicated texas two step to solve
+      rv = cacheEntry->NewChannel(m_loadGroup, getter_AddRefs(cacheChannel));
-      // a nasty reference counting problem...
+      if (NS_SUCCEEDED(rv))
-      NS_IF_ADDREF(m_url);
-      mOwningRefToUrl = PR_TRUE;
-      imapUrl->SetMockChannel(nsnull);
-
-      // if we are going to read from the cache, then create a mock stream listener class and use it
-      nsImapCacheStreamListener * cacheListener = new nsImapCacheStreamListener();
-      NS_ADDREF(cacheListener);
-      cacheListener->Init(m_channelListener, NS_STATIC_CAST(nsIChannel *, this));
-      rv = cacheChannel->AsyncRead(cacheListener, m_channelContext);
-      NS_RELEASE(cacheListener);
-
-      if (NS_SUCCEEDED(rv)) // ONLY if we succeeded in actually starting the read should we return
       {
-        // if the msg is unread, we should mark it read on the server. This lets
+        // turn around and make our ref on m_url an owning ref...and force the url to remove
-        // the code running this url we're loading from the cache, if it cares.
+        // its reference on the mock channel...this is a complicated texas two step to solve
-        imapUrl->SetMsgLoadingFromCache(PR_TRUE);
+        // a nasty reference counting problem...
-        return rv;
+        NS_IF_ADDREF(m_url);
+        mOwningRefToUrl = PR_TRUE;
+        imapUrl->SetMockChannel(nsnull);
+
+        // if we are going to read from the cache, then create a mock stream listener class and use it
+        nsImapCacheStreamListener * cacheListener = new nsImapCacheStreamListener();
+        NS_ADDREF(cacheListener);
+        cacheListener->Init(m_channelListener, NS_STATIC_CAST(nsIChannel *, this));
+        rv = cacheChannel->AsyncRead(cacheListener, m_channelContext);
+        NS_RELEASE(cacheListener);
+
+        if (NS_SUCCEEDED(rv)) // ONLY if we succeeded in actually starting the read should we return
+        {
+          // if the msg is unread, we should mark it read on the server. This lets
+          // the code running this url we're loading from the cache, if it cares.
+          imapUrl->SetMsgLoadingFromCache(PR_TRUE);
+          return rv;
+        }
       }    
     }
   }

mailnews/imap/src/nsImapUrl.cpp

@@ -42,7 +42,7 @@
 #include "nsXPIDLString.h"
 #include "nsAutoLock.h"
 #include "nsIMAPNamespace.h"
-
+#include "nsICachedNetData.h"
 // rdf stuff is needed to get the charset from the imap folder associated with the url.
 #include "nsIRDFService.h"
 #include "rdf.h"
@@ -893,6 +893,28 @@ NS_IMETHODIMP nsImapUrl::SetAllowContentChange(PRBool allowContentChange)
 NS_IMETHODIMP nsImapUrl::SetContentModified(nsImapContentModifiedType contentModified)
 {
 	m_contentModified = contentModified;
+  nsCOMPtr<nsICachedNetData>  cacheEntry;
+  nsresult res = GetMemCacheEntry(getter_AddRefs(cacheEntry));
+  if (NS_SUCCEEDED(res) && cacheEntry)
+  {
+    const char *contentModifiedAnnotation = "";
+    switch (m_contentModified)
+    {
+    case IMAP_CONTENT_NOT_MODIFIED:
+      contentModifiedAnnotation = "Not Modified";
+      break;
+    case IMAP_CONTENT_MODIFIED_VIEW_INLINE:
+      contentModifiedAnnotation = "Modified View Inline";
+      break;
+    case IMAP_CONTENT_MODIFIED_VIEW_AS_LINKS:
+      NS_ASSERTION(PR_FALSE, "we're not using this anymore!");
+      break;
+    case IMAP_CONTENT_FORCE_CONTENT_NOT_MODIFIED:
+      contentModifiedAnnotation = "Force Content Not Modified";
+      break;
+    }
+    cacheEntry->SetAnnotation("ContentModified", nsCRT::strlen(contentModifiedAnnotation), contentModifiedAnnotation);
+  }
 	return NS_OK;
 }
 

mailnews/imap/src/nsImapUtils.cpp

@@ -159,7 +159,7 @@ nsImapURI2Path(const char* rootURI, const char* uriStr, nsFileSpec& pathResult)
 }
 
 nsresult
-nsImapURI2FullName(const char* rootURI, const char* hostname, char* uriStr,
+nsImapURI2FullName(const char* rootURI, const char* hostname, const char* uriStr,
                    char **name)
 {
     nsAutoString uri; uri.AssignWithConversion(uriStr);

mailnews/imap/src/nsImapUtils.h

@@ -35,7 +35,7 @@ nsImapURI2Path(const char* rootURI, const char* uriStr,
                nsFileSpec& pathResult);
 
 extern nsresult
-nsImapURI2FullName(const char* rootURI, const char* hostname, char* uriStr,
+nsImapURI2FullName(const char* rootURI, const char* hostname, const char* uriStr,
                    char **name);
 
 extern nsresult