Clean up FTPNetworkTransaction to match the latest trace

net/ftp/ftp_network_transaction.cc

Index: net/ftp/ftp_network_transaction.cc
diff --git a/net/ftp/ftp_network_transaction.cc b/net/ftp/ftp_network_transaction.cc
index 2b308c7eed4cddf225b90b270d4554a89bd03f8e..074df3eae2f888de6f9dece3fb937e9f7658614f 100644
--- a/net/ftp/ftp_network_transaction.cc
+++ b/net/ftp/ftp_network_transaction.cc
@@ -239,11 +239,6 @@ int FtpNetworkTransaction::Stop(int error) {
   return OK;
 }
 
-int FtpNetworkTransaction::RestartIgnoringLastError(

[xunjieli, 2015/01/30 21:50:44]

This is never used anywhere. Is it okay to remove it?

[davidben, 2015/02/04 18:51:18]

Sounds good. :-)

-    const CompletionCallback& callback) {
-  return ERR_NOT_IMPLEMENTED;
-}
-
 int FtpNetworkTransaction::Start(const FtpRequestInfo* request_info,
                                  const CompletionCallback& callback,
                                  const BoundNetLog& net_log) {
@@ -1033,16 +1028,18 @@ int FtpNetworkTransaction::DoCtrlWriteRETR() {
 
 int FtpNetworkTransaction::ProcessResponseRETR(
     const FtpCtrlResponse& response) {
+  // Resource type should be either filled in by DetectTypecode() or
+  // detected with CWD. RETR is sent only when the resource is a file.
+  DCHECK_EQ(RESOURCE_TYPE_FILE, resource_type_);
+
   switch (GetErrorClass(response.status_code)) {
     case ERROR_CLASS_INITIATED:
       // We want the client to start reading the response at this point.
       // It got here either through Start or RestartWithAuth. We want that
       // method to complete. Not setting next state here will make DoLoop exit
       // and in turn make Start/RestartWithAuth complete.
-      resource_type_ = RESOURCE_TYPE_FILE;
       break;
     case ERROR_CLASS_OK:
-      resource_type_ = RESOURCE_TYPE_FILE;
       next_state_ = STATE_CTRL_WRITE_QUIT;
       break;
     case ERROR_CLASS_INFO_NEEDED:
@@ -1056,10 +1053,6 @@ int FtpNetworkTransaction::ProcessResponseRETR(
       return Stop(ERR_UNEXPECTED);
   }
 
-  // We should be sure about our resource type now. Otherwise we risk
-  // an infinite loop (RETR can later send CWD, and CWD can later send RETR).
-  DCHECK_NE(RESOURCE_TYPE_UNKNOWN, resource_type_);
-
   return OK;
 }
 
@@ -1119,13 +1112,14 @@ int FtpNetworkTransaction::DoCtrlWriteCWD() {
 }
 
 int FtpNetworkTransaction::ProcessResponseCWD(const FtpCtrlResponse& response) {
-  // We should never issue CWD if we know the target resource is a file.
+  // CWD should be invoked only when the resource is not a file.
   DCHECK_NE(RESOURCE_TYPE_FILE, resource_type_);
 
   switch (GetErrorClass(response.status_code)) {
     case ERROR_CLASS_INITIATED:
       return Stop(ERR_INVALID_RESPONSE);
     case ERROR_CLASS_OK:
+      resource_type_ = RESOURCE_TYPE_DIRECTORY;
       EstablishDataConnection(STATE_CTRL_WRITE_LIST);
       break;
     case ERROR_CLASS_INFO_NEEDED:
@@ -1158,9 +1152,7 @@ int FtpNetworkTransaction::ProcessResponseCWDNotADirectory() {
     return Stop(ERR_FILE_NOT_FOUND);
   }
 
-  // We are here because SIZE failed and we are not sure what the resource
-  // type is. It could still be file, and SIZE could fail because of
-  // an access error (http://crbug.com/56734). Try RETR just to be sure.
+  // If it is not a directory, it is probably a file.
   resource_type_ = RESOURCE_TYPE_FILE;
 
   EstablishDataConnection(STATE_CTRL_WRITE_RETR);
@@ -1182,6 +1174,10 @@ int FtpNetworkTransaction::DoCtrlWriteLIST() {
 
 int FtpNetworkTransaction::ProcessResponseLIST(
     const FtpCtrlResponse& response) {
+  // Resource type should be either filled in by DetectTypecode() or
+  // detected with CWD. LIST is sent only when the resource is a directory.
+  DCHECK_EQ(RESOURCE_TYPE_DIRECTORY, resource_type_);
+
   switch (GetErrorClass(response.status_code)) {
     case ERROR_CLASS_INITIATED:
       // We want the client to start reading the response at this point.