Re-land: Defer SpdySession destruction to support closing writes

net/spdy/spdy_session.h

Index: net/spdy/spdy_session.h
diff --git a/net/spdy/spdy_session.h b/net/spdy/spdy_session.h
index 51ed3f27aa1ae8c0cbbfb5b62ee4676185e1fa3b..d2e07ac90a26b4aad83c0b517f9d25b4659598e8 100644
--- a/net/spdy/spdy_session.h
+++ b/net/spdy/spdy_session.h
@@ -343,12 +343,10 @@ class NET_EXPORT SpdySession : public BufferedSpdyFramerVisitorInterface,
   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.
-  //
-  // TODO(akalin): This is only used in tests. Remove this function
-  // and have tests test the WeakPtr instead.
-  bool IsClosed() const { return availability_state_ == STATE_CLOSED; }
+  // Accessors for the session's availability state.
+  bool IsAvailable() const { return availability_state_ == STATE_AVAILABLE; }
+  bool IsGoingAway() const { return availability_state_ == STATE_GOING_AWAY; }
+  bool IsDraining() const { return availability_state_ == STATE_DRAINING; }
 
   // Closes this session. This will close all active streams and mark
   // the session as permanently closed. Callers must assume that the
@@ -532,9 +530,11 @@ class NET_EXPORT SpdySession : public BufferedSpdyFramerVisitorInterface,
     // 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
+    // The session is draining its write queue in preparation of closing.
+    // Further writes will not be queued, and further reads will not be issued
+    // (though the remainder of a current read may be processed). The session
+    // will be destroyed by its write loop once the write queue is drained.
+    STATE_DRAINING,
   };
 
   enum ReadState {
@@ -549,18 +549,6 @@ class NET_EXPORT SpdySession : public BufferedSpdyFramerVisitorInterface,
     WRITE_STATE_DO_WRITE_COMPLETE,
   };
 
-  // The return value of DoCloseSession() describing what was done.
-  enum CloseSessionResult {
-    // The session was already closed so nothing was done.
-    SESSION_ALREADY_CLOSED,
-    // The session was moved into the closed state but was not removed
-    // from |pool_| (because we're in an IO loop).
-    SESSION_CLOSED_BUT_NOT_REMOVED,
-    // The session was moved into the closed state and removed from
-    // |pool_|.
-    SESSION_CLOSED_AND_REMOVED,
-  };
-
   // Checks whether a stream for the given |url| can be created or
   // retrieved from the set of unclaimed push streams. Returns OK if
   // so. Otherwise, the session is closed and an error <
@@ -618,11 +606,8 @@ class NET_EXPORT SpdySession : public BufferedSpdyFramerVisitorInterface,
                                SpdyRstStreamStatus status,
                                const std::string& description);
 
-  // Calls DoReadLoop and then if |availability_state_| is
-  // STATE_CLOSED, calls RemoveFromPool().
-  //
-  // Use this function instead of DoReadLoop when posting a task to
-  // pump the read loop.
+  // Calls DoReadLoop. Use this function instead of DoReadLoop when
+  // posting a task to pump the read loop.
   void PumpReadLoop(ReadState expected_read_state, int result);
 
   // Advance the ReadState state machine. |expected_read_state| is the
@@ -634,13 +619,18 @@ class NET_EXPORT SpdySession : public BufferedSpdyFramerVisitorInterface,
   int DoRead();
   int DoReadComplete(int result);
 
-  // Calls DoWriteLoop and then if |availability_state_| is
-  // STATE_CLOSED, calls RemoveFromPool().
+  // Calls DoWriteLoop. If |availability_state_| is STATE_DRAINING and no
+  // writes remain, the session is removed from the session pool and
+  // destroyed.
   //
   // Use this function instead of DoWriteLoop when posting a task to
   // pump the write loop.
   void PumpWriteLoop(WriteState expected_write_state, int result);
 
+  // Iff the write loop is not currently active, posts a callback into
+  // PumpWriteLoop().
+  void MaybePostWriteLoop();
+
   // Advance the WriteState state machine. |expected_write_state| is
   // the expected starting write state.
   //
@@ -743,10 +733,9 @@ class NET_EXPORT SpdySession : public BufferedSpdyFramerVisitorInterface,
   void DcheckGoingAway() const;
 
   // Calls DcheckGoingAway(), then DCHECKs that |availability_state_|
-  // == STATE_CLOSED, |error_on_close_| has a valid value, that there
-  // are no active streams or unclaimed pushed streams, and that the
-  // write queue is empty.
-  void DcheckClosed() const;
+  // == STATE_DRAINING, |error_on_close_| has a valid value, and that there
+  // are no active streams or unclaimed pushed streams.
+  void DcheckDraining() const;
 
   // Closes all active streams with stream id's greater than
   // |last_good_stream_id|, as well as any created or pending
@@ -760,19 +749,9 @@ class NET_EXPORT SpdySession : public BufferedSpdyFramerVisitorInterface,
   // isn't closed yet, close it.
   void MaybeFinishGoingAway();
 
-  // If the stream is already closed, does nothing. Otherwise, moves
-  // the session to a closed state. Then, if we're in an IO loop,
-  // returns (as the IO loop will do the pool removal itself when its
-  // done). Otherwise, also removes |this| from |pool_|. The returned
-  // result describes what was done.
-  CloseSessionResult DoCloseSession(Error err, const std::string& description);
-
-  // Remove this session from its pool, which must exist. Must be
-  // called only when the session is closed.
-  //
-  // Must be called only via Pump{Read,Write}Loop() or
-  // DoCloseSession().
-  void RemoveFromPool();
+  // If the session is already draining, does nothing. Otherwise, moves
+  // the session to the draining state.
+  void DoDrainSession(Error err, const std::string& description);
 
   // Called right before closing a (possibly-inactive) stream for a
   // reason other than being requested to by the stream.
@@ -1017,10 +996,10 @@ class NET_EXPORT SpdySession : public BufferedSpdyFramerVisitorInterface,
   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.
+  // If the session is closing (i.e., |availability_state_| is STATE_DRAINING),
+  // then |error_on_close_| holds the error with which it was closed, which
+  // may be OK (upon a polite GOAWAY) or an error < ERR_IO_PENDING otherwise.
+  // Initialized to OK.
   Error error_on_close_;
 
   // Limits