fix some cases of copies of messages ending up in offline store, r=standar8, bug 609683
authorDavid Bienvenu <bienvenu@nventure.com>
Mon, 13 Dec 2010 08:24:49 -0800
changeset 6827 9d7a6096be8008ac4aeae6182b7d431222bff7ae
parent 6826 873e68c06d96764fcbc8b1ffc90f2dc62c1a82f5
child 6828 870f6de6d8579aeee1c749c2449fa6ddce0aab2d
push idunknown
push userunknown
push dateunknown
reviewersstandar8, bug
bugs609683
fix some cases of copies of messages ending up in offline store, r=standar8, bug 609683
mailnews/imap/src/nsImapIncomingServer.cpp
mailnews/imap/src/nsImapProtocol.cpp
mailnews/imap/src/nsImapProtocol.h
mailnews/imap/test/unit/test_imapStoreMsgOffline.js
--- a/mailnews/imap/src/nsImapIncomingServer.cpp
+++ b/mailnews/imap/src/nsImapIncomingServer.cpp
@@ -530,17 +530,20 @@ nsImapIncomingServer::LoadNextQueuedUrl(
         if (NS_SUCCEEDED(rv) && protocolInstance)
         {
           nsCOMPtr<nsIURI> url = do_QueryInterface(aImapUrl, &rv);
           if (NS_SUCCEEDED(rv) && url)
           {
             nsImapProtocol::LogImapUrl("playing queued url", aImapUrl);
             rv = protocolInstance->LoadImapUrl(url, aConsumer);
             NS_ASSERTION(NS_SUCCEEDED(rv), "failed running queued url");
-            urlRun = PR_TRUE;
+            PRBool isInbox;
+            protocolInstance->IsBusy(&urlRun, &isInbox);
+            if (!urlRun)
+              nsImapProtocol::LogImapUrl("didn't need to run", aImapUrl);
             removeUrlFromQueue = PR_TRUE;
           }
         }
         else
         {
           nsImapProtocol::LogImapUrl("failed creating protocol instance to play queued url", aImapUrl);
           keepGoing = PR_FALSE;
         }
--- a/mailnews/imap/src/nsImapProtocol.cpp
+++ b/mailnews/imap/src/nsImapProtocol.cpp
@@ -1988,36 +1988,98 @@ nsresult nsImapProtocol::SendData(const 
 
 /////////////////////////////////////////////////////////////////////////////////////////////
 // Begin protocol state machine functions...
 //////////////////////////////////////////////////////////////////////////////////////////////
 
   // ProcessProtocolState - we override this only so we'll link - it should never get called.
 
 nsresult nsImapProtocol::ProcessProtocolState(nsIURI * url, nsIInputStream * inputStream,
-									PRUint32 sourceOffset, PRUint32 length)
+                                              PRUint32 sourceOffset, PRUint32 length)
 {
   return NS_OK;
 }
 
+PRBool nsImapProtocol::TryToRunUrlLocally(nsIURI *aURL, nsISupports *aConsumer)
+{
+  nsresult rv;
+  nsCOMPtr<nsIImapUrl> imapUrl(do_QueryInterface(aURL, &rv));
+  NS_ENSURE_SUCCESS(rv, PR_FALSE);
+  nsCOMPtr<nsIMsgMailNewsUrl> mailnewsUrl = do_QueryInterface(aURL);
+  nsCString messageIdString;
+  imapUrl->GetListOfMessageIds(messageIdString);
+  PRBool useLocalCache = PR_FALSE;
+  if (!messageIdString.IsEmpty() && !HandlingMultipleMessages(messageIdString))
+  {
+    nsImapAction action;
+    imapUrl->GetImapAction(&action);
+    nsCOMPtr <nsIMsgFolder> folder;
+    mailnewsUrl->GetFolder(getter_AddRefs(folder));
+    NS_ENSURE_TRUE(folder, PR_FALSE);
+
+    folder->HasMsgOffline(atoi(messageIdString.get()), &useLocalCache);
+    mailnewsUrl->SetMsgIsInLocalCache(useLocalCache);
+    // We're downloading a single message for offline use, and it's
+    // already offline. So we shouldn't do anything, but we do
+    // need to notify the url listener.
+    if (useLocalCache && action == nsIImapUrl::nsImapMsgDownloadForOffline)
+    {
+      nsCOMPtr<nsIImapMailFolderSink> folderSink(do_QueryInterface(folder));
+      // This causes the url listener to get OnStart and Stop notifications.
+      folderSink->SetUrlState(this, mailnewsUrl, PR_TRUE, NS_OK);
+      folderSink->SetUrlState(this, mailnewsUrl, PR_FALSE, NS_OK);
+      return PR_TRUE;
+    }
+  }
+  if (!useLocalCache)
+    return PR_FALSE;
+
+  nsCOMPtr<nsIImapMockChannel> mockChannel;
+  imapUrl->GetMockChannel(getter_AddRefs(mockChannel));
+  if (!mockChannel)
+    return PR_FALSE;
+
+  nsImapMockChannel *imapChannel = static_cast<nsImapMockChannel *>(mockChannel.get());
+  if (!imapChannel)
+    return PR_FALSE;
+
+  nsCOMPtr <nsILoadGroup> loadGroup;
+  imapChannel->GetLoadGroup(getter_AddRefs(loadGroup));
+  if (!loadGroup) // if we don't have one, the url will snag one from the msg window...
+    mailnewsUrl->GetLoadGroup(getter_AddRefs(loadGroup));
+
+  if (loadGroup)
+    loadGroup->RemoveRequest((nsIRequest *) mockChannel, nsnull /* context isupports */, NS_OK);
+
+  if (imapChannel->ReadFromLocalCache())
+  {
+    (void) imapChannel->NotifyStartEndReadFromCache(PR_TRUE);
+    return PR_TRUE;
+  }
+  return PR_FALSE;
+}
+
+
 // LoadImapUrl takes a url, initializes all of our url specific data by calling SetupUrl.
 // If we don't have a connection yet, we open the connection. Finally, we signal the
 // url to run monitor to let the imap main thread loop process the current url (it is waiting
 // on this monitor). There is a contract that the imap thread has already been started b4 we
 // attempt to load a url....
 NS_IMETHODIMP nsImapProtocol::LoadImapUrl(nsIURI * aURL, nsISupports * aConsumer)
 {
   nsresult rv = NS_OK;
   if (aURL)
   {
 #ifdef DEBUG_bienvenu
     nsCAutoString urlSpec;
     aURL->GetSpec(urlSpec);
     printf("loading url %s\n", urlSpec.get());
 #endif
+    if (TryToRunUrlLocally(aURL, aConsumer))
+      return NS_OK;
     m_urlInProgress = PR_TRUE;
     m_imapMailFolderSink = nsnull;
     rv = SetupWithUrl(aURL, aConsumer);
     NS_ASSERTION(NS_SUCCEEDED(rv), "error setting up imap url");
     if (NS_FAILED(rv))
       return rv;
 
     SetupSinkProxy(); // generate proxies for all of the event sinks in the url
@@ -8983,16 +9045,36 @@ nsresult nsImapMockChannel::ReadFromMemC
     shouldUseCacheEntry = PR_TRUE;
   }
   else
   {
     // otherwise, we have the whole msg, and we should make sure the content isn't modified.
     rv = entry->GetMetaDataElement("ContentModified", getter_Copies(annotation));
     if (NS_SUCCEEDED(rv) && !annotation.IsEmpty())
       shouldUseCacheEntry = annotation.EqualsLiteral("Not Modified");
+    if (shouldUseCacheEntry)
+    {
+      PRUint32 entrySize;
+
+      rv = entry->GetDataSize(&entrySize);
+      nsCOMPtr<nsIMsgMessageUrl> msgUrl(do_QueryInterface(m_url));
+      if (msgUrl)
+      {
+        nsCOMPtr<nsIMsgDBHdr> msgHdr;
+        // A failure to get a message header isn't an error
+        msgUrl->GetMessageHeader(getter_AddRefs(msgHdr));
+        if (msgHdr)
+        {
+          PRUint32 messageSize;
+          if (NS_SUCCEEDED(msgHdr->GetMessageSize(&messageSize)) &&
+              messageSize != entrySize)
+            shouldUseCacheEntry = PR_FALSE;
+        }
+      }
+    }
   }
   if (shouldUseCacheEntry)
   {
     nsCOMPtr<nsIInputStream> in;
     PRUint32 readCount;
     rv = entry->OpenInputStream(0, getter_AddRefs(in));
     NS_ENSURE_SUCCESS(rv, rv);
     const int kFirstBlockSize = 100;
@@ -9016,17 +9098,17 @@ nsresult nsImapMockChannel::ReadFromMemC
     PRUint32 bytesAvailable;
     rv = in->Available(&bytesAvailable);
     NS_ENSURE_SUCCESS(rv, rv);
     if (!bytesAvailable)
       return NS_ERROR_FAILURE;
 
     nsCOMPtr<nsIInputStreamPump> pump;
     rv = NS_NewInputStreamPump(getter_AddRefs(pump), in);
-    if (NS_FAILED(rv)) return rv;
+    NS_ENSURE_SUCCESS(rv, rv);
 
     // 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, this);
     rv = pump->AsyncRead(cacheListener, m_channelContext);
     NS_RELEASE(cacheListener);
 
--- a/mailnews/imap/src/nsImapProtocol.h
+++ b/mailnews/imap/src/nsImapProtocol.h
@@ -448,16 +448,25 @@ private:
   PRUint32  *m_fetchMsgIdList;
   PRBool    m_fetchBodyListIsNew;
   PRUint32  m_fetchBodyCount;
   PRUint32  *m_fetchBodyIdList;
 
   // initialization function given a new url and transport layer
   nsresult  SetupWithUrl(nsIURI * aURL, nsISupports* aConsumer);
   void ReleaseUrlState(PRBool rerunningUrl); // release any state that is stored on a per action basis.
+  /**
+   * Last ditch effort to run the url without using an imap connection.
+   * If it turns out that we don't need to run the url at all (e.g., we're
+   * trying to download a single message for offline use and it has already
+   * been downloaded, this function will send the appropriate notifications.
+   *
+   * @returns true if the url has been run locally, or doesn't need to be run.
+   */
+  PRBool TryToRunUrlLocally(nsIURI *aURL, nsISupports *aConsumer);
 
   ////////////////////////////////////////////////////////////////////////////////////////
   // Communication methods --> Reading and writing protocol
   ////////////////////////////////////////////////////////////////////////////////////////
 
   // SendData not only writes the NULL terminated data in dataBuffer to our output stream
   // but it also informs the consumer that the data has been written to the stream.
   // aSuppressLogging --> set to true if you wish to suppress logging for this particular command.
@@ -694,16 +703,17 @@ private:
 class nsICacheEntryDescriptor;
 
 class nsImapMockChannel : public nsIImapMockChannel
                         , public nsICacheListener
                         , public nsITransportEventSink
                         , public nsSupportsWeakReference
 {
 public:
+  friend class nsImapProtocol;
 
   NS_DECL_ISUPPORTS
   NS_DECL_NSIIMAPMOCKCHANNEL
   NS_DECL_NSICHANNEL
   NS_DECL_NSIREQUEST
   NS_DECL_NSICACHELISTENER
   NS_DECL_NSITRANSPORTEVENTSINK
 
--- a/mailnews/imap/test/unit/test_imapStoreMsgOffline.js
+++ b/mailnews/imap/test/unit/test_imapStoreMsgOffline.js
@@ -5,16 +5,25 @@
  *   - Normal messages, no attachments.
  *   - Message with inline attachment (e.g., image)
  *   - Message with non-inline attachment (e.g., .doc file)
  *   - Message with mix of attachment types.
  */
 
 Components.utils.import("resource://gre/modules/XPCOMUtils.jsm");
 
+load("../../../resources/asyncTestUtils.js");
+
+load("../../../resources/messageGenerator.js");
+load("../../../resources/messageModifier.js");
+load("../../../resources/messageInjection.js");
+
+var gMessageGenerator = new MessageGenerator();
+var gScenarioFactory = new MessageScenarioFactory(gMessageGenerator);
+
 const nsMsgMessageFlags = Ci.nsMsgMessageFlags;
 
 var gServer;
 var gIMAPDaemon;
 var gIMAPInbox;
 var gIMAPIncomingServer;
 var gIMAPTrashFolder;
 var gMessenger;
@@ -24,16 +33,20 @@ var gCurTestNum;
 
 var gMsgFile1 = do_get_file("../../../data/bugmail10");
 const gMsgId1 = "200806061706.m56H6RWT004933@mrapp54.mozilla.org";
 var gMsgFile2 = do_get_file("../../../data/image-attach-test");
 const gMsgId2 = "4A947F73.5030709@xxx.com";
 var gMsgFile3 = do_get_file("../../../data/external-attach-test");
 const gMsgId3 = "876TY.5030709@xxx.com";
 
+var gFirstNewMsg;
+var gFirstMsgSize;
+var gImapInboxOfflineStoreSize;
+
 // We use this as a display consumer
 var streamListener =
 {
   _data: "",
 
   QueryInterface:
     XPCOMUtils.generateQI([Ci.nsIStreamListener, Ci.nsIRequestObserver]),
 
@@ -203,19 +216,95 @@ const gTestArray =
                                             null,
                                             url);
   },
   function verify3rdMsg() {
     let msg3 = gIMAPInbox.msgDatabase.getMsgHdrForMessageID(gMsgId3);
     // can't turn this on because our fake server doesn't support body structure.
 //    do_check_eq(msg3.flags & nsMsgMessageFlags.Offline, 0);
     do_timeout(0, function(){doTest(++gCurTestNum)});
-  }
+  },
+  function addNewMsgs() {
+    let mbox = gIMAPDaemon.getMailbox("INBOX")
+    // make a couple messges
+    let messages = [];
+    let bodyString = "";
+    for (i = 0; i < 100; i++)
+      bodyString += "1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890\r\n";
+
+    let gMessageGenerator = new MessageGenerator();
+    messages = messages.concat(gMessageGenerator.makeMessage({body: {body: bodyString, contentType: "text/plain"}}));
+
+    gFirstNewMsg = mbox.uidnext;
+    // need to account for x-mozilla-status, status2, and envelope.
+    gFirstMsgSize = messages[0].toMessageString().length + 102;
+
+    let ioService = Cc["@mozilla.org/network/io-service;1"]
+                      .getService(Ci.nsIIOService);
+
+    messages.forEach(function (message)
+    {
+      let dataUri = ioService.newURI("data:text/plain;base64," +
+                                      btoa(message.toMessageString()),
+                                     null, null);
+      mbox.addMessage(new imapMessage(dataUri.spec, mbox.uidnext++, []));
+    });
+    gIMAPInbox.updateFolderWithListener(null, URLListener);
+  },
+  function testQueuedOfflineDownload()
+  {
+    // Make sure that streaming the same message and then trying to download
+    // it for offline use doesn't end up in it getting added to the offline 
+    // store twice.
+    gImapInboxOfflineStoreSize = gIMAPInbox.filePath.fileSize + gFirstMsgSize;
+    let newMsgHdr = gIMAPInbox.GetMessageHeader(gFirstNewMsg);
+    let msgURI = newMsgHdr.folder.getUriForMsg(newMsgHdr);
+    let messenger = Cc["@mozilla.org/messenger;1"].createInstance(Ci.nsIMessenger);
+    let msgServ = messenger.messageServiceFromURI(msgURI);
+    msgServ.streamMessage(msgURI, gStreamListener, null, null, false, "", false);
+    let msgs = Cc["@mozilla.org/array;1"].createInstance(Ci.nsIMutableArray);
+    msgs.appendElement(gIMAPInbox.GetMessageHeader(gFirstNewMsg), false);
+    gIMAPInbox.DownloadMessagesForOffline(msgs, null);
+  },
+  function firstStreamFinished()
+  {
+    dump("first stream finished\n");
+    // Wait for a couple seconds for the second download to either start
+    // writing to the offline store (failure case), or complete instantly.
+    // If DownloadMessagesForOffline took a listener, we wouldn't have to
+    // do this.
+    do_timeout(2000, function(){doTest(++gCurTestNum);});
+  },
+  function checkOfflineStoreSize()
+  {
+    dump("checking offline store size\n");
+    do_check_true(gIMAPInbox.filePath.fileSize <= gImapInboxOfflineStoreSize);
+    do_timeout(0, function(){doTest(++gCurTestNum);});
+  },
 ]
 
+gStreamListener = {
+  QueryInterface : XPCOMUtils.generateQI([Ci.nsIStreamListener]),
+  _stream : null,
+  _data : null,
+  onStartRequest : function (aRequest, aContext) {
+    this._data = "";
+  },
+  onStopRequest : function (aRequest, aContext, aStatusCode) {
+    do_timeout(0, function(){doTest(++gCurTestNum);});
+  },
+  onDataAvailable : function (aRequest, aContext, aInputStream, aOff, aCount) {
+    if (this._stream == null) {
+      this._stream = Cc["@mozilla.org/scriptableinputstream;1"].createInstance(Ci.nsIScriptableInputStream);
+      this._stream.init(aInputStream);
+    }
+    this._data += this._stream.read(aCount);
+  },
+};
+
 function doTest(test)
 {
   if (test <= gTestArray.length)
   {
     dump("Doing test " + test + "\n");
     gCurTestNum = test;
 
     var testFn = gTestArray[test - 1];