Bug 1661955 - Followup. Add some comments on NNTP implementation. r=mkmelin

mailnews/news/src/nsNNTPProtocol.cpp

@@ -1977,6 +1977,11 @@ nsresult nsNNTPProtocol::BeginArticle() {
   return NS_OK;
 }
 
+/**
+ * Handler for NNTP_READ_ARTICLE state when being used as an nsIChannel
+ * (e.g. for displaying an article in a nsDocShell). The nsIChannel
+ * counterpart of ReadArticle().
+ */
 nsresult nsNNTPProtocol::DisplayArticle(nsIInputStream* inputStream,
                                         uint32_t length) {
   uint32_t line_length = 0;
@@ -2034,6 +2039,11 @@ nsresult nsNNTPProtocol::DisplayArticle(nsIInputStream* inputStream,
   return NS_OK;
 }
 
+/**
+ * Handler for NNTP_READ_ARTICLE state.
+ * If we're running as an nsIChannel, defers handling to DisplayArticle()
+ * instead.
+ */
 nsresult nsNNTPProtocol::ReadArticle(nsIInputStream* inputStream,
                                      uint32_t length) {
   uint32_t status = 0;

mailnews/news/src/nsNNTPProtocol.h

@@ -125,6 +125,16 @@ typedef enum _StatesEnum {
 
 class nsICacheEntry;
 
+/**
+ * nsNNTPProtocol handles an NNTP connection. It manages the socket
+ * communications and state machine for issuing NNTP commands and parsing the
+ * responses.
+ * For some operations it can also be used as a nsIChannel (for example, to
+ * directly feed an article out to a nsDocShell for display).
+ * It cooperates with nsNntpMockChannel. When a command is completed, it
+ * informs the nsNntpIncomingServer by calling PrepareForNextUrl(), which
+ * sets up the next queued nsNntpMockChannel.
+ */
 class nsNNTPProtocol : public nsMsgProtocol,
                        public nsINNTPProtocol,
                        public nsITimerCallback,
@@ -207,8 +217,11 @@ class nsNNTPProtocol : public nsMsgProtocol,
   nsCOMPtr<nsIMsgNewsFolder> m_newsFolder;
   nsCOMPtr<nsIMsgWindow> m_msgWindow;
 
+  // Pipe endpoints when nsNNTPProtocol is being used as an nsIChannel.
+  // Used to stream article data onward to the consumer (in m_channelListener).
   nsCOMPtr<nsIAsyncInputStream> mDisplayInputStream;
   nsCOMPtr<nsIAsyncOutputStream> mDisplayOutputStream;
+
   RefPtr<nsMsgLineStreamBuffer>
       m_lineStreamBuffer;  // used to efficiently extract lines from the
                            // incoming data stream

mailnews/news/src/nsNntpIncomingServer.cpp

@@ -485,6 +485,11 @@ nsresult nsNntpIncomingServer::CreateProtocolInstance(
   return rv;
 }
 
+/**
+ * Find an available nsNNTPConnection. Can create new connections as long as
+ * we stay under the maximum connection limit.
+ * If none are available, returns NS_OK and nullptr.
+ */
 nsresult nsNntpIncomingServer::GetNntpConnection(
     nsIURI* aUri, nsIMsgWindow* aMsgWindow, nsINNTPProtocol** aNntpConnection) {
   int32_t maxConnections;
@@ -528,6 +533,11 @@ nsresult nsNntpIncomingServer::GetNntpConnection(
   return (*aNntpConnection)->Initialize(aUri, aMsgWindow);
 }
 
+/**
+ * Returns an nsIChannel to run the given URI.
+ * The returned channel might be either an nsNNTPProtocol or nsNntpMockChannel,
+ * representing a real connection or a queued command, respectively.
+ */
 NS_IMETHODIMP
 nsNntpIncomingServer::GetNntpChannel(nsIURI* aURI, nsIMsgWindow* aMsgWindow,
                                      nsIChannel** aChannel) {
@@ -547,6 +557,10 @@ nsNntpIncomingServer::GetNntpChannel(nsIURI* aURI, nsIMsgWindow* aMsgWindow,
   return NS_OK;
 }
 
+/**
+ * Submits a news URI to run. If no connections are free, the command will
+ * be queued to run when one becomes available.
+ */
 NS_IMETHODIMP
 nsNntpIncomingServer::LoadNewsUrl(nsIURI* aURI, nsIMsgWindow* aMsgWindow,
                                   nsISupports* aConsumer) {
@@ -578,6 +592,10 @@ nsNntpIncomingServer::LoadNewsUrl(nsIURI* aURI, nsIMsgWindow* aMsgWindow,
   return NS_OK;
 }
 
+/**
+ * Called when an nsNNTPProtocol finishes running a command and is ready to be
+ * assigned a new one.
+ */
 NS_IMETHODIMP
 nsNntpIncomingServer::PrepareForNextUrl(nsNNTPProtocol* aConnection) {
   NS_ENSURE_ARG(aConnection);

mailnews/news/src/nsNntpIncomingServer.h

@@ -84,7 +84,11 @@ class nsNntpIncomingServer : public nsMsgIncomingServer,
   nsresult CreateProtocolInstance(nsINNTPProtocol** aNntpConnection,
                                   nsIURI* url, nsIMsgWindow* window);
   bool ConnectionTimeOut(nsINNTPProtocol* aNntpConnection);
+
+  // The pool of current connections (up to 'max_cached_connections').
   nsCOMArray<nsINNTPProtocol> mConnectionCache;
+
+  // Pending requests which couldn't immediately be handled by a connection.
   nsTArray<RefPtr<nsNntpMockChannel> > m_queuedChannels;
 
   /**

mailnews/news/src/nsNntpMockChannel.cpp

@@ -12,17 +12,21 @@
 #include "nsIInputStream.h"
 #include "nsContentSecurityManager.h"
 
-NS_IMPL_ISUPPORTS_INHERITED(nsNntpMockChannel, nsHashPropertyBag, nsIChannel, nsIRequest)
+NS_IMPL_ISUPPORTS_INHERITED(nsNntpMockChannel, nsHashPropertyBag, nsIChannel,
+                            nsIRequest)
 
+// For use as an nsIChannel.
 nsNntpMockChannel::nsNntpMockChannel(nsIURI* aUri, nsIMsgWindow* aMsgWindow)
     : m_url(aUri),
       m_msgWindow(aMsgWindow),
+      // We'll be expecting the recipient to open us (via AsyncOpen()).
       m_channelState(CHANNEL_UNOPENED),
       m_protocol(nullptr),
       m_cancelStatus(NS_OK),
       m_loadFlags(0),
       m_contentLength(-1) {}
 
+// For LoadUrl() use.
 nsNntpMockChannel::nsNntpMockChannel(nsIURI* aUri, nsIMsgWindow* aMsgWindow,
                                      nsISupports* aConsumer)
     : m_url(aUri),
@@ -282,6 +286,10 @@ NS_IMETHODIMP nsNntpMockChannel::AsyncOpen(nsIStreamListener* aListener) {
   return NS_OK;
 }
 
+/**
+ * The nsNntpIncomingServer calls this when a real connection (nsNNTPProtocol)
+ * becomes available.
+ */
 nsresult nsNntpMockChannel::AttachNNTPConnection(nsNNTPProtocol& protocol) {
   // First things first. Were we canceled? If so, tell the protocol.
   if (m_channelState == CHANNEL_CLOSED || m_channelState == CHANNEL_UNOPENED)

mailnews/news/src/nsNntpMockChannel.h

@@ -15,14 +15,35 @@
 
 class nsNNTPProtocol;
 
-class nsNntpMockChannel : public nsIChannel,
-                          public nsHashPropertyBag {
+/**
+ * nsNntpMockChannel is used to queue up NNTP operations when no connection
+ * is available for immediate use.
+ * It handles two distinct types of queued operation:
+ * 1) non nsIChannel-based commands, issued via nsNNTPProtocol::LoadNewsUrl().
+ * 2) nsIChannel operations. These are a little trickier, as the recipient
+ *    expects the nsNntpMockChannel to follow the standard lifecycle of a
+ *    nsIChannel, even though the bulk of the work is being passed over
+ *    to a persistent, reusable nsNNTPProtocol object. So there is a degree
+ *    of faking OnStartRequest/OnStopRequest nsIStreamListener callbacks to
+ *    make this nsNntpMockChannel/nsNNTPProtocol amalgam act like a normal
+ *    nsIChannel.
+ *
+ *  The different uses are determined by which constructor is used.
+ */
+class nsNntpMockChannel : public nsIChannel, public nsHashPropertyBag {
  public:
   NS_DECL_ISUPPORTS_INHERITED
   NS_DECL_NSICHANNEL
   NS_DECL_NSIREQUEST
 
+  /**
+   * Create a mockchannel for use as a nsIChannel.
+   */
   nsNntpMockChannel(nsIURI* aUri, nsIMsgWindow* aMsgWindow);
+
+  /**
+   * Create a mockchannel for deferred LoadUrl() use.
+   */
   nsNntpMockChannel(nsIURI* aUri, nsIMsgWindow* aMsgWindow,
                     nsISupports* aConsumer);