Fix hitting a DCHECK in FtpNetworkTransaction on extra responses.

net/ftp/ftp_network_transaction.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/ftp/ftp_network_transaction.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/compiler_specific.h"
 #include "base/metrics/histogram_macros.h"
@@ skipping 399 matching lines @@
       break;
     case COMMAND_QUIT:
       rv = ProcessResponseQUIT(response);
       break;
     default:
       LOG(DFATAL) << "Unexpected value of command_sent_: " << command_sent_;
       return ERR_UNEXPECTED;
   }
 
   // We may get multiple responses for some commands,
-  // see http://crbug.com/18036.
+  // see http://crbug.com/18036. Consume all responses, regardless of whether
+  // they make sense. On unexpected responses, SendFtpCommand expects all data
+  // to have already been consumed, even when sending the QUIT command.

[mmenke, 2016/11/28 17:28:22]

Another option would be to skip that DCHECK in case of a quit message, which
would mean we wouldn't wait for the response for the quit command, but only if
we received 2 or more unexpected messages, which seems weird (We should either
always wait for the quit response or never do it, when we get bonus messages,
regardless of how many we get).

   while (ctrl_response_buffer_->ResponseAvailable() && rv == OK) {
     response = ctrl_response_buffer_->PopResponse();
 
     switch (command_sent_) {
       case COMMAND_RETR:
         rv = ProcessResponseRETR(response);
         break;
       case COMMAND_LIST:
         rv = ProcessResponseLIST(response);
         break;
       default:
         // Multiple responses for other commands are invalid.
-        return Stop(ERR_INVALID_RESPONSE);
+        rv = Stop(ERR_INVALID_RESPONSE);

[eroman, 2016/11/28 17:51:24]

I am not sure I understand this code flow.

rv will be OK in every case except when the sent command was COMMAND_QUIT.
But maybe I am misunderstanding the role of Stop()

[mmenke, 2016/11/28 18:21:01]

Stop sets the next state to send the quit message, so multiple calls don't cause
problems.  If we've already sent a quit messages, it just returns the error
synchronously, and we exit the loop, but don't DCHECK, because we don't send the
quit message again.

+        break;
     }
   }
 
   return rv;
 }
 
 // Used to prepare and send FTP command.
 int FtpNetworkTransaction::SendFtpCommand(const std::string& command,
                                           const std::string& command_for_log,
                                           Command cmd) {
@@ skipping 899 matching lines @@
   if (!had_error_type[type]) {
     had_error_type[type] = true;
     UMA_HISTOGRAM_ENUMERATION("Net.FtpDataConnectionErrorHappened",
         type, NUM_OF_NET_ERROR_TYPES);
   }
   UMA_HISTOGRAM_ENUMERATION("Net.FtpDataConnectionErrorCount",
       type, NUM_OF_NET_ERROR_TYPES);
 }
 
 }  // namespace net