Revert "[SPDY] Refactor SpdySession state machine"

This causes spurious page load failures for SPDY sites.

Will reland with regression tests for the introduced bug.

BUG=255701,261043
TBR=rch@chromium.org,rtenneti@chromium.org

Review URL: https://codereview.chromium.org/19567002

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

1 files changed, 45 insertions, 48 deletions

diff --git a/net/spdy/spdy_session.h b/net/spdy/spdy_session.h
index 0cca912..c2c6cfd 100644
--- a/net/spdy/spdy_session.h
+++ b/net/spdy/spdy_session.h
@@ -51,9 +51,9 @@ const int kMaxSpdyFrameChunkSize = (2 * kMss) - 8;
 // Specifies the maxiumum concurrent streams server could send (via push).
 const int kMaxConcurrentPushedStreams = 1000;
 
-// Specifies the maximum number of bytes to read synchronously before
-// yielding.
-const int kMaxReadBytesWithoutYielding = 32 * 1024;
+// Specifies the number of bytes read synchronously (without yielding) if the
+// data is available.
+const int kMaxReadBytes = 32 * 1024;
 
 // The initial receive window size for both streams and sessions.
 const int32 kDefaultInitialRecvWindowSize = 10 * 1024 * 1024;  // 10MB
@@ -336,9 +336,8 @@ class NET_EXPORT SpdySession : public base::RefCounted<SpdySession>,
   void SendStreamWindowUpdate(SpdyStreamId stream_id,
                               uint32 delta_window_size);
 
-  // Whether the stream is closed, i.e. it has stopped processing data
-  // and is about to be destroyed.
-  bool IsClosed() const { return availability_state_ == STATE_CLOSED; }
+  // If session is closed, no new streams/transactions should be created.
+  bool IsClosed() const { return state_ == STATE_CLOSED; }
 
   // Closes this session.  This will close all active streams and mark
   // the session as permanently closed.
@@ -387,7 +386,7 @@ class NET_EXPORT SpdySession : public base::RefCounted<SpdySession>,
   }
   size_t num_created_streams() const { return created_streams_.size(); }
 
-  size_t pending_create_stream_queue_size(RequestPriority priority) const {
+  size_t pending_create_stream_queue_size(int priority) const {
     DCHECK_LT(priority, NUM_PRIORITIES);
     return pending_create_stream_queues_[priority].size();
   }
@@ -502,28 +501,14 @@ class NET_EXPORT SpdySession : public base::RefCounted<SpdySession>,
 
   typedef std::set<SpdyStream*> CreatedStreamSet;
 
-  enum AvailabilityState {
-    // The session is available in its socket pool and can be used
-    // freely.
-    STATE_AVAILABLE,
-    // The session can process data on existing streams but will
-    // refuse to create new ones.
-    STATE_GOING_AWAY,
-    // The session has been closed, is waiting to be deleted, and will
-    // refuse to process any more data.
+  enum State {
+    STATE_IDLE,
+    STATE_CONNECTING,
+    STATE_DO_READ,
+    STATE_DO_READ_COMPLETE,
     STATE_CLOSED
   };
 
-  enum ReadState {
-    READ_STATE_DO_READ,
-    READ_STATE_DO_READ_COMPLETE,
-  };
-
-  enum WriteState {
-    WRITE_STATE_DO_WRITE,
-    WRITE_STATE_DO_WRITE_COMPLETE,
-  };
-
   virtual ~SpdySession();
 
   // Called by SpdyStreamRequest to start a request to create a
@@ -569,19 +554,24 @@ class NET_EXPORT SpdySession : public base::RefCounted<SpdySession>,
                             SpdyRstStreamStatus status,
                             const std::string& description);
 
-  // Advance the ReadState state machine. |expected_read_state| is the
-  // expected starting read state.
-  int DoReadLoop(ReadState expected_read_state, int result);
-  // The implementations of the states of the ReadState state machine.
+  // Start the DoLoop to read data from socket.
+  void StartRead();
+
+  // Try to make progress by reading and processing data.
+  int DoLoop(int result);
+  // The implementations of STATE_DO_READ/STATE_DO_READ_COMPLETE state changes
+  // of the state machine.
   int DoRead();
-  int DoReadComplete(int result);
+  int DoReadComplete(int bytes_read);
+
+  // Check if session is connected or not.
+  bool IsConnected() const {
+    return state_ == STATE_DO_READ || state_ == STATE_DO_READ_COMPLETE;
+  }
 
-  // Advance the WriteState state machine. |expected_write_state| is
-  // the expected starting write state.
-  int DoWriteLoop(WriteState expected_write_state, int result);
-  // The implementations of the states of the WriteState state machine.
-  int DoWrite();
-  int DoWriteComplete(int result);
+  // IO Callbacks
+  void OnReadComplete(int result);
+  void OnWriteComplete(int result);
 
   // Send relevant SETTINGS.  This is generally called on connection setup.
   void SendInitialSettings();
@@ -617,6 +607,10 @@ class NET_EXPORT SpdySession : public base::RefCounted<SpdySession>,
   // haven't received any data in |kHungInterval| time period.
   void CheckPingStatus(base::TimeTicks last_check_time);
 
+  // Write current data to the socket.
+  void WriteSocketLater();
+  void WriteSocket();
+
   // Get a new stream id.
   int GetNewStreamId();
 
@@ -890,7 +884,8 @@ class NET_EXPORT SpdySession : public base::RefCounted<SpdySession>,
   SpdyWriteQueue write_queue_;
 
   // Data for the frame we are currently sending.
-
+  // Whether we have a socket write pending completion.
+  bool write_pending_;
   // The buffer we're currently writing.
   scoped_ptr<SpdyBuffer> in_flight_write_;
   // The type of the frame in |in_flight_write_|.
@@ -901,6 +896,9 @@ class NET_EXPORT SpdySession : public base::RefCounted<SpdySession>,
   // the socket completely.
   base::WeakPtr<SpdyStream> in_flight_write_stream_;
 
+  // Flag if we have a pending message scheduled for WriteSocket.
+  bool delayed_write_pending_;
+
   // Flag if we're using an SSL connection for this SpdySession.
   bool is_secure_;
 
@@ -910,16 +908,11 @@ class NET_EXPORT SpdySession : public base::RefCounted<SpdySession>,
   // Spdy Frame state.
   scoped_ptr<BufferedSpdyFramer> buffered_spdy_framer_;
 
-  // The state variables.
-  AvailabilityState availability_state_;
-  ReadState read_state_;
-  WriteState write_state_;
-
-  // If the session was closed (i.e., |availability_state_| is
-  // STATE_CLOSED), then |error_on_close_| holds the error with which
-  // it was closed, which is < ERR_IO_PENDING. Otherwise, it is set to
-  // OK.
-  Error error_on_close_;
+  // If an error has occurred on the session, the session is effectively
+  // dead.  Record this error here.  When no error has occurred, |error_| will
+  // be OK.
+  Error error_;
+  State state_;
 
   // Limits
   size_t max_concurrent_streams_;  // 0 if no limit
@@ -935,6 +928,10 @@ class NET_EXPORT SpdySession : public base::RefCounted<SpdySession>,
   // SpdySession. It is used by the |Net.SpdySettingsCwnd...| histograms.
   int total_bytes_received_;
 
+  // |bytes_read_| keeps track of number of bytes read continously in the
+  // DoLoop() without yielding.
+  int bytes_read_;
+
   bool sent_settings_;      // Did this session send settings when it started.
   bool received_settings_;  // Did this session receive at least one settings
                             // frame.