Clean up FTP code a bit:

- remove unneeded states
- minor style and error handling improvements

This change removes ACCT command support. It was really untested, and I think it wouldn't work in case it was needed. We may add it back when we get a good test case. However, people are unlikely to use a web browser to access servers that require usage of ACCT command.

TEST=net_unittests
BUG=none

Review URL: http://codereview.chromium.org/2985004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@52266 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 12 insertions, 78 deletions

diff --git a/net/ftp/ftp_network_transaction.cc b/net/ftp/ftp_network_transaction.cc
index 123e399..83e2997 100644
--- a/net/ftp/ftp_network_transaction.cc
+++ b/net/ftp/ftp_network_transaction.cc
@@ -1,6 +1,6 @@
-// Copyright (c) 2010 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.
+// Copyright (c) 2010 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"
 
@@ -150,7 +150,7 @@ int FtpNetworkTransaction::Start(const FtpRequestInfo* request_info,
 
   DetectTypecode();
 
-  next_state_ = STATE_CTRL_INIT;
+  next_state_ = STATE_CTRL_RESOLVE_HOST;
   int rv = DoLoop(OK);
   if (rv == ERR_IO_PENDING)
     user_callback_ = callback;
@@ -174,7 +174,7 @@ int FtpNetworkTransaction::RestartWithAuth(const std::wstring& username,
   username_ = username;
   password_ = password;
 
-  next_state_ = STATE_CTRL_INIT;
+  next_state_ = STATE_CTRL_RESOLVE_HOST;
   int rv = DoLoop(OK);
   if (rv == ERR_IO_PENDING)
     user_callback_ = callback;
@@ -301,9 +301,6 @@ int FtpNetworkTransaction::ProcessCtrlResponse() {
     case COMMAND_PASS:
       rv = ProcessResponsePASS(response);
       break;
-    case COMMAND_ACCT:
-      rv = ProcessResponseACCT(response);
-      break;
     case COMMAND_SYST:
       rv = ProcessResponseSYST(response);
       break;
@@ -462,13 +459,6 @@ int FtpNetworkTransaction::DoLoop(int result) {
     State state = next_state_;
     next_state_ = STATE_NONE;
     switch (state) {
-      case STATE_CTRL_INIT:
-        DCHECK(rv == OK);
-        rv = DoCtrlInit();
-        break;
-      case STATE_CTRL_INIT_COMPLETE:
-        rv = DoCtrlInitComplete(rv);
-        break;
       case STATE_CTRL_RESOLVE_HOST:
         DCHECK(rv == OK);
         rv = DoCtrlResolveHost();
@@ -509,10 +499,6 @@ int FtpNetworkTransaction::DoLoop(int result) {
         DCHECK(rv == OK);
         rv = DoCtrlWriteSYST();
         break;
-      case STATE_CTRL_WRITE_ACCT:
-        DCHECK(rv == OK);
-        rv = DoCtrlWriteACCT();
-        break;
       case STATE_CTRL_WRITE_PWD:
         DCHECK(rv == OK);
         rv = DoCtrlWritePWD();
@@ -576,17 +562,6 @@ int FtpNetworkTransaction::DoLoop(int result) {
   return rv;
 }
 
-// TODO(ibrar): Yet to see if we need any intialization
-int FtpNetworkTransaction::DoCtrlInit() {
-  next_state_ = STATE_CTRL_INIT_COMPLETE;
-  return OK;
-}
-
-int FtpNetworkTransaction::DoCtrlInitComplete(int result) {
-  next_state_ = STATE_CTRL_RESOLVE_HOST;
-  return OK;
-}
-
 int FtpNetworkTransaction::DoCtrlResolveHost() {
   next_state_ = STATE_CTRL_RESOLVE_HOST_COMPLETE;
 
@@ -728,8 +703,7 @@ int FtpNetworkTransaction::ProcessResponsePASS(
       next_state_ = STATE_CTRL_WRITE_SYST;
       break;
     case ERROR_CLASS_INFO_NEEDED:
-      next_state_ = STATE_CTRL_WRITE_ACCT;
-      break;
+      return Stop(ERR_FAILED);
     case ERROR_CLASS_TRANSIENT_ERROR:
       return Stop(ERR_FAILED);
     case ERROR_CLASS_PERMANENT_ERROR:
@@ -876,34 +850,6 @@ int FtpNetworkTransaction::ProcessResponseTYPE(
   return OK;
 }
 
-// ACCT command.
-int FtpNetworkTransaction::DoCtrlWriteACCT() {
-  std::string command = "ACCT noaccount";
-  next_state_ = STATE_CTRL_READ;
-  return SendFtpCommand(command, COMMAND_ACCT);
-}
-
-int FtpNetworkTransaction::ProcessResponseACCT(
-    const FtpCtrlResponse& response) {
-  switch (GetErrorClass(response.status_code)) {
-    case ERROR_CLASS_INITIATED:
-      return Stop(ERR_INVALID_RESPONSE);
-    case ERROR_CLASS_OK:
-      next_state_ = STATE_CTRL_WRITE_SYST;
-      break;
-    case ERROR_CLASS_INFO_NEEDED:
-      return Stop(ERR_INVALID_RESPONSE);
-    case ERROR_CLASS_TRANSIENT_ERROR:
-      return Stop(ERR_FAILED);
-    case ERROR_CLASS_PERMANENT_ERROR:
-      return Stop(ERR_FAILED);
-    default:
-      NOTREACHED();
-      return Stop(ERR_UNEXPECTED);
-  }
-  return OK;
-}
-
 // EPSV command
 int FtpNetworkTransaction::DoCtrlWriteEPSV() {
   const std::string command = "EPSV";
@@ -1039,13 +985,9 @@ int FtpNetworkTransaction::ProcessResponseRETR(
       next_state_ = STATE_CTRL_WRITE_QUIT;
       break;
     case ERROR_CLASS_INFO_NEEDED:
-      next_state_ = use_epsv_ ? STATE_CTRL_WRITE_EPSV : STATE_CTRL_WRITE_PASV;
-      break;
+      return Stop(ERR_FAILED);
     case ERROR_CLASS_TRANSIENT_ERROR:
-      if (response.status_code == 421 || response.status_code == 425 ||
-          response.status_code == 426)
-        return Stop(ERR_FAILED);
-      return ERR_FAILED;  // TODO(ibrar): Retry here.
+      return Stop(ERR_FAILED);
     case ERROR_CLASS_PERMANENT_ERROR:
       // Code 550 means "Failed to open file". Other codes are unrelated,
       // like "Not logged in" etc.
diff --git a/net/ftp/ftp_network_transaction.h b/net/ftp/ftp_network_transaction.h
index 23a53cbc..66724c6 100644
--- a/net/ftp/ftp_network_transaction.h
+++ b/net/ftp/ftp_network_transaction.h
@@ -1,6 +1,6 @@
-// Copyright (c) 2008 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.
+// Copyright (c) 2010 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.
 
 #ifndef NET_FTP_FTP_NETWORK_TRANSACTION_H_
 #define NET_FTP_FTP_NETWORK_TRANSACTION_H_
@@ -50,7 +50,6 @@ class FtpNetworkTransaction : public FtpTransaction {
     COMMAND_NONE,
     COMMAND_USER,
     COMMAND_PASS,
-    COMMAND_ACCT,
     COMMAND_SYST,
     COMMAND_TYPE,
     COMMAND_EPSV,
@@ -142,8 +141,6 @@ class FtpNetworkTransaction : public FtpTransaction {
   // argument receive the result from the previous state.  If a method returns
   // ERR_IO_PENDING, then the result from OnIOComplete will be passed to the
   // next state method as the result arg.
-  int DoCtrlInit();
-  int DoCtrlInitComplete(int result);
   int DoCtrlResolveHost();
   int DoCtrlResolveHostComplete(int result);
   int DoCtrlConnect();
@@ -156,8 +153,6 @@ class FtpNetworkTransaction : public FtpTransaction {
   int ProcessResponseUSER(const FtpCtrlResponse& response);
   int DoCtrlWritePASS();
   int ProcessResponsePASS(const FtpCtrlResponse& response);
-  int DoCtrlWriteACCT();
-  int ProcessResponseACCT(const FtpCtrlResponse& response);
   int DoCtrlWriteSYST();
   int ProcessResponseSYST(const FtpCtrlResponse& response);
   int DoCtrlWritePWD();
@@ -250,8 +245,6 @@ class FtpNetworkTransaction : public FtpTransaction {
 
   enum State {
     // Control connection states:
-    STATE_CTRL_INIT,
-    STATE_CTRL_INIT_COMPLETE,
     STATE_CTRL_RESOLVE_HOST,
     STATE_CTRL_RESOLVE_HOST_COMPLETE,
     STATE_CTRL_CONNECT,
@@ -262,7 +255,6 @@ class FtpNetworkTransaction : public FtpTransaction {
     STATE_CTRL_WRITE_COMPLETE,
     STATE_CTRL_WRITE_USER,
     STATE_CTRL_WRITE_PASS,
-    STATE_CTRL_WRITE_ACCT,
     STATE_CTRL_WRITE_SYST,
     STATE_CTRL_WRITE_TYPE,
     STATE_CTRL_WRITE_EPSV,
diff --git a/net/ftp/ftp_network_transaction_unittest.cc b/net/ftp/ftp_network_transaction_unittest.cc
index 1ca3253..137b4a5 100644
--- a/net/ftp/ftp_network_transaction_unittest.cc
+++ b/net/ftp/ftp_network_transaction_unittest.cc
@@ -36,9 +36,9 @@ class FtpSocketDataProvider : public DynamicSocketDataProvider {
     PRE_SYST,
     PRE_PWD,
     PRE_TYPE,
+    PRE_SIZE,
     PRE_EPSV,
     PRE_PASV,
-    PRE_SIZE,
     PRE_MLSD,
     PRE_LIST,
     PRE_RETR,