[SPDY] Remove a SPDY session from its pool only when not in an IO loop

This is in preparation for making pool removal destroy the SPDY session.

Add an in_io_loop_ variable to SpdySession to help with
the above. Assert its state in various functions.

Move the guts of CloseSessionOnError into DoCloseSession(). Make
DoCloseSession() close the session only if it's not already closed
and remove the session only if it's not in an IO loop. Make it return
a value describing what it did and add assertions for that value
whenever it's called.

Add Pump{Read,Write}Loop() functions that check for
session closure after calling Do{Read,Write}Loop()
and remove from the pool if so.

Some minor test changes to reflect the above.

BUG=255701

Review URL: https://chromiumcodereview.appspot.com/18408008

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

6 files changed, 407 insertions, 127 deletions

diff --git a/net/spdy/spdy_session.cc b/net/spdy/spdy_session.cc
index 95fc4bd..3a56a90 100644
--- a/net/spdy/spdy_session.cc
+++ b/net/spdy/spdy_session.cc
@@ -9,7 +9,6 @@
 
 #include "base/basictypes.h"
 #include "base/bind.h"
-#include "base/bind_helpers.h"
 #include "base/compiler_specific.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
@@ -349,6 +348,7 @@ SpdySession::SpdySession(
     const HostPortPair& trusted_spdy_proxy,
     NetLog* net_log)
     : weak_factory_(this),
+      in_io_loop_(false),
       spdy_session_key_(spdy_session_key),
       pool_(NULL),
       http_server_properties_(http_server_properties),
@@ -413,11 +413,8 @@ SpdySession::SpdySession(
 }
 
 SpdySession::~SpdySession() {
-  // TODO(akalin): CHECK that we're already closed once we're owned by
-  // |pool_|.
-  if (availability_state_ != STATE_CLOSED)
-    DoCloseSession(ERR_ABORTED);
-
+  CHECK(!in_io_loop_);
+  DCHECK(!pool_);
   DcheckClosed();
 
   // TODO(akalin): Check connection->is_initialized() instead. This
@@ -436,9 +433,11 @@ Error SpdySession::InitializeWithSocket(
     SpdySessionPool* pool,
     bool is_secure,
     int certificate_error_code) {
+  CHECK(!in_io_loop_);
   DCHECK(!connection_);
   DCHECK(certificate_error_code == OK ||
          certificate_error_code < ERR_IO_PENDING);
+
   // TODO(akalin): Check connection->is_initialized() instead. This
   // requires re-working CreateFakeSpdySession(), though.
   DCHECK(connection->socket());
@@ -501,9 +500,12 @@ Error SpdySession::InitializeWithSocket(
   if (error == ERR_IO_PENDING)
     error = OK;
   if (error == OK) {
+    DCHECK_NE(availability_state_, STATE_CLOSED);
     connection_->AddLayeredPool(this);
     SendInitialSettings();
     pool_ = pool;
+  } else {
+    DcheckClosed();
   }
   return static_cast<Error>(error);
 }
@@ -532,6 +534,8 @@ int SpdySession::GetPushStream(
     const GURL& url,
     base::WeakPtr<SpdyStream>* stream,
     const BoundNetLog& stream_net_log) {
+  CHECK(!in_io_loop_);
+
   stream->reset();
 
   if (availability_state_ == STATE_CLOSED)
@@ -550,14 +554,18 @@ int SpdySession::GetPushStream(
 }
 
 Error SpdySession::TryAccessStream(const GURL& url) {
+  CHECK(!in_io_loop_);
+  DCHECK_NE(availability_state_, STATE_CLOSED);
+
   if (is_secure_ && certificate_error_code_ != OK &&
       (url.SchemeIs("https") || url.SchemeIs("wss"))) {
     RecordProtocolErrorHistogram(
         PROTOCOL_ERROR_REQUEST_FOR_SECURE_CONTENT_OVER_INSECURE_SESSION);
-    CloseSessionOnError(
+    CloseSessionResult result = DoCloseSession(
         static_cast<Error>(certificate_error_code_),
         "Tried to get SPDY stream for secure content over an unauthenticated "
         "session.");
+    DCHECK_EQ(result, SESSION_CLOSED_AND_REMOVED);
     return ERR_SPDY_PROTOCOL_ERROR;
   }
   return OK;
@@ -566,9 +574,8 @@ Error SpdySession::TryAccessStream(const GURL& url) {
 int SpdySession::TryCreateStream(SpdyStreamRequest* request,
                                  base::WeakPtr<SpdyStream>* stream) {
   CHECK(request);
+  CHECK(!in_io_loop_);
 
-  // TODO(akalin): Also refuse to create the stream when
-  // |availability_state_| == STATE_GOING_AWAY.
   if (availability_state_ == STATE_GOING_AWAY)
     return ERR_FAILED;
 
@@ -593,6 +600,7 @@ int SpdySession::TryCreateStream(SpdyStreamRequest* request,
 
 int SpdySession::CreateStream(const SpdyStreamRequest& request,
                               base::WeakPtr<SpdyStream>* stream) {
+  CHECK(!in_io_loop_);
   DCHECK_GE(request.priority(), MINIMUM_PRIORITY);
   DCHECK_LT(request.priority(), NUM_PRIORITIES);
 
@@ -615,9 +623,10 @@ int SpdySession::CreateStream(const SpdyStreamRequest& request,
     UMA_HISTOGRAM_BOOLEAN("Net.SpdySession.CreateStreamWithSocketConnected",
                           connection_->socket()->IsConnected());
     if (!connection_->socket()->IsConnected()) {
-      CloseSessionOnError(
+      CloseSessionResult result = DoCloseSession(
           ERR_CONNECTION_CLOSED,
           "Tried to create SPDY stream for a closed socket connection.");
+      DCHECK_EQ(result, SESSION_CLOSED_AND_REMOVED);
       return ERR_CONNECTION_CLOSED;
     }
   }
@@ -715,12 +724,15 @@ int SpdySession::GetProtocolVersion() const {
 }
 
 bool SpdySession::CloseOneIdleConnection() {
+  CHECK(!in_io_loop_);
   DCHECK(pool_);
   DCHECK_NE(availability_state_, STATE_CLOSED);
   if (!active_streams_.empty())
     return false;
   base::WeakPtr<SpdySession> weak_ptr = weak_factory_.GetWeakPtr();
-  CloseSessionOnError(ERR_CONNECTION_CLOSED, "Closing one idle connection.");
+  CloseSessionResult result =
+      DoCloseSession(ERR_CONNECTION_CLOSED, "Closing one idle connection.");
+  DCHECK_EQ(result, SESSION_CLOSED_AND_REMOVED);
   // Since the underlying socket is only returned when |this| is destroyed,
   // we should only return true if |this| no longer exists.
   return !weak_ptr;
@@ -810,6 +822,11 @@ scoped_ptr<SpdyBuffer> SpdySession::CreateDataBuffer(SpdyStreamId stream_id,
                                                      IOBuffer* data,
                                                      int len,
                                                      SpdyDataFlags flags) {
+  if (availability_state_ == STATE_CLOSED) {
+    NOTREACHED();
+    return scoped_ptr<SpdyBuffer>();
+  }
+
   ActiveStreamMap::const_iterator it = active_streams_.find(stream_id);
   CHECK(it != active_streams_.end());
   SpdyStream* stream = it->second.stream;
@@ -1008,10 +1025,6 @@ void SpdySession::CloseCreatedStreamIterator(CreatedStreamSet::iterator it,
 void SpdySession::ResetStreamIterator(ActiveStreamMap::iterator it,
                                       SpdyRstStreamStatus status,
                                       const std::string& description) {
-  // Make sure CloseActiveStreamIterator() does not release the last
-  // reference to |this|.
-  scoped_refptr<SpdySession> self(this);
-
   SpdyStreamId stream_id = it->first;
   RequestPriority priority = it->second.stream->priority();
   // Removes any pending writes for the stream except for possibly an
@@ -1041,19 +1054,35 @@ void SpdySession::SendResetStreamFrame(SpdyStreamId stream_id,
       static_cast<SpdyProtocolErrorDetails>(status + STATUS_CODE_INVALID));
 }
 
-int SpdySession::DoReadLoop(ReadState expected_read_state, int result) {
+void SpdySession::PumpReadLoop(ReadState expected_read_state, int result) {
+  CHECK(!in_io_loop_);
   DCHECK_EQ(read_state_, expected_read_state);
 
+  // TODO(akalin): Change this to a DCHECK once the SpdySessionPool
+  // will hold the last reference to this.
   if (availability_state_ == STATE_CLOSED) {
     DCHECK_LT(error_on_close_, ERR_IO_PENDING);
-    return error_on_close_;
+    return;
   }
 
-  // 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);
+  result = DoReadLoop(expected_read_state, result);
+
+  if (availability_state_ == STATE_CLOSED) {
+    DCHECK_EQ(result, error_on_close_);
+    DCHECK_LT(error_on_close_, ERR_IO_PENDING);
+    RemoveFromPool();
+    return;
+  }
+
+  DCHECK(result == OK || result == ERR_IO_PENDING);
+}
+
+int SpdySession::DoReadLoop(ReadState expected_read_state, int result) {
+  CHECK(!in_io_loop_);
+  DCHECK_NE(availability_state_, STATE_CLOSED);
+  DCHECK_EQ(read_state_, expected_read_state);
+
+  in_io_loop_ = true;
 
   int bytes_read_without_yielding = 0;
 
@@ -1078,24 +1107,31 @@ int SpdySession::DoReadLoop(ReadState expected_read_state, int result) {
     if (availability_state_ == STATE_CLOSED) {
       DCHECK_EQ(result, error_on_close_);
       DCHECK_LT(result, ERR_IO_PENDING);
-      return result;
+      break;
     }
 
     if (result == ERR_IO_PENDING)
-      return ERR_IO_PENDING;
+      break;
 
     if (bytes_read_without_yielding > kMaxReadBytesWithoutYielding) {
       read_state_ = READ_STATE_DO_READ;
       base::MessageLoop::current()->PostTask(
           FROM_HERE,
-          base::Bind(base::IgnoreResult(&SpdySession::DoReadLoop),
+          base::Bind(&SpdySession::PumpReadLoop,
                      weak_factory_.GetWeakPtr(), READ_STATE_DO_READ, OK));
-      return ERR_IO_PENDING;
+      result = ERR_IO_PENDING;
+      break;
     }
   }
+
+  CHECK(in_io_loop_);
+  in_io_loop_ = false;
+
+  return result;
 }
 
 int SpdySession::DoRead() {
+  CHECK(in_io_loop_);
   DCHECK_NE(availability_state_, STATE_CLOSED);
 
   CHECK(connection_);
@@ -1104,11 +1140,12 @@ int SpdySession::DoRead() {
   return connection_->socket()->Read(
       read_buffer_.get(),
       kReadBufferSize,
-      base::Bind(base::IgnoreResult(&SpdySession::DoReadLoop),
+      base::Bind(&SpdySession::PumpReadLoop,
                  weak_factory_.GetWeakPtr(), READ_STATE_DO_READ_COMPLETE));
 }
 
 int SpdySession::DoReadComplete(int result) {
+  CHECK(in_io_loop_);
   DCHECK_NE(availability_state_, STATE_CLOSED);
 
   // Parse a frame.  For now this code requires that the frame fit into our
@@ -1118,14 +1155,18 @@ int SpdySession::DoReadComplete(int result) {
   if (result == 0) {
     UMA_HISTOGRAM_CUSTOM_COUNTS("Net.SpdySession.BytesRead.EOF",
                                 total_bytes_received_, 1, 100000000, 50);
-    CloseSessionOnError(ERR_CONNECTION_CLOSED, "Connection closed");
+    CloseSessionResult close_session_result =
+        DoCloseSession(ERR_CONNECTION_CLOSED, "Connection closed");
+    DCHECK_EQ(close_session_result, SESSION_CLOSED_BUT_NOT_REMOVED);
     DCHECK_EQ(availability_state_, STATE_CLOSED);
     DCHECK_EQ(error_on_close_, ERR_CONNECTION_CLOSED);
     return ERR_CONNECTION_CLOSED;
   }
 
   if (result < 0) {
-    CloseSessionOnError(static_cast<Error>(result), "result is < 0.");
+    CloseSessionResult close_session_result =
+        DoCloseSession(static_cast<Error>(result), "result is < 0.");
+    DCHECK_EQ(close_session_result, SESSION_CLOSED_BUT_NOT_REMOVED);
     DCHECK_EQ(availability_state_, STATE_CLOSED);
     DCHECK_EQ(error_on_close_, result);
     return result;
@@ -1154,19 +1195,36 @@ int SpdySession::DoReadComplete(int result) {
   return OK;
 }
 
-int SpdySession::DoWriteLoop(WriteState expected_write_state, int result) {
-  DCHECK_NE(write_state_, WRITE_STATE_IDLE);
+void SpdySession::PumpWriteLoop(WriteState expected_write_state, int result) {
+  CHECK(!in_io_loop_);
   DCHECK_EQ(write_state_, expected_write_state);
 
+  // TODO(akalin): Change this to a DCHECK once the SpdySessionPool
+  // will hold the last reference to this.
+  if (availability_state_ == STATE_CLOSED) {
+    DCHECK_LT(error_on_close_, ERR_IO_PENDING);
+    return;
+  }
+
+  result = DoWriteLoop(expected_write_state, result);
+
   if (availability_state_ == STATE_CLOSED) {
+    DCHECK_EQ(result, error_on_close_);
     DCHECK_LT(error_on_close_, ERR_IO_PENDING);
-    return error_on_close_;
+    RemoveFromPool();
+    return;
   }
 
-  // Releasing the in-flight write in DoWriteComplete() can have a
-  // side-effect of dropping the last reference to |this|.  Hold a
-  // reference through this function.
-  scoped_refptr<SpdySession> self(this);
+  DCHECK(result == OK || result == ERR_IO_PENDING);
+}
+
+int SpdySession::DoWriteLoop(WriteState expected_write_state, int result) {
+  CHECK(!in_io_loop_);
+  DCHECK_NE(availability_state_, STATE_CLOSED);
+  DCHECK_NE(write_state_, WRITE_STATE_IDLE);
+  DCHECK_EQ(write_state_, expected_write_state);
+
+  in_io_loop_ = true;
 
   // Loop until the session is closed or the write becomes blocked.
   while (true) {
@@ -1187,20 +1245,26 @@ int SpdySession::DoWriteLoop(WriteState expected_write_state, int result) {
     if (availability_state_ == STATE_CLOSED) {
       DCHECK_EQ(result, error_on_close_);
       DCHECK_LT(result, ERR_IO_PENDING);
-      return result;
+      break;
     }
 
     if (write_state_ == WRITE_STATE_IDLE) {
       DCHECK_EQ(result, ERR_IO_PENDING);
-      return ERR_IO_PENDING;
+      break;
     }
 
     if (result == ERR_IO_PENDING)
-      return ERR_IO_PENDING;
+      break;
   }
+
+  CHECK(in_io_loop_);
+  in_io_loop_ = false;
+
+  return result;
 }
 
 int SpdySession::DoWrite() {
+  CHECK(in_io_loop_);
   DCHECK_NE(availability_state_, STATE_CLOSED);
 
   DCHECK(buffered_spdy_framer_);
@@ -1254,11 +1318,12 @@ int SpdySession::DoWrite() {
   return connection_->socket()->Write(
       write_io_buffer.get(),
       in_flight_write_->GetRemainingSize(),
-      base::Bind(base::IgnoreResult(&SpdySession::DoWriteLoop),
+      base::Bind(&SpdySession::PumpWriteLoop,
                  weak_factory_.GetWeakPtr(), WRITE_STATE_DO_WRITE_COMPLETE));
 }
 
 int SpdySession::DoWriteComplete(int result) {
+  CHECK(in_io_loop_);
   DCHECK_NE(availability_state_, STATE_CLOSED);
 
   DCHECK_GT(in_flight_write_->GetRemainingSize(), 0u);
@@ -1271,7 +1336,10 @@ int SpdySession::DoWriteComplete(int result) {
     in_flight_write_frame_type_ = DATA;
     in_flight_write_frame_size_ = 0;
     in_flight_write_stream_.reset();
-    CloseSessionOnError(static_cast<Error>(result), "Write error");
+    CloseSessionResult close_session_result =
+        DoCloseSession(static_cast<Error>(result), "Write error");
+    DCHECK_EQ(close_session_result, SESSION_CLOSED_BUT_NOT_REMOVED);
+    DCHECK_EQ(availability_state_, STATE_CLOSED);
     DCHECK_EQ(error_on_close_, result);
     return result;
   }
@@ -1371,21 +1439,57 @@ void SpdySession::StartGoingAway(SpdyStreamId last_good_stream_id,
 }
 
 void SpdySession::MaybeFinishGoingAway() {
-    DcheckGoingAway();
-    if (active_streams_.empty() && availability_state_ != STATE_CLOSED)
-      CloseSessionOnError(ERR_CONNECTION_CLOSED, "Finished going away");
+  DcheckGoingAway();
+  if (active_streams_.empty() && availability_state_ != STATE_CLOSED) {
+    CloseSessionResult result =
+        DoCloseSession(ERR_CONNECTION_CLOSED, "Finished going away");
+    DCHECK_NE(result, SESSION_ALREADY_CLOSED);
+  }
 }
 
-void SpdySession::DoCloseSession(Error status) {
-  DCHECK_NE(availability_state_, STATE_CLOSED);
+SpdySession::CloseSessionResult SpdySession::DoCloseSession(
+    Error err,
+    const std::string& description) {
+  DCHECK_LT(err, ERR_IO_PENDING);
+
+  if (availability_state_ == STATE_CLOSED)
+    return SESSION_ALREADY_CLOSED;
+
+  net_log_.AddEvent(
+      NetLog::TYPE_SPDY_SESSION_CLOSE,
+      base::Bind(&NetLogSpdySessionCloseCallback, err, &description));
+
+  UMA_HISTOGRAM_SPARSE_SLOWLY("Net.SpdySession.ClosedOnError", -err);
+  UMA_HISTOGRAM_CUSTOM_COUNTS("Net.SpdySession.BytesRead.OtherErrors",
+                              total_bytes_received_, 1, 100000000, 50);
+
+  // |pool_| will be NULL when |InitializeWithSocket()| is in the
+  // call stack.
+  if (pool_ && availability_state_ != STATE_GOING_AWAY)
+    pool_->MakeSessionUnavailable(make_scoped_refptr(this));
 
   availability_state_ = STATE_CLOSED;
-  error_on_close_ = status;
+  error_on_close_ = err;
 
-  StartGoingAway(0, status);
+  StartGoingAway(0, err);
   write_queue_.Clear();
 
   DcheckClosed();
+
+  if (in_io_loop_)
+    return SESSION_CLOSED_BUT_NOT_REMOVED;
+
+  RemoveFromPool();
+  return SESSION_CLOSED_AND_REMOVED;
+}
+
+void SpdySession::RemoveFromPool() {
+  DcheckClosed();
+  CHECK(pool_);
+
+  SpdySessionPool* pool = pool_;
+  pool_ = NULL;
+  pool->RemoveUnavailableSession(make_scoped_refptr(this));
 }
 
 void SpdySession::LogAbandonedStream(SpdyStream* stream, Error status) {
@@ -1425,39 +1529,9 @@ int SpdySession::GetNewStreamId() {
 
 void SpdySession::CloseSessionOnError(Error err,
                                       const std::string& description) {
-  // Closing all streams can have a side-effect of dropping the last reference
-  // to |this|.  Hold a reference through this function.
-  scoped_refptr<SpdySession> self(this);
-
-  DCHECK_LT(err, ERR_IO_PENDING);
-  net_log_.AddEvent(
-      NetLog::TYPE_SPDY_SESSION_CLOSE,
-      base::Bind(&NetLogSpdySessionCloseCallback, err, &description));
-
-  // 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 (availability_state_ != STATE_CLOSED) {
-    UMA_HISTOGRAM_SPARSE_SLOWLY("Net.SpdySession.ClosedOnError", -err);
-    UMA_HISTOGRAM_CUSTOM_COUNTS("Net.SpdySession.BytesRead.OtherErrors",
-                                total_bytes_received_, 1, 100000000, 50);
-
-    // |pool_| will be NULL when |InitializeWithSocket()| is in the
-    // call stack.
-    if (pool_) {
-      SpdySessionPool* pool = pool_;
-      pool_ = NULL;
-      if (availability_state_ != STATE_GOING_AWAY)
-        pool->MakeSessionUnavailable(self);
-      pool->RemoveUnavailableSession(self);
-    }
-
-    // TODO(akalin): Move this before RemoveUnavailableSessions() (but
-    // still after MakeSessionUnavailable()) once we're owned by the
-    // pool.
-    DoCloseSession(err);
-    DcheckClosed();
-  }
+  // We may be called from anywhere, so we can't expect a particular
+  // return value.
+  ignore_result(DoCloseSession(err, description));
 }
 
 base::Value* SpdySession::GetInfoAsValue() const {
@@ -1574,7 +1648,7 @@ void SpdySession::EnqueueWrite(RequestPriority priority,
     write_state_ = WRITE_STATE_DO_WRITE;
     base::MessageLoop::current()->PostTask(
         FROM_HERE,
-        base::Bind(base::IgnoreResult(&SpdySession::DoWriteLoop),
+        base::Bind(&SpdySession::PumpWriteLoop,
                    weak_factory_.GetWeakPtr(), WRITE_STATE_DO_WRITE, OK));
   }
 }
@@ -1680,6 +1754,8 @@ ServerBoundCertService* SpdySession::GetServerBoundCertService() const {
 }
 
 void SpdySession::OnError(SpdyFramer::SpdyError error_code) {
+  CHECK(in_io_loop_);
+
   if (availability_state_ == STATE_CLOSED)
     return;
 
@@ -1687,11 +1763,15 @@ void SpdySession::OnError(SpdyFramer::SpdyError error_code) {
       static_cast<SpdyProtocolErrorDetails>(error_code));
   std::string description = base::StringPrintf(
       "SPDY_ERROR error_code: %d.", error_code);
-  CloseSessionOnError(ERR_SPDY_PROTOCOL_ERROR, description);
+  CloseSessionResult result =
+      DoCloseSession(ERR_SPDY_PROTOCOL_ERROR, description);
+  DCHECK_EQ(result, SESSION_CLOSED_BUT_NOT_REMOVED);
 }
 
 void SpdySession::OnStreamError(SpdyStreamId stream_id,
                                 const std::string& description) {
+  CHECK(in_io_loop_);
+
   if (availability_state_ == STATE_CLOSED)
     return;
 
@@ -1711,6 +1791,8 @@ void SpdySession::OnStreamFrameData(SpdyStreamId stream_id,
                                     const char* data,
                                     size_t len,
                                     bool fin) {
+  CHECK(in_io_loop_);
+
   if (availability_state_ == STATE_CLOSED)
     return;
 
@@ -1755,6 +1837,8 @@ void SpdySession::OnStreamFrameData(SpdyStreamId stream_id,
 }
 
 void SpdySession::OnSettings(bool clear_persisted) {
+  CHECK(in_io_loop_);
+
   if (availability_state_ == STATE_CLOSED)
     return;
 
@@ -1772,6 +1856,8 @@ void SpdySession::OnSettings(bool clear_persisted) {
 void SpdySession::OnSetting(SpdySettingsIds id,
                             uint8 flags,
                             uint32 value) {
+  CHECK(in_io_loop_);
+
   if (availability_state_ == STATE_CLOSED)
     return;
 
@@ -1815,10 +1901,7 @@ int SpdySession::OnInitialResponseHeadersReceived(
     base::Time response_time,
     base::TimeTicks recv_first_byte_time,
     SpdyStream* stream) {
-  // Make sure the stream being closed does not release the last
-  // reference to |this|.
-  scoped_refptr<SpdySession> self(this);
-
+  CHECK(in_io_loop_);
   SpdyStreamId stream_id = stream->stream_id();
   // May invalidate |stream|.
   int rv = stream->OnInitialResponseHeadersReceived(
@@ -1837,6 +1920,8 @@ void SpdySession::OnSynStream(SpdyStreamId stream_id,
                               bool fin,
                               bool unidirectional,
                               const SpdyHeaderBlock& headers) {
+  CHECK(in_io_loop_);
+
   if (availability_state_ == STATE_CLOSED)
     return;
 
@@ -2013,6 +2098,8 @@ void SpdySession::DeleteExpiredPushedStreams() {
 void SpdySession::OnSynReply(SpdyStreamId stream_id,
                              bool fin,
                              const SpdyHeaderBlock& headers) {
+  CHECK(in_io_loop_);
+
   if (availability_state_ == STATE_CLOSED)
     return;
 
@@ -2052,6 +2139,8 @@ void SpdySession::OnSynReply(SpdyStreamId stream_id,
 void SpdySession::OnHeaders(SpdyStreamId stream_id,
                             bool fin,
                             const SpdyHeaderBlock& headers) {
+  CHECK(in_io_loop_);
+
   if (availability_state_ == STATE_CLOSED)
     return;
 
@@ -2082,6 +2171,8 @@ void SpdySession::OnHeaders(SpdyStreamId stream_id,
 
 void SpdySession::OnRstStream(SpdyStreamId stream_id,
                               SpdyRstStreamStatus status) {
+  CHECK(in_io_loop_);
+
   if (availability_state_ == STATE_CLOSED)
     return;
 
@@ -2118,6 +2209,8 @@ void SpdySession::OnRstStream(SpdyStreamId stream_id,
 
 void SpdySession::OnGoAway(SpdyStreamId last_accepted_stream_id,
                            SpdyGoAwayStatus status) {
+  CHECK(in_io_loop_);
+
   if (availability_state_ == STATE_CLOSED)
     return;
 
@@ -2129,6 +2222,8 @@ void SpdySession::OnGoAway(SpdyStreamId last_accepted_stream_id,
                  status));
   if (availability_state_ < STATE_GOING_AWAY) {
     availability_state_ = STATE_GOING_AWAY;
+    // |pool_| will be NULL when |InitializeWithSocket()| is in the
+    // call stack.
     if (pool_)
       pool_->MakeSessionUnavailable(make_scoped_refptr(this));
   }
@@ -2141,6 +2236,8 @@ void SpdySession::OnGoAway(SpdyStreamId last_accepted_stream_id,
 }
 
 void SpdySession::OnPing(uint32 unique_id) {
+  CHECK(in_io_loop_);
+
   if (availability_state_ == STATE_CLOSED)
     return;
 
@@ -2157,7 +2254,9 @@ void SpdySession::OnPing(uint32 unique_id) {
   --pings_in_flight_;
   if (pings_in_flight_ < 0) {
     RecordProtocolErrorHistogram(PROTOCOL_ERROR_UNEXPECTED_PING);
-    CloseSessionOnError(ERR_SPDY_PROTOCOL_ERROR, "pings_in_flight_ is < 0.");
+    CloseSessionResult result =
+        DoCloseSession(ERR_SPDY_PROTOCOL_ERROR, "pings_in_flight_ is < 0.");
+    DCHECK_EQ(result, SESSION_CLOSED_BUT_NOT_REMOVED);
     pings_in_flight_ = 0;
     return;
   }
@@ -2172,6 +2271,8 @@ void SpdySession::OnPing(uint32 unique_id) {
 
 void SpdySession::OnWindowUpdate(SpdyStreamId stream_id,
                                  uint32 delta_window_size) {
+  CHECK(in_io_loop_);
+
   if (availability_state_ == STATE_CLOSED)
     return;
 
@@ -2192,10 +2293,11 @@ void SpdySession::OnWindowUpdate(SpdyStreamId stream_id,
 
     if (delta_window_size < 1u) {
       RecordProtocolErrorHistogram(PROTOCOL_ERROR_INVALID_WINDOW_UPDATE_SIZE);
-      CloseSessionOnError(
+      CloseSessionResult result = DoCloseSession(
           ERR_SPDY_PROTOCOL_ERROR,
           "Received WINDOW_UPDATE with an invalid delta_window_size " +
           base::UintToString(delta_window_size));
+      DCHECK_EQ(result, SESSION_CLOSED_BUT_NOT_REMOVED);
       return;
     }
 
@@ -2251,6 +2353,8 @@ void SpdySession::SendStreamWindowUpdate(SpdyStreamId stream_id,
 }
 
 void SpdySession::SendInitialSettings() {
+  DCHECK_NE(availability_state_, STATE_CLOSED);
+
   // First, notify the server about the settings they should use when
   // communicating with us.
   if (GetProtocolVersion() >= 2 && enable_sending_initial_settings_) {
@@ -2310,6 +2414,8 @@ void SpdySession::SendInitialSettings() {
 
 
 void SpdySession::SendSettings(const SettingsMap& settings) {
+  DCHECK_NE(availability_state_, STATE_CLOSED);
+
   net_log_.AddEvent(
       NetLog::TYPE_SPDY_SESSION_SEND_SETTINGS,
       base::Bind(&NetLogSpdySendSettingsCallback, &settings));
@@ -2437,6 +2543,8 @@ void SpdySession::PlanToCheckPingStatus() {
 }
 
 void SpdySession::CheckPingStatus(base::TimeTicks last_check_time) {
+  CHECK(!in_io_loop_);
+
   // Check if we got a response back for all PINGs we had sent.
   if (pings_in_flight_ == 0) {
     check_ping_status_pending_ = false;
@@ -2449,11 +2557,13 @@ void SpdySession::CheckPingStatus(base::TimeTicks last_check_time) {
   base::TimeDelta delay = hung_interval_ - (now - last_activity_time_);
 
   if (delay.InMilliseconds() < 0 || last_activity_time_ < last_check_time) {
-    CloseSessionOnError(ERR_SPDY_PING_FAILED, "Failed ping.");
     // Track all failed PING messages in a separate bucket.
     const base::TimeDelta kFailedPing =
         base::TimeDelta::FromInternalValue(INT_MAX);
     RecordPingRTTHistogram(kFailedPing);
+    CloseSessionResult result =
+        DoCloseSession(ERR_SPDY_PING_FAILED, "Failed ping.");
+    DCHECK_EQ(result, SESSION_CLOSED_AND_REMOVED);
     return;
   }
 
@@ -2585,7 +2695,16 @@ void SpdySession::OnWriteBufferConsumed(
     size_t frame_payload_size,
     size_t consume_size,
     SpdyBuffer::ConsumeSource consume_source) {
+  // This can happen when pending writes are deleted when closing the
+  // session.
+  if (availability_state_ == STATE_CLOSED)
+    return;
+
   DCHECK_EQ(flow_control_state_, FLOW_CONTROL_STREAM_AND_SESSION);
+
+  // We can be called with |in_io_loop_| set if a write SpdyBuffer is
+  // deleted (e.g., a stream is closed due to incoming data).
+
   if (consume_source == SpdyBuffer::DISCARD) {
     // If we're discarding a frame or part of it, increase the send
     // window by the number of discarded bytes. (Although if we're
@@ -2600,6 +2719,10 @@ void SpdySession::OnWriteBufferConsumed(
 }
 
 void SpdySession::IncreaseSendWindowSize(int32 delta_window_size) {
+  // We can be called with |in_io_loop_| set if a SpdyBuffer is
+  // deleted (e.g., a stream is closed due to incoming data).
+
+  DCHECK_NE(availability_state_, STATE_CLOSED);
   DCHECK_EQ(flow_control_state_, FLOW_CONTROL_STREAM_AND_SESSION);
   DCHECK_GE(delta_window_size, 1);
 
@@ -2607,12 +2730,13 @@ void SpdySession::IncreaseSendWindowSize(int32 delta_window_size) {
   int32 max_delta_window_size = kint32max - session_send_window_size_;
   if (delta_window_size > max_delta_window_size) {
     RecordProtocolErrorHistogram(PROTOCOL_ERROR_INVALID_WINDOW_UPDATE_SIZE);
-    CloseSessionOnError(
+    CloseSessionResult result = DoCloseSession(
         ERR_SPDY_PROTOCOL_ERROR,
         "Received WINDOW_UPDATE [delta: " +
-            base::IntToString(delta_window_size) +
-            "] for session overflows session_send_window_size_ [current: " +
-            base::IntToString(session_send_window_size_) + "]");
+        base::IntToString(delta_window_size) +
+        "] for session overflows session_send_window_size_ [current: " +
+        base::IntToString(session_send_window_size_) + "]");
+    DCHECK_NE(result, SESSION_ALREADY_CLOSED);
     return;
   }
 
@@ -2628,6 +2752,7 @@ void SpdySession::IncreaseSendWindowSize(int32 delta_window_size) {
 }
 
 void SpdySession::DecreaseSendWindowSize(int32 delta_window_size) {
+  DCHECK_NE(availability_state_, STATE_CLOSED);
   DCHECK_EQ(flow_control_state_, FLOW_CONTROL_STREAM_AND_SESSION);
 
   // We only call this method when sending a frame. Therefore,
@@ -2650,6 +2775,14 @@ void SpdySession::DecreaseSendWindowSize(int32 delta_window_size) {
 void SpdySession::OnReadBufferConsumed(
     size_t consume_size,
     SpdyBuffer::ConsumeSource consume_source) {
+  // TODO(akalin): Change this to a DCHECK when we make |pool_| hold
+  // the last reference to |this|.
+  if (availability_state_ == STATE_CLOSED)
+    return;
+
+  // We can be called with |in_io_loop_| set if a read SpdyBuffer is
+  // deleted (e.g., discarded by a SpdyReadQueue).
+
   DCHECK_EQ(flow_control_state_, FLOW_CONTROL_STREAM_AND_SESSION);
   DCHECK_GE(consume_size, 1u);
   DCHECK_LE(consume_size, static_cast<size_t>(kint32max));
@@ -2657,6 +2790,7 @@ void SpdySession::OnReadBufferConsumed(
 }
 
 void SpdySession::IncreaseRecvWindowSize(int32 delta_window_size) {
+  DCHECK_NE(availability_state_, STATE_CLOSED);
   DCHECK_EQ(flow_control_state_, FLOW_CONTROL_STREAM_AND_SESSION);
   DCHECK_GE(session_unacked_recv_window_bytes_, 0);
   DCHECK_GE(session_recv_window_size_, session_unacked_recv_window_bytes_);
@@ -2680,6 +2814,7 @@ void SpdySession::IncreaseRecvWindowSize(int32 delta_window_size) {
 }
 
 void SpdySession::DecreaseRecvWindowSize(int32 delta_window_size) {
+  CHECK(in_io_loop_);
   DCHECK_EQ(flow_control_state_, FLOW_CONTROL_STREAM_AND_SESSION);
   DCHECK_GE(delta_window_size, 1);
 
@@ -2688,11 +2823,12 @@ void SpdySession::DecreaseRecvWindowSize(int32 delta_window_size) {
   // negative. If we do, the receive window isn't being respected.
   if (delta_window_size > session_recv_window_size_) {
     RecordProtocolErrorHistogram(PROTOCOL_ERROR_RECEIVE_WINDOW_VIOLATION);
-    CloseSessionOnError(
+    CloseSessionResult result = DoCloseSession(
         ERR_SPDY_PROTOCOL_ERROR,
         "delta_window_size is " + base::IntToString(delta_window_size) +
             " in DecreaseRecvWindowSize, which is larger than the receive " +
             "window size of " + base::IntToString(session_recv_window_size_));
+    DCHECK_EQ(result, SESSION_CLOSED_BUT_NOT_REMOVED);
     return;
   }
 
diff --git a/net/spdy/spdy_session.h b/net/spdy/spdy_session.h
index 52d4843..5f6f4e0 100644
--- a/net/spdy/spdy_session.h
+++ b/net/spdy/spdy_session.h
@@ -346,12 +346,14 @@ class NET_EXPORT SpdySession : public base::RefCounted<SpdySession>,
   // 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.
-  // |err| should not be OK; this function is intended to be called on
-  // error.
-  // |remove_from_pool| indicates whether to also remove the session from the
-  // session pool.
+  // Closes this session. This will close all active streams and mark
+  // the session as permanently closed. Callers must assume that the
+  // session is destroyed after this is called. (However, it may not
+  // be destroyed right away, e.g. when a SpdySession function is
+  // present in the call stack.)
+  //
+  // |err| should be < ERR_IO_PENDING; this function is intended to be
+  // called on error.
   // |description| indicates the reason for the error.
   void CloseSessionOnError(Error err, const std::string& description);
 
@@ -532,6 +534,18 @@ class NET_EXPORT SpdySession : public base::RefCounted<SpdySession>,
     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,
+  };
+
   virtual ~SpdySession();
 
   // Checks whether a stream for the given |url| can be created or
@@ -583,15 +597,34 @@ class NET_EXPORT SpdySession : public base::RefCounted<SpdySession>,
                             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.
+  void PumpReadLoop(ReadState expected_read_state, int result);
+
   // Advance the ReadState state machine. |expected_read_state| is the
   // expected starting read state.
+  //
+  // This function must always be called via PumpReadLoop() except for
+  // from InitializeWithSocket().
   int DoReadLoop(ReadState expected_read_state, int result);
   // The implementations of the states of the ReadState state machine.
   int DoRead();
   int DoReadComplete(int result);
 
+  // Calls DoWriteLoop and then if |availability_state_| is
+  // STATE_CLOSED, calls RemoveFromPool().
+  //
+  // Use this function instead of DoWriteLoop when posting a task to
+  // pump the write loop.
+  void PumpWriteLoop(WriteState expected_write_state, int result);
+
   // Advance the WriteState state machine. |expected_write_state| is
   // the expected starting write state.
+  //
+  // This function must always be called via PumpWriteLoop().
   int DoWriteLoop(WriteState expected_write_state, int result);
   // The implementations of the states of the WriteState state machine.
   int DoWrite();
@@ -702,11 +735,19 @@ class NET_EXPORT SpdySession : public base::RefCounted<SpdySession>,
   // isn't closed yet, close it.
   void MaybeFinishGoingAway();
 
-  // Sets |availability_state_| to STATE_CLOSED, closes all streams,
-  // and clears the write queue. Must be called only when
-  // |availability_state_| < STATE_CLOSED. After this function,
-  // DcheckClosed() will pass.
-  void DoCloseSession(Error status);
+  // 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();
 
   // Called right before closing a (possibly-inactive) stream for a
   // reason other than being requested to by the stream.
@@ -868,6 +909,10 @@ class NET_EXPORT SpdySession : public base::RefCounted<SpdySession>,
   // method.
   base::WeakPtrFactory<SpdySession> weak_factory_;
 
+  // Whether Do{Read,Write}Loop() is in the call stack. Useful for
+  // making sure we don't destroy ourselves prematurely in that case.
+  bool in_io_loop_;
+
   // The key used to identify this session.
   const SpdySessionKey spdy_session_key_;
 
diff --git a/net/spdy/spdy_session_pool_unittest.cc b/net/spdy/spdy_session_pool_unittest.cc
index 576d30a..58fedcb 100644
--- a/net/spdy/spdy_session_pool_unittest.cc
+++ b/net/spdy/spdy_session_pool_unittest.cc
@@ -418,15 +418,13 @@ void SpdySessionPoolTest::RunIPPoolingTest(
   // Cleanup the sessions.
   switch (close_sessions_type) {
     case SPDY_POOL_CLOSE_SESSIONS_MANUALLY:
-      spdy_session_pool_->MakeSessionUnavailable(session);
-      spdy_session_pool_->RemoveUnavailableSession(session);
+      session->CloseSessionOnError(ERR_ABORTED, std::string());
       session = NULL;
-      spdy_session_pool_->MakeSessionUnavailable(session2);
-      spdy_session_pool_->RemoveUnavailableSession(session2);
+      session2->CloseSessionOnError(ERR_ABORTED, std::string());
       session2 = NULL;
       break;
     case SPDY_POOL_CLOSE_CURRENT_SESSIONS:
-      spdy_session_pool_->CloseCurrentSessions(net::ERR_ABORTED);
+      spdy_session_pool_->CloseCurrentSessions(ERR_ABORTED);
       break;
     case SPDY_POOL_CLOSE_IDLE_SESSIONS:
       GURL url(test_hosts[0].url);
@@ -472,8 +470,7 @@ void SpdySessionPoolTest::RunIPPoolingTest(
       EXPECT_EQ(NULL, spdy_stream.get());
       EXPECT_EQ(NULL, spdy_stream1.get());
       EXPECT_EQ(NULL, spdy_stream2.get());
-      spdy_session_pool_->MakeSessionUnavailable(session2);
-      spdy_session_pool_->RemoveUnavailableSession(session2);
+      session2->CloseSessionOnError(ERR_ABORTED, std::string());
       session2 = NULL;
       break;
   }
diff --git a/net/spdy/spdy_session_unittest.cc b/net/spdy/spdy_session_unittest.cc
index 06b3979..2de35ff 100644
--- a/net/spdy/spdy_session_unittest.cc
+++ b/net/spdy/spdy_session_unittest.cc
@@ -183,6 +183,15 @@ INSTANTIATE_TEST_CASE_P(
     SpdySessionTest,
     testing::Values(kProtoSPDY2, kProtoSPDY3, kProtoSPDY31, kProtoSPDY4a2));
 
+// Try to create a SPDY session that will fail during
+// initialization. Nothing should blow up.
+TEST_P(SpdySessionTest, InitialReadError) {
+  CreateDeterministicNetworkSession();
+
+  TryCreateFakeSpdySessionExpectingFailure(
+      spdy_session_pool_, key_, ERR_FAILED);
+}
+
 // A session receiving a GOAWAY frame with no active streams should
 // immediately close.
 TEST_P(SpdySessionTest, GoAwayWithNoActiveStreams) {
@@ -222,6 +231,33 @@ TEST_P(SpdySessionTest, GoAwayWithNoActiveStreams) {
   session = NULL;
 }
 
+// A session receiving a GOAWAY frame immediately with no active
+// streams should then close.
+TEST_P(SpdySessionTest, GoAwayImmediatelyWithNoActiveStreams) {
+  session_deps_.host_resolver->set_synchronous_mode(true);
+
+  MockConnect connect_data(SYNCHRONOUS, OK);
+  scoped_ptr<SpdyFrame> goaway(spdy_util_.ConstructSpdyGoAway(1));
+  MockRead reads[] = {
+    CreateMockRead(*goaway, 0, SYNCHRONOUS),
+  };
+  DeterministicSocketData data(reads, arraysize(reads), NULL, 0);
+  data.set_connect_data(connect_data);
+  session_deps_.deterministic_socket_factory->AddSocketDataProvider(&data);
+
+  SSLSocketDataProvider ssl(SYNCHRONOUS, OK);
+  session_deps_.deterministic_socket_factory->AddSSLSocketDataProvider(&ssl);
+
+  CreateDeterministicNetworkSession();
+
+  data.StopAfter(1);
+
+  TryCreateInsecureSpdySessionExpectingFailure(
+      http_session_, key_, ERR_CONNECTION_CLOSED, BoundNetLog());
+
+  EXPECT_FALSE(HasSpdySession(spdy_session_pool_, key_));
+}
+
 // A session receiving a GOAWAY frame with active streams should close
 // when the last active stream is closed.
 TEST_P(SpdySessionTest, GoAwayWithActiveStreams) {
@@ -812,7 +848,10 @@ TEST_P(SpdySessionTest, DeleteExpiredPushStreams) {
   SpdyHeaderBlock headers;
   spdy_util_.AddUrlToHeaderBlock("http://www.google.com/a.dat", &headers);
 
+  // OnSynStream() expects |in_io_loop_| to be true.
+  session->in_io_loop_ = true;
   session->OnSynStream(2, 1, 0, 0, true, false, headers);
+  session->in_io_loop_ = false;
 
   // Verify that there is one unclaimed push stream.
   EXPECT_EQ(1u, session->num_unclaimed_pushed_streams());
@@ -825,7 +864,9 @@ TEST_P(SpdySessionTest, DeleteExpiredPushStreams) {
   g_time_delta = base::TimeDelta::FromSeconds(301);
 
   spdy_util_.AddUrlToHeaderBlock("http://www.google.com/b.dat", &headers);
+  session->in_io_loop_ = true;
   session->OnSynStream(4, 1, 0, 0, true, false, headers);
+  session->in_io_loop_ = false;
 
   // Verify that the second pushed stream evicted the first pushed stream.
   EXPECT_EQ(1u, session->num_unclaimed_pushed_streams());
@@ -1307,6 +1348,11 @@ TEST_P(SpdySessionTest, CloseSessionOnError) {
   } else {
     ADD_FAILURE();
   }
+
+  // Release the last session reference here so it doesn't try to
+  // access |log| after its destroyed.
+  EXPECT_TRUE(session->HasOneRef());
+  session = NULL;
 }
 
 // Queue up a low-priority SYN_STREAM followed by a high-priority
@@ -3017,9 +3063,12 @@ TEST_P(SpdySessionTest, AdjustRecvWindowSize) {
 
   data.RunFor(1);
 
+  // DecreaseRecvWindowSize() expects |in_io_loop_| to be true.
+  session->in_io_loop_ = true;
   session->DecreaseRecvWindowSize(
       kSpdySessionInitialWindowSize + delta_window_size +
       kSpdySessionInitialWindowSize);
+  session->in_io_loop_ = false;
   EXPECT_EQ(0, session->session_recv_window_size_);
   EXPECT_EQ(0, session->session_unacked_recv_window_bytes_);
 }
diff --git a/net/spdy/spdy_test_util_common.cc b/net/spdy/spdy_test_util_common.cc
index d8712cb..ef603fb 100644
--- a/net/spdy/spdy_test_util_common.cc
+++ b/net/spdy/spdy_test_util_common.cc
@@ -493,6 +493,7 @@ scoped_refptr<SpdySession> CreateSpdySessionHelper(
     const scoped_refptr<HttpNetworkSession>& http_session,
     const SpdySessionKey& key,
     const BoundNetLog& net_log,
+    Error expected_status,
     bool is_secure) {
   EXPECT_FALSE(HasSpdySession(http_session->spdy_session_pool(), key));
 
@@ -544,11 +545,13 @@ scoped_refptr<SpdySession> CreateSpdySessionHelper(
 
   scoped_refptr<SpdySession> spdy_session;
   EXPECT_EQ(
-      OK,
+      expected_status,
       http_session->spdy_session_pool()->CreateAvailableSessionFromSocket(
           key, connection.Pass(), net_log, OK, &spdy_session,
           is_secure));
-  EXPECT_TRUE(HasSpdySession(http_session->spdy_session_pool(), key));
+  EXPECT_EQ(expected_status == OK, spdy_session != NULL);
+  EXPECT_EQ(expected_status == OK,
+            HasSpdySession(http_session->spdy_session_pool(), key));
   return spdy_session;
 }
 
@@ -559,7 +562,17 @@ scoped_refptr<SpdySession> CreateInsecureSpdySession(
     const SpdySessionKey& key,
     const BoundNetLog& net_log) {
   return CreateSpdySessionHelper(http_session, key, net_log,
-                                 false /* is_secure */);
+                                 OK, false /* is_secure */);
+}
+
+void TryCreateInsecureSpdySessionExpectingFailure(
+    const scoped_refptr<HttpNetworkSession>& http_session,
+    const SpdySessionKey& key,
+    Error expected_error,
+    const BoundNetLog& net_log) {
+  DCHECK_LT(expected_error, ERR_IO_PENDING);
+  CreateSpdySessionHelper(http_session, key, net_log,
+                          expected_error, false /* is_secure */);
 }
 
 scoped_refptr<SpdySession> CreateSecureSpdySession(
@@ -567,7 +580,7 @@ scoped_refptr<SpdySession> CreateSecureSpdySession(
     const SpdySessionKey& key,
     const BoundNetLog& net_log) {
   return CreateSpdySessionHelper(http_session, key, net_log,
-                                 true /* is_secure */);
+                                 OK, true /* is_secure */);
 }
 
 namespace {
@@ -575,12 +588,15 @@ namespace {
 // A ClientSocket used for CreateFakeSpdySession() below.
 class FakeSpdySessionClientSocket : public MockClientSocket {
  public:
-  FakeSpdySessionClientSocket() : MockClientSocket(BoundNetLog()) {}
+  FakeSpdySessionClientSocket(int read_result)
+      : MockClientSocket(BoundNetLog()),
+        read_result_(read_result) {}
+
   virtual ~FakeSpdySessionClientSocket() {}
 
   virtual int Read(IOBuffer* buf, int buf_len,
                    const CompletionCallback& callback) OVERRIDE {
-    return ERR_IO_PENDING;
+    return read_result_;
   }
 
   virtual int Write(IOBuffer* buf, int buf_len,
@@ -619,25 +635,45 @@ class FakeSpdySessionClientSocket : public MockClientSocket {
     ADD_FAILURE();
     return false;
   }
-};
 
-}  // namespace
+ private:
+  int read_result_;
+};
 
-scoped_refptr<SpdySession> CreateFakeSpdySession(SpdySessionPool* pool,
-                                                 const SpdySessionKey& key) {
+scoped_refptr<SpdySession> CreateFakeSpdySessionHelper(
+    SpdySessionPool* pool,
+    const SpdySessionKey& key,
+    Error expected_status) {
+  EXPECT_NE(expected_status, ERR_IO_PENDING);
   EXPECT_FALSE(HasSpdySession(pool, key));
   scoped_refptr<SpdySession> spdy_session;
   scoped_ptr<ClientSocketHandle> handle(new ClientSocketHandle());
-  handle->set_socket(new FakeSpdySessionClientSocket());
+  handle->set_socket(new FakeSpdySessionClientSocket(
+      expected_status == OK ? ERR_IO_PENDING : expected_status));
   EXPECT_EQ(
-      OK,
+      expected_status,
       pool->CreateAvailableSessionFromSocket(
           key, handle.Pass(), BoundNetLog(), OK, &spdy_session,
           true /* is_secure */));
-  EXPECT_TRUE(HasSpdySession(pool, key));
+  EXPECT_EQ(expected_status == OK, spdy_session != NULL);
+  EXPECT_EQ(expected_status == OK, HasSpdySession(pool, key));
   return spdy_session;
 }
 
+}  // namespace
+
+scoped_refptr<SpdySession> CreateFakeSpdySession(SpdySessionPool* pool,
+                                                 const SpdySessionKey& key) {
+  return CreateFakeSpdySessionHelper(pool, key, OK);
+}
+
+void TryCreateFakeSpdySessionExpectingFailure(SpdySessionPool* pool,
+                                              const SpdySessionKey& key,
+                                              Error expected_error) {
+  DCHECK_LT(expected_error, ERR_IO_PENDING);
+  CreateFakeSpdySessionHelper(pool, key, expected_error);
+}
+
 SpdySessionPoolPeer::SpdySessionPoolPeer(SpdySessionPool* pool) : pool_(pool) {
 }
 
diff --git a/net/spdy/spdy_test_util_common.h b/net/spdy/spdy_test_util_common.h
index d7dfbf2..a65efc4 100644
--- a/net/spdy/spdy_test_util_common.h
+++ b/net/spdy/spdy_test_util_common.h
@@ -245,6 +245,15 @@ scoped_refptr<SpdySession> CreateInsecureSpdySession(
     const SpdySessionKey& key,
     const BoundNetLog& net_log);
 
+// Tries to create a SPDY session for the given key but expects the
+// attempt to fail with the given error. A SPDY session for |key| must
+// not already exist.
+void TryCreateInsecureSpdySessionExpectingFailure(
+    const scoped_refptr<HttpNetworkSession>& http_session,
+    const SpdySessionKey& key,
+    Error expected_error,
+    const BoundNetLog& net_log);
+
 // Like CreateInsecureSpdySession(), but uses TLS.
 scoped_refptr<SpdySession> CreateSecureSpdySession(
     const scoped_refptr<HttpNetworkSession>& http_session,
@@ -252,11 +261,19 @@ scoped_refptr<SpdySession> CreateSecureSpdySession(
     const BoundNetLog& net_log);
 
 // Creates an insecure SPDY session for the given key and puts it in
-// |pool|. The returned session will neither receiver nor send any
+// |pool|. The returned session will neither receive nor send any
 // data. A SPDY session for |key| must not already exist.
 scoped_refptr<SpdySession> CreateFakeSpdySession(SpdySessionPool* pool,
                                                  const SpdySessionKey& key);
 
+// Tries to create an insecure SPDY session for the given key but
+// expects the attempt to fail with the given error. The session will
+// neither receive nor send any data. A SPDY session for |key| must
+// not already exist.
+void TryCreateFakeSpdySessionExpectingFailure(SpdySessionPool* pool,
+                                              const SpdySessionKey& key,
+                                              Error expected_error);
+
 class SpdySessionPoolPeer {
  public:
   explicit SpdySessionPoolPeer(SpdySessionPool* pool);