Revert 181390 "SPDY - implement greedy approach to read all the ..."

net/spdy/spdy_session.cc

Index: net/spdy/spdy_session.cc
===================================================================
--- net/spdy/spdy_session.cc	(revision 192196)
+++ net/spdy/spdy_session.cc	(working copy)
@@ -314,13 +314,14 @@
       http_server_properties_(http_server_properties),
       connection_(new ClientSocketHandle),
       read_buffer_(new IOBuffer(kReadBufferSize)),
+      read_pending_(false),
       stream_hi_water_mark_(kFirstStreamId),
       write_pending_(false),
       delayed_write_pending_(false),
       is_secure_(false),
       certificate_error_code_(OK),
       error_(OK),
-      state_(STATE_IDLE),
+      state_(IDLE),
       max_concurrent_streams_(initial_max_concurrent_streams == 0 ?
                               kInitialMaxConcurrentStreams :
                               initial_max_concurrent_streams),
@@ -331,8 +332,7 @@
       streams_pushed_count_(0),
       streams_pushed_and_claimed_count_(0),
       streams_abandoned_count_(0),
-      total_bytes_received_(0),
-      bytes_read_(0),
+      bytes_received_(0),
       sent_settings_(false),
       received_settings_(false),
       stalled_streams_(0),
@@ -373,8 +373,8 @@
 }
 
 SpdySession::~SpdySession() {
-  if (state_ != STATE_CLOSED) {
-    state_ = STATE_CLOSED;
+  if (state_ != CLOSED) {
+    state_ = CLOSED;
 
     // Cleanup all the streams.
     CloseAllStreams(net::ERR_ABORTED);
@@ -406,7 +406,7 @@
   base::StatsCounter spdy_sessions("spdy.sessions");
   spdy_sessions.Increment();
 
-  state_ = STATE_DO_READ;
+  state_ = CONNECTED;
   connection_.reset(connection);
   is_secure_ = is_secure;
   certificate_error_code_ = certificate_error_code;
@@ -458,17 +458,17 @@
 
   // Write out any data that we might have to send, such as the settings frame.
   WriteSocketLater();
-  int error = DoLoop(OK);
+  net::Error error = ReadSocket();
   if (error == ERR_IO_PENDING)
     return OK;
-  return static_cast<net::Error>(error);
+  return error;
 }
 
 bool SpdySession::VerifyDomainAuthentication(const std::string& domain) {
   if (!verify_domain_authentication_)
     return true;
 
-  if (!IsConnected())
+  if (state_ != CONNECTED)
     return false;
 
   SSLInfo ssl_info;
@@ -496,7 +496,7 @@
     const GURL& url,
     scoped_refptr<SpdyStream>* stream,
     const BoundNetLog& stream_net_log) {
-  CHECK_NE(state_, STATE_CLOSED);
+  CHECK_NE(state_, CLOSED);
 
   *stream = NULL;
 
@@ -866,7 +866,7 @@
 
   // If we're connecting, defer to the connection to give us the actual
   // LoadState.
-  if (state_ == STATE_CONNECTING)
+  if (state_ == CONNECTING)
     return connection_->GetLoadState();
 
   // Just report that we're idle since the session could be doing
@@ -875,97 +875,44 @@
 }
 
 void SpdySession::OnReadComplete(int bytes_read) {
-  DCHECK_NE(state_, STATE_DO_READ);
-  DoLoop(bytes_read);
-}
-
-void SpdySession::StartRead() {
-  DCHECK_NE(state_, STATE_DO_READ_COMPLETE);
-  DoLoop(OK);
-}
-
-int SpdySession::DoLoop(int result) {
-  bytes_read_ = 0;
-
-  // The SpdyFramer will use callbacks onto |this| as it parses frames.
-  // When errors occur, those callbacks can lead to teardown of all references
-  // to |this|, so maintain a reference to self during this call for safe
-  // cleanup.
-  scoped_refptr<SpdySession> self(this);
-
-  do {
-    switch (state_) {
-      case STATE_DO_READ:
-        DCHECK_EQ(result, OK);
-        result = DoRead();
-        break;
-      case STATE_DO_READ_COMPLETE:
-        result = DoReadComplete(result);
-        break;
-      case STATE_CLOSED:
-        result = ERR_CONNECTION_CLOSED;
-        break;
-      default:
-        NOTREACHED() << "state_: " << state_;
-        break;
-    }
-  } while (result != ERR_IO_PENDING && result != ERR_CONNECTION_CLOSED);
-
-  return result;
-}
-
-int SpdySession::DoRead() {
-  if (bytes_read_ > kMaxReadBytes) {
-    state_ = STATE_DO_READ;
-    MessageLoop::current()->PostTask(
-        FROM_HERE,
-        base::Bind(&SpdySession::StartRead,
-                   weak_factory_.GetWeakPtr()));
-    return ERR_IO_PENDING;
-  }
-
-  CHECK(connection_.get());
-  CHECK(connection_->socket());
-  state_ = STATE_DO_READ_COMPLETE;
-  return connection_->socket()->Read(
-      read_buffer_.get(),
-      kReadBufferSize,
-      base::Bind(&SpdySession::OnReadComplete, weak_factory_.GetWeakPtr()));
-}
-
-int SpdySession::DoReadComplete(int result) {
   // Parse a frame.  For now this code requires that the frame fit into our
   // buffer (32KB).
   // TODO(mbelshe): support arbitrarily large frames!
 
-  if (result <= 0) {
+  read_pending_ = false;
+
+  if (bytes_read <= 0) {
     // Session is tearing down.
-    net::Error error = static_cast<net::Error>(result);
-    if (result == 0)
+    net::Error error = static_cast<net::Error>(bytes_read);
+    if (bytes_read == 0)
       error = ERR_CONNECTION_CLOSED;
-    CloseSessionOnError(error, true, "result is <= 0.");
-    return ERR_CONNECTION_CLOSED;
+    CloseSessionOnError(error, true, "bytes_read is <= 0.");
+    return;
   }
 
-  total_bytes_received_ += result;
-  bytes_read_ += result;
+  bytes_received_ += bytes_read;
 
   last_activity_time_ = base::TimeTicks::Now();
 
+  // The SpdyFramer will use callbacks onto |this| as it parses frames.
+  // When errors occur, those callbacks can lead to teardown of all references
+  // to |this|, so maintain a reference to self during this call for safe
+  // cleanup.
+  scoped_refptr<SpdySession> self(this);
+
   DCHECK(buffered_spdy_framer_.get());
   char* data = read_buffer_->data();
-  while (result &&
+  while (bytes_read &&
          buffered_spdy_framer_->error_code() ==
              SpdyFramer::SPDY_NO_ERROR) {
     uint32 bytes_processed =
-        buffered_spdy_framer_->ProcessInput(data, result);
-    result -= bytes_processed;
+        buffered_spdy_framer_->ProcessInput(data, bytes_read);
+    bytes_read -= bytes_processed;
     data += bytes_processed;
   }
 
-  if (IsConnected())
-    state_ = STATE_DO_READ;
-  return OK;
+  if (state_ != CLOSED)
+    ReadSocket();
 }
 
 void SpdySession::OnWriteComplete(int result) {
@@ -1022,11 +969,49 @@
   }
 }
 
+net::Error SpdySession::ReadSocket() {
+  if (read_pending_)
+    return OK;
+
+  if (state_ == CLOSED) {
+    NOTREACHED();
+    return ERR_UNEXPECTED;
+  }
+
+  CHECK(connection_.get());
+  CHECK(connection_->socket());
+  int bytes_read = connection_->socket()->Read(
+      read_buffer_.get(),
+      kReadBufferSize,
+      base::Bind(&SpdySession::OnReadComplete, base::Unretained(this)));
+  switch (bytes_read) {
+    case 0:
+      // Socket is closed!
+      CloseSessionOnError(ERR_CONNECTION_CLOSED, true, "bytes_read is 0.");
+      return ERR_CONNECTION_CLOSED;
+    case net::ERR_IO_PENDING:
+      // Waiting for data.  Nothing to do now.
+      read_pending_ = true;
+      return ERR_IO_PENDING;
+    default:
+      // Data was read, process it.
+      // Schedule the work through the message loop to avoid recursive
+      // callbacks.
+      read_pending_ = true;
+      MessageLoop::current()->PostTask(
+          FROM_HERE,
+          base::Bind(&SpdySession::OnReadComplete,
+                     weak_factory_.GetWeakPtr(), bytes_read));
+      break;
+  }
+  return OK;
+}
+
 void SpdySession::WriteSocketLater() {
   if (delayed_write_pending_)
     return;
 
-  if (!IsConnected())
+  if (state_ < CONNECTED)
     return;
 
   delayed_write_pending_ = true;
@@ -1043,7 +1028,7 @@
   // If the socket isn't connected yet, just wait; we'll get called
   // again when the socket connection completes.  If the socket is
   // closed, just return.
-  if (!IsConnected())
+  if (state_ < CONNECTED || state_ == CLOSED)
     return;
 
   if (write_pending_)   // Another write is in progress still.
@@ -1166,8 +1151,8 @@
   // Don't close twice.  This can occur because we can have both
   // a read and a write outstanding, and each can complete with
   // an error.
-  if (!IsClosed()) {
-    state_ = STATE_CLOSED;
+  if (state_ != CLOSED) {
+    state_ = CLOSED;
     error_ = err;
     if (remove_from_pool)
       RemoveFromPool();
@@ -2121,16 +2106,16 @@
           // for larger volumes of data being sent.
           UMA_HISTOGRAM_CUSTOM_COUNTS("Net.SpdySettingsCwnd",
                                       val, 1, 200, 100);
-          if (total_bytes_received_ > 10 * 1024) {
+          if (bytes_received_ > 10 * 1024) {
             UMA_HISTOGRAM_CUSTOM_COUNTS("Net.SpdySettingsCwnd10K",
                                         val, 1, 200, 100);
-            if (total_bytes_received_ > 25 * 1024) {
+            if (bytes_received_ > 25 * 1024) {
               UMA_HISTOGRAM_CUSTOM_COUNTS("Net.SpdySettingsCwnd25K",
                                           val, 1, 200, 100);
-              if (total_bytes_received_ > 50 * 1024) {
+              if (bytes_received_ > 50 * 1024) {
                 UMA_HISTOGRAM_CUSTOM_COUNTS("Net.SpdySettingsCwnd50K",
                                             val, 1, 200, 100);
-                if (total_bytes_received_ > 100 * 1024) {
+                if (bytes_received_ > 100 * 1024) {
                   UMA_HISTOGRAM_CUSTOM_COUNTS("Net.SpdySettingsCwnd100K",
                                               val, 1, 200, 100);
                 }