Bug 1857450 - Write out the whole retrieved nntp article in one go. r=BenC

Especially as messages are usually small, just buffer the download in memory and write out on completion.

Differential Revision: https://phabricator.services.mozilla.com/D208492

--HG--
extra : moz-landing-system : lando

mailnews/imap/src/nsImapMailFolder.cpp

@@ -4317,14 +4317,8 @@ nsImapMailFolder::ParseAdoptedMsgLine(const char* adoptedMessageLine,
   }
 
   // adoptedMessageLine is actually a string with a lot of message lines,
-  // separated by native line terminators we need to count the number of
-  // MSG_LINEBREAK's to determine how much to increment m_numOfflineMsgLines by.
-  const char* nextLine = adoptedMessageLine;
-  do {
-    m_numOfflineMsgLines++;
-    nextLine = PL_strstr(nextLine, MSG_LINEBREAK);
-    if (nextLine) nextLine += MSG_LINEBREAK_LEN;
-  } while (nextLine && *nextLine);
+  nsDependentCString data(adoptedMessageLine);
+  m_numOfflineMsgLines += data.CountChar('\n');
 
   if (m_tempMessageStream) {
     rv = m_tempMessageStream->Write(adoptedMessageLine,

mailnews/news/public/nsIMsgNewsFolder.idl

@@ -6,10 +6,6 @@
 #include "nsISupports.idl"
 #include "nsIMsgFolder.idl"
 
-%{C++
-#include "nsTArray.h"
-%}
-
 interface nsIMsgWindow;
 interface nsINntpIncomingServer;
 
@@ -112,29 +108,14 @@ interface nsIMsgNewsFolder : nsISupports {
   void getNextNMessages(in nsIMsgWindow aMsgWindow);
 
   /**
-   * Begin loading a message into the folder's offline store.
-   * @param key - the key of the message being loaded (the folder must
-   *              already have the message in it's DB)
-   */
-  void notifyDownloadBegin(in nsMsgKey key);
-
-  /**
-   * Feed the next line of a message into the folder, to be invoked multiple
-   * times between notifyDownloadBegin() and notifyDownloadEnd().
+   * Feed the article (message) into the folder, allowing offline storage
+   * to take place if configured.
    *
-   * @param line - a single line of data to append to the message, including
-   *               end-of-line terminator.
+   * @param articleNumber - The article being loaded. It must already be in
+   *   the database.
+   * @param data - Article data, including end-of-line terminators.
    */
-  void notifyDownloadedLine(in ACString line);
-
-  /**
-   * Finish loading a message into the folder. If an error occurs, the
-   * folder hears about it via this function, and should abort the message.
-   *
-   * @param status - NS_OK if the operation was completed, an error code
-   *                 otherwise.
-   */
-  void notifyDownloadEnd(in nsresult status);
+  void notifyArticleDownloaded(in unsigned long articleNumber, in ACString data);
 
   void notifyFinishedDownloadinghdrs();
 

mailnews/news/src/NntpClient.sys.mjs

@@ -763,8 +763,6 @@ export class NntpClient {
   _actionArticle = () => {
     this._sendCommand(`ARTICLE ${this._articleNumber || this._messageId}`);
     this._nextAction = this._actionArticleResponse;
-    this._newsFolder?.notifyDownloadBegin(this._articleNumber);
-    this._downloadingToFolder = true;
   };
 
   /**
@@ -775,18 +773,15 @@ export class NntpClient {
   _actionArticleResponse = ({ data }) => {
     const lineSeparator = AppConstants.platform == "win" ? "\r\n" : "\n";
 
+    let article = "";
     this._lineReader.read(
       data,
       line => {
-        // NewsFolder will decide whether to save it to the offline storage.
-        this._newsFolder?.notifyDownloadedLine(
-          line.slice(0, -2) + lineSeparator
-        );
+        article += line.slice(0, -2) + lineSeparator;
         this.onData(line);
       },
       () => {
-        this._newsFolder?.notifyDownloadEnd(Cr.NS_OK);
-        this._downloadingToFolder = false;
+        this._newsFolder?.notifyArticleDownloaded(this._articleNumber, article);
         this._actionDone();
       }
     );
@@ -958,12 +953,6 @@ export class NntpClient {
     if (this._done) {
       return;
     }
-    if (this._downloadingToFolder) {
-      // If we're in the middle of sending a message to the folder, make sure
-      // the folder knows we're aborting.
-      this._newsFolder?.notifyDownloadEnd(Cr.NS_ERROR_FAILURE);
-      this._downloadingToFolder = false;
-    }
     this._done = true;
     this._logger.debug(`Done with status=${status}`);
     this.onDone(status);

mailnews/news/src/nsNewsFolder.cpp

@@ -1420,31 +1420,22 @@ NS_IMETHODIMP nsMsgNewsFolder::GetLocalMsgStream(nsIMsgDBHdr* hdr,
   return GetMsgInputStream(hdr, stream);
 }
 
-NS_IMETHODIMP nsMsgNewsFolder::NotifyDownloadBegin(nsMsgKey key) {
+NS_IMETHODIMP nsMsgNewsFolder::NotifyArticleDownloaded(uint32_t articleNumber,
+                                                       nsACString const& data) {
   if (!m_downloadMessageForOfflineUse) {
     return NS_OK;
   }
-  nsresult rv = GetMessageHeader(key, getter_AddRefs(m_offlineHeader));
+  nsresult rv =
+      GetMessageHeader(articleNumber, getter_AddRefs(m_offlineHeader));
   NS_ENSURE_SUCCESS(rv, rv);
-  return StartNewOfflineMessage();  // Sets up m_tempMessageStream et al.
-}
-
-NS_IMETHODIMP nsMsgNewsFolder::NotifyDownloadedLine(nsACString const& line) {
-  nsresult rv = NS_OK;
+  StartNewOfflineMessage();  // Sets up m_tempMessageStream et al.
   if (m_tempMessageStream) {
-    m_numOfflineMsgLines++;
+    m_numOfflineMsgLines += data.CountChar('\n');
+
     uint32_t count = 0;
-    rv = m_tempMessageStream->Write(line.BeginReading(), line.Length(), &count);
-    NS_ENSURE_SUCCESS(rv, rv);
+    rv = m_tempMessageStream->Write(data.BeginReading(), data.Length(), &count);
     m_tempMessageStreamBytesWritten += count;
-  }
-
-  return rv;
-}
-
-NS_IMETHODIMP nsMsgNewsFolder::NotifyDownloadEnd(nsresult status) {
-  if (m_tempMessageStream) {
-    return EndNewOfflineMessage(status);
+    return EndNewOfflineMessage(rv);
   }
   return NS_OK;
 }