[SPDY] Refactor SpdySession state machine

Represent a SpdySession's state by three variables of type:
AvailabilityState, ReadState (the old State type), and WriteState.

Make some important functions, including the BufferedSpdyWriter
callbacks, do nothing if availability_state_ == STATE_CLOSED.

Rename the current state machine from DoLoop etc. to DoReadLoop.
Refactor the write state machine to be parallel to the read state machine.

Keep track of the number of bytes read without yielding in DoReadLoop
itself. This fixes a slight bug where the counter isn't reset if the loop
yields due to ERR_IO_PENDING being returned.

The new write state machine (DoWriteLoop, etc.) has almost the same
behavior as the old one, except if a write completes asynchronously
the write loop is immediately re-entered instead of via a posted task.

Fix tests to match new write loop behavior.

BUG=255701
R=rch@chromium.org, rtenneti@chromium.org

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

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

1 files changed, 48 insertions, 45 deletions

diff --git a/net/spdy/spdy_session.h b/net/spdy/spdy_session.h
index c2c6cfd..0cca912 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 number of bytes read synchronously (without yielding) if the
-// data is available.
-const int kMaxReadBytes = 32 * 1024;
+// Specifies the maximum number of bytes to read synchronously before
+// yielding.
+const int kMaxReadBytesWithoutYielding = 32 * 1024;
 
 // The initial receive window size for both streams and sessions.
 const int32 kDefaultInitialRecvWindowSize = 10 * 1024 * 1024;  // 10MB
@@ -336,8 +336,9 @@ class NET_EXPORT SpdySession : public base::RefCounted<SpdySession>,
   void SendStreamWindowUpdate(SpdyStreamId stream_id,
                               uint32 delta_window_size);
 
-  // If session is closed, no new streams/transactions should be created.
-  bool IsClosed() const { return state_ == STATE_CLOSED; }
+  // 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; }
 
   // Closes this session.  This will close all active streams and mark
   // the session as permanently closed.
@@ -386,7 +387,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(int priority) const {
+  size_t pending_create_stream_queue_size(RequestPriority priority) const {
     DCHECK_LT(priority, NUM_PRIORITIES);
     return pending_create_stream_queues_[priority].size();
   }
@@ -501,14 +502,28 @@ class NET_EXPORT SpdySession : public base::RefCounted<SpdySession>,
 
   typedef std::set<SpdyStream*> CreatedStreamSet;
 
-  enum State {
-    STATE_IDLE,
-    STATE_CONNECTING,
-    STATE_DO_READ,
-    STATE_DO_READ_COMPLETE,
+  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.
     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
@@ -554,24 +569,19 @@ class NET_EXPORT SpdySession : public base::RefCounted<SpdySession>,
                             SpdyRstStreamStatus status,
                             const std::string& description);
 
-  // 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.
+  // 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.
   int DoRead();
-  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;
-  }
+  int DoReadComplete(int result);
 
-  // IO Callbacks
-  void OnReadComplete(int result);
-  void OnWriteComplete(int result);
+  // 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);
 
   // Send relevant SETTINGS.  This is generally called on connection setup.
   void SendInitialSettings();
@@ -607,10 +617,6 @@ 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();
 
@@ -884,8 +890,7 @@ 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_|.
@@ -896,9 +901,6 @@ 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_;
 
@@ -908,11 +910,16 @@ class NET_EXPORT SpdySession : public base::RefCounted<SpdySession>,
   // Spdy Frame state.
   scoped_ptr<BufferedSpdyFramer> buffered_spdy_framer_;
 
-  // 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_;
+  // 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_;
 
   // Limits
   size_t max_concurrent_streams_;  // 0 if no limit
@@ -928,10 +935,6 @@ 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.