Revert of Fix SPDY error-handling if the connection gets closed just after use. (https://codereview.chromium.org/200723004/)

Reason for revert:
Linux LSan isn't happy: AddressSanitizer: stack-buffer-overflow

http://build.chromium.org/p/chromium.memory/builders/Linux%20ASan%2BLSan%20Tests%20%281%29/builds/650/steps/net_unittests/logs/GetUploadProgressBeforeInitialization_0

Original issue's description:
> Fix SPDY error-handling if the connection gets closed just after use.
> 
> - Make SpdySession::IsReused() return true if the underlying socket was
>   UNUSED_IDLE. This makes the HttpNetworkTransaction-level retry try a fresh
>   socket in case a preconnected socket was stale.
> 
> - If the SpdySession closes in an event loop iteration between
>   HttpStreamFactoryImplJob::DoCreateStream and OnNewSpdySessionReadyCallback,
>   propogate the error to the request to prevent it from hanging. Do so by
>   creating the originating request's SpdyHttpStream as soon as the SpdySession
>   is created so it can sample SpdySession::IsReused() and advise
>   HttpNetworkTransaction on error.
> 
> - Delay pumping the SpdySession read loop by an event loop iteration. This
>   simplifies some logic and ensures that HttpNetworkTransaction receives a
>   SpdyHttpStream to advise retry logic. This does mean we lose the error code;
>   it now follows the asynchronous case and turns into ERR_CONNECTION_CLOSED in
>   SpdyHttpStream::InitializeStream.
> 
> BUG=352156
> 
> Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=258647

TBR=rch@chromium.org,davidben@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=352156

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

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

12 files changed, 144 insertions, 213 deletions

diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc
index ca06785..7dda13e 100644
--- a/net/http/http_network_transaction_unittest.cc
+++ b/net/http/http_network_transaction_unittest.cc
@@ -289,8 +289,7 @@ class HttpNetworkTransactionTest
   // failure should cause the network transaction to resend the request, and the
   // other argument should be NULL.
   void PreconnectErrorResendRequestTest(const MockWrite* write_failure,
-                                        const MockRead* read_failure,
-                                        bool use_spdy);
+                                        const MockRead* read_failure);
 
   SimpleGetHelperResult SimpleGetHelperForData(StaticSocketDataProvider* data[],
                                                size_t data_count) {
@@ -1309,81 +1308,46 @@ void HttpNetworkTransactionTest::KeepAliveConnectionResendRequestTest(
 
 void HttpNetworkTransactionTest::PreconnectErrorResendRequestTest(
     const MockWrite* write_failure,
-    const MockRead* read_failure,
-    bool use_spdy) {
+    const MockRead* read_failure) {
   HttpRequestInfo request;
   request.method = "GET";
-  request.url = GURL("https://www.foo.com/");
+  request.url = GURL("http://www.foo.com/");
   request.load_flags = 0;
 
   CapturingNetLog net_log;
   session_deps_.net_log = &net_log;
   scoped_refptr<HttpNetworkSession> session(CreateSession(&session_deps_));
 
-  SSLSocketDataProvider ssl1(ASYNC, OK);
-  SSLSocketDataProvider ssl2(ASYNC, OK);
-  if (use_spdy) {
-    ssl1.SetNextProto(GetParam());
-    ssl2.SetNextProto(GetParam());
-  }
-  session_deps_.socket_factory->AddSSLSocketDataProvider(&ssl1);
-  session_deps_.socket_factory->AddSSLSocketDataProvider(&ssl2);
-
-  // SPDY versions of the request and response.
-  scoped_ptr<SpdyFrame> spdy_request(spdy_util_.ConstructSpdyGet(
-      request.url.spec().c_str(), false, 1, DEFAULT_PRIORITY));
-  scoped_ptr<SpdyFrame> spdy_response(
-      spdy_util_.ConstructSpdyGetSynReply(NULL, 0, 1));
-  scoped_ptr<SpdyFrame> spdy_data(
-      spdy_util_.ConstructSpdyBodyFrame(1, "hello", 5, true));
+  // Written data for successfully sending a request.
+  MockWrite data1_writes[] = {
+    MockWrite("GET / HTTP/1.1\r\n"
+              "Host: www.foo.com\r\n"
+              "Connection: keep-alive\r\n\r\n"),
+  };
 
-  // HTTP/1.1 versions of the request and response.
-  const char kHttpRequest[] = "GET / HTTP/1.1\r\n"
-      "Host: www.foo.com\r\n"
-      "Connection: keep-alive\r\n\r\n";
-  const char kHttpResponse[] = "HTTP/1.1 200 OK\r\nContent-Length: 5\r\n\r\n";
-  const char kHttpData[] = "hello";
+  // Read results for the first request.
+  MockRead data1_reads[] = {
+    MockRead(ASYNC, OK),
+  };
 
-  std::vector<MockRead> data1_reads;
-  std::vector<MockWrite> data1_writes;
   if (write_failure) {
     ASSERT_FALSE(read_failure);
-    data1_writes.push_back(*write_failure);
-    data1_reads.push_back(MockRead(ASYNC, OK));
+    data1_writes[0] = *write_failure;
   } else {
     ASSERT_TRUE(read_failure);
-    if (use_spdy) {
-      data1_writes.push_back(CreateMockWrite(*spdy_request));
-    } else {
-      data1_writes.push_back(MockWrite(kHttpRequest));
-    }
-    data1_reads.push_back(*read_failure);
+    data1_reads[0] = *read_failure;
   }
 
-  StaticSocketDataProvider data1(&data1_reads[0], data1_reads.size(),
-                                 &data1_writes[0], data1_writes.size());
+  StaticSocketDataProvider data1(data1_reads, arraysize(data1_reads),
+                                 data1_writes, arraysize(data1_writes));
   session_deps_.socket_factory->AddSocketDataProvider(&data1);
 
-  std::vector<MockRead> data2_reads;
-  std::vector<MockWrite> data2_writes;
-
-  if (use_spdy) {
-    data2_writes.push_back(CreateMockWrite(*spdy_request, 0, ASYNC));
-
-    data2_reads.push_back(CreateMockRead(*spdy_response, 1, ASYNC));
-    data2_reads.push_back(CreateMockRead(*spdy_data, 2, ASYNC));
-    data2_reads.push_back(MockRead(ASYNC, OK, 3));
-  } else {
-    data2_writes.push_back(
-        MockWrite(ASYNC, kHttpRequest, strlen(kHttpRequest), 0));
-
-    data2_reads.push_back(
-        MockRead(ASYNC, kHttpResponse, strlen(kHttpResponse), 1));
-    data2_reads.push_back(MockRead(ASYNC, kHttpData, strlen(kHttpData), 2));
-    data2_reads.push_back(MockRead(ASYNC, OK, 3));
-  }
-  OrderedSocketData data2(&data2_reads[0], data2_reads.size(),
-                          &data2_writes[0], data2_writes.size());
+  MockRead data2_reads[] = {
+    MockRead("HTTP/1.1 200 OK\r\nContent-Length: 5\r\n\r\n"),
+    MockRead("hello"),
+    MockRead(ASYNC, OK),
+  };
+  StaticSocketDataProvider data2(data2_reads, arraysize(data2_reads), NULL, 0);
   session_deps_.socket_factory->AddSocketDataProvider(&data2);
 
   // Preconnect a socket.
@@ -1396,7 +1360,7 @@ void HttpNetworkTransactionTest::PreconnectErrorResendRequestTest(
   // Wait for the preconnect to complete.
   // TODO(davidben): Some way to wait for an idle socket count might be handy.
   base::RunLoop().RunUntilIdle();
-  EXPECT_EQ(1, GetIdleSocketCountInSSLSocketPool(session.get()));
+  EXPECT_EQ(1, GetIdleSocketCountInTransportSocketPool(session.get()));
 
   // Make the request.
   TestCompletionCallback callback;
@@ -1412,9 +1376,7 @@ void HttpNetworkTransactionTest::PreconnectErrorResendRequestTest(
 
   LoadTimingInfo load_timing_info;
   EXPECT_TRUE(trans->GetLoadTimingInfo(&load_timing_info));
-  TestLoadTimingNotReused(
-      load_timing_info,
-      CONNECT_TIMING_HAS_DNS_TIMES|CONNECT_TIMING_HAS_SSL_TIMES);
+  TestLoadTimingNotReused(load_timing_info, CONNECT_TIMING_HAS_DNS_TIMES);
 
   const HttpResponseInfo* response = trans->GetResponseInfo();
   ASSERT_TRUE(response != NULL);
@@ -1425,7 +1387,7 @@ void HttpNetworkTransactionTest::PreconnectErrorResendRequestTest(
   std::string response_data;
   rv = ReadTransaction(trans.get(), &response_data);
   EXPECT_EQ(OK, rv);
-  EXPECT_EQ(kHttpData, response_data);
+  EXPECT_EQ("hello", response_data);
 }
 
 TEST_P(HttpNetworkTransactionTest,
@@ -1447,43 +1409,17 @@ TEST_P(HttpNetworkTransactionTest, KeepAliveConnectionEOF) {
 TEST_P(HttpNetworkTransactionTest,
        PreconnectErrorNotConnectedOnWrite) {
   MockWrite write_failure(ASYNC, ERR_SOCKET_NOT_CONNECTED);
-  PreconnectErrorResendRequestTest(&write_failure, NULL, false);
+  PreconnectErrorResendRequestTest(&write_failure, NULL);
 }
 
 TEST_P(HttpNetworkTransactionTest, PreconnectErrorReset) {
   MockRead read_failure(ASYNC, ERR_CONNECTION_RESET);
-  PreconnectErrorResendRequestTest(NULL, &read_failure, false);
+  PreconnectErrorResendRequestTest(NULL, &read_failure);
 }
 
 TEST_P(HttpNetworkTransactionTest, PreconnectErrorEOF) {
   MockRead read_failure(SYNCHRONOUS, OK);  // EOF
-  PreconnectErrorResendRequestTest(NULL, &read_failure, false);
-}
-
-TEST_P(HttpNetworkTransactionTest, PreconnectErrorAsyncEOF) {
-  MockRead read_failure(ASYNC, OK);  // EOF
-  PreconnectErrorResendRequestTest(NULL, &read_failure, false);
-}
-
-TEST_P(HttpNetworkTransactionTest,
-       SpdyPreconnectErrorNotConnectedOnWrite) {
-  MockWrite write_failure(ASYNC, ERR_SOCKET_NOT_CONNECTED);
-  PreconnectErrorResendRequestTest(&write_failure, NULL, true);
-}
-
-TEST_P(HttpNetworkTransactionTest, SpdyPreconnectErrorReset) {
-  MockRead read_failure(ASYNC, ERR_CONNECTION_RESET);
-  PreconnectErrorResendRequestTest(NULL, &read_failure, true);
-}
-
-TEST_P(HttpNetworkTransactionTest, SpdyPreconnectErrorEOF) {
-  MockRead read_failure(SYNCHRONOUS, OK);  // EOF
-  PreconnectErrorResendRequestTest(NULL, &read_failure, true);
-}
-
-TEST_P(HttpNetworkTransactionTest, SpdyPreconnectErrorAsyncEOF) {
-  MockRead read_failure(ASYNC, OK);  // EOF
-  PreconnectErrorResendRequestTest(NULL, &read_failure, true);
+  PreconnectErrorResendRequestTest(NULL, &read_failure);
 }
 
 TEST_P(HttpNetworkTransactionTest, NonKeepAliveConnectionReset) {
diff --git a/net/http/http_proxy_client_socket_pool.cc b/net/http/http_proxy_client_socket_pool.cc
index e2529f8..8d691b7 100644
--- a/net/http/http_proxy_client_socket_pool.cc
+++ b/net/http/http_proxy_client_socket_pool.cc
@@ -315,11 +315,11 @@ int HttpProxyConnectJob::DoSpdyProxyCreateStream() {
     }
   } else {
     // Create a session direct to the proxy itself
-    spdy_session =
-        spdy_pool->CreateAvailableSessionFromSocket(
-            key, transport_socket_handle_.Pass(),
-            net_log(), OK, /*using_ssl_*/ true);
-    DCHECK(spdy_session);
+    int rv = spdy_pool->CreateAvailableSessionFromSocket(
+        key, transport_socket_handle_.Pass(),
+        net_log(), OK, &spdy_session, /*using_ssl_*/ true);
+    if (rv < 0)
+      return rv;
   }
 
   next_state_ = STATE_SPDY_PROXY_CREATE_STREAM_COMPLETE;
diff --git a/net/http/http_stream_factory_impl_job.cc b/net/http/http_stream_factory_impl_job.cc
index d595f2a..960bc58 100644
--- a/net/http/http_stream_factory_impl_job.cc
+++ b/net/http/http_stream_factory_impl_job.cc
@@ -334,23 +334,20 @@ void HttpStreamFactoryImpl::Job::OnWebSocketHandshakeStreamReadyCallback() {
 }
 
 void HttpStreamFactoryImpl::Job::OnNewSpdySessionReadyCallback() {
-  DCHECK(stream_.get());
+  DCHECK(!stream_.get());
   DCHECK(!IsPreconnecting());
   DCHECK(using_spdy());
-  // Note: an event loop iteration has passed, so |new_spdy_session_| may be
-  // NULL at this point if the SpdySession closed immediately after creation.
+  if (!new_spdy_session_)
+    return;
   base::WeakPtr<SpdySession> spdy_session = new_spdy_session_;
   new_spdy_session_.reset();
   if (IsOrphaned()) {
-    if (spdy_session) {
-      stream_factory_->OnNewSpdySessionReady(
-          spdy_session, spdy_session_direct_, server_ssl_config_, proxy_info_,
-          was_npn_negotiated(), protocol_negotiated(), using_spdy(), net_log_);
-    }
+    stream_factory_->OnNewSpdySessionReady(
+        spdy_session, spdy_session_direct_, server_ssl_config_, proxy_info_,
+        was_npn_negotiated(), protocol_negotiated(), using_spdy(), net_log_);
     stream_factory_->OnOrphanedJobComplete(this);
   } else {
-    request_->OnNewSpdySessionReady(
-        this, stream_.Pass(), spdy_session, spdy_session_direct_);
+    request_->OnNewSpdySessionReady(this, spdy_session, spdy_session_direct_);
   }
   // |this| may be deleted after this call.
 }
@@ -1107,27 +1104,21 @@ int HttpStreamFactoryImpl::Job::DoCreateStream() {
     SpdySessionPool* spdy_pool = session_->spdy_session_pool();
     spdy_session = spdy_pool->FindAvailableSession(spdy_session_key, net_log_);
     if (!spdy_session) {
-      new_spdy_session_ =
+      int error =
           spdy_pool->CreateAvailableSessionFromSocket(spdy_session_key,
                                                       connection_.Pass(),
                                                       net_log_,
                                                       spdy_certificate_error_,
+                                                      &new_spdy_session_,
                                                       using_ssl_);
+      if (error != OK)
+        return error;
       const HostPortPair& host_port_pair = spdy_session_key.host_port_pair();
       base::WeakPtr<HttpServerProperties> http_server_properties =
           session_->http_server_properties();
       if (http_server_properties)
         http_server_properties->SetSupportsSpdy(host_port_pair, true);
       spdy_session_direct_ = direct;
-
-      // Create a SpdyHttpStream attached to the session;
-      // OnNewSpdySessionReadyCallback is not called until an event loop
-      // iteration later, so if the SpdySession is closed between then, allow
-      // reuse state from the underlying socket, sampled by SpdyHttpStream,
-      // bubble up to the request.
-      bool use_relative_url = direct || request_info_.url.SchemeIs("https");
-      stream_.reset(new SpdyHttpStream(new_spdy_session_, use_relative_url));
-
       return OK;
     }
   }
diff --git a/net/http/http_stream_factory_impl_request.cc b/net/http/http_stream_factory_impl_request.cc
index 5f129ac..ff66f14 100644
--- a/net/http/http_stream_factory_impl_request.cc
+++ b/net/http/http_stream_factory_impl_request.cc
@@ -291,16 +291,11 @@ HttpStreamFactoryImpl::Request::RemoveRequestFromHttpPipeliningRequestMap() {
 
 void HttpStreamFactoryImpl::Request::OnNewSpdySessionReady(
     Job* job,
-    scoped_ptr<HttpStream> stream,
     const base::WeakPtr<SpdySession>& spdy_session,
     bool direct) {
   DCHECK(job);
   DCHECK(job->using_spdy());
 
-  // Note: |spdy_session| may be NULL. In that case, |delegate_| should still
-  // receive |stream| so the error propogates up correctly, however there is no
-  // point in broadcasting |spdy_session| to other requests.
-
   // The first case is the usual case.
   if (!bound_job_.get()) {
     OrphanJobsExcept(job);
@@ -327,20 +322,21 @@ void HttpStreamFactoryImpl::Request::OnNewSpdySessionReady(
     // implemented.
     NOTREACHED();
   } else {
-    delegate_->OnStreamReady(job->server_ssl_config(), job->proxy_info(),
-                             stream.release());
+    bool use_relative_url = direct || url().SchemeIs("https");
+    delegate_->OnStreamReady(
+        job->server_ssl_config(),
+        job->proxy_info(),
+        new SpdyHttpStream(spdy_session, use_relative_url));
   }
   // |this| may be deleted after this point.
-  if (spdy_session) {
-    factory->OnNewSpdySessionReady(spdy_session,
-                                   direct,
-                                   used_ssl_config,
-                                   used_proxy_info,
-                                   was_npn_negotiated,
-                                   protocol_negotiated,
-                                   using_spdy,
-                                   net_log);
-  }
+  factory->OnNewSpdySessionReady(spdy_session,
+                                 direct,
+                                 used_ssl_config,
+                                 used_proxy_info,
+                                 was_npn_negotiated,
+                                 protocol_negotiated,
+                                 using_spdy,
+                                 net_log);
 }
 
 void HttpStreamFactoryImpl::Request::OrphanJobsExcept(Job* job) {
diff --git a/net/http/http_stream_factory_impl_request.h b/net/http/http_stream_factory_impl_request.h
index 22c8ff2..3d3b2c8 100644
--- a/net/http/http_stream_factory_impl_request.h
+++ b/net/http/http_stream_factory_impl_request.h
@@ -16,7 +16,6 @@
 namespace net {
 
 class ClientSocketHandle;
-class HttpStream;
 class SpdySession;
 
 class HttpStreamFactoryImpl::Request : public HttpStreamRequest {
@@ -65,7 +64,6 @@ class HttpStreamFactoryImpl::Request : public HttpStreamRequest {
 
   // Called by an attached Job if it sets up a SpdySession.
   void OnNewSpdySessionReady(Job* job,
-                             scoped_ptr<HttpStream> stream,
                              const base::WeakPtr<SpdySession>& spdy_session,
                              bool direct);
 
diff --git a/net/spdy/spdy_session.cc b/net/spdy/spdy_session.cc
index 5f0e285..c20d89e 100644
--- a/net/spdy/spdy_session.cc
+++ b/net/spdy/spdy_session.cc
@@ -532,7 +532,7 @@ SpdySession::~SpdySession() {
   net_log_.EndEvent(NetLog::TYPE_SPDY_SESSION);
 }
 
-void SpdySession::InitializeWithSocket(
+Error SpdySession::InitializeWithSocket(
     scoped_ptr<ClientSocketHandle> connection,
     SpdySessionPool* pool,
     bool is_secure,
@@ -593,17 +593,19 @@ void SpdySession::InitializeWithSocket(
       NetLog::TYPE_SPDY_SESSION_INITIALIZED,
       connection_->socket()->NetLog().source().ToEventParametersCallback());
 
-  DCHECK_NE(availability_state_, STATE_CLOSED);
-  connection_->AddHigherLayeredPool(this);
-  if (enable_sending_initial_data_)
-    SendInitialData();
-  pool_ = pool;
-
-  // Bootstrap the read loop.
-  base::MessageLoop::current()->PostTask(
-      FROM_HERE,
-      base::Bind(&SpdySession::PumpReadLoop,
-                 weak_factory_.GetWeakPtr(), READ_STATE_DO_READ, OK));
+  int error = DoReadLoop(READ_STATE_DO_READ, OK);
+  if (error == ERR_IO_PENDING)
+    error = OK;
+  if (error == OK) {
+    DCHECK_NE(availability_state_, STATE_CLOSED);
+    connection_->AddHigherLayeredPool(this);
+    if (enable_sending_initial_data_)
+      SendInitialData();
+    pool_ = pool;
+  } else {
+    DcheckClosed();
+  }
+  return static_cast<Error>(error);
 }
 
 bool SpdySession::VerifyDomainAuthentication(const std::string& domain) {
@@ -1552,8 +1554,9 @@ SpdySession::CloseSessionResult SpdySession::DoCloseSession(
   UMA_HISTOGRAM_CUSTOM_COUNTS("Net.SpdySession.BytesRead.OtherErrors",
                               total_bytes_received_, 1, 100000000, 50);
 
-  DCHECK(pool_);
-  if (availability_state_ != STATE_GOING_AWAY)
+  // |pool_| will be NULL when |InitializeWithSocket()| is in the
+  // call stack.
+  if (pool_ && availability_state_ != STATE_GOING_AWAY)
     pool_->MakeSessionUnavailable(GetWeakPtr());
 
   availability_state_ = STATE_CLOSED;
@@ -1625,8 +1628,10 @@ void SpdySession::CloseSessionOnError(Error err,
 void SpdySession::MakeUnavailable() {
   if (availability_state_ < STATE_GOING_AWAY) {
     availability_state_ = STATE_GOING_AWAY;
-    DCHECK(pool_);
-    pool_->MakeSessionUnavailable(GetWeakPtr());
+    // |pool_| will be NULL when |InitializeWithSocket()| is in the
+    // call stack.
+    if (pool_)
+      pool_->MakeSessionUnavailable(GetWeakPtr());
   }
 }
 
@@ -1681,8 +1686,7 @@ base::Value* SpdySession::GetInfoAsValue() const {
 }
 
 bool SpdySession::IsReused() const {
-  return buffered_spdy_framer_->frames_received() > 0 ||
-      connection_->reuse_type() == ClientSocketHandle::UNUSED_IDLE;
+  return buffered_spdy_framer_->frames_received() > 0;
 }
 
 bool SpdySession::GetLoadTimingInfo(SpdyStreamId stream_id,
diff --git a/net/spdy/spdy_session.h b/net/spdy/spdy_session.h
index 203f1ef..cbb87bf 100644
--- a/net/spdy/spdy_session.h
+++ b/net/spdy/spdy_session.h
@@ -256,13 +256,13 @@ class NET_EXPORT SpdySession : public BufferedSpdyFramerVisitorInterface,
   // |certificate_error_code| must either be OK or less than
   // ERR_IO_PENDING.
   //
-  // The session begins reading from |connection| on a subsequent event loop
-  // iteration, so the SpdySession may close immediately afterwards if the first
-  // read of |connection| fails.
-  void InitializeWithSocket(scoped_ptr<ClientSocketHandle> connection,
-                            SpdySessionPool* pool,
-                            bool is_secure,
-                            int certificate_error_code);
+  // Returns OK on success, or an error on failure. Never returns
+  // ERR_IO_PENDING. If an error is returned, the session must be
+  // destroyed immediately.
+  Error InitializeWithSocket(scoped_ptr<ClientSocketHandle> connection,
+                             SpdySessionPool* pool,
+                             bool is_secure,
+                             int certificate_error_code);
 
   // Returns the protocol used by this session. Always between
   // kProtoSPDYMinimumVersion and kProtoSPDYMaximumVersion.
@@ -367,8 +367,7 @@ class NET_EXPORT SpdySession : public BufferedSpdyFramerVisitorInterface,
   base::Value* GetInfoAsValue() const;
 
   // Indicates whether the session is being reused after having successfully
-  // used to send/receive data in the past or if the underlying socket was idle
-  // before being used for a SPDY session.
+  // used to send/receive data in the past.
   bool IsReused() const;
 
   // Returns true if the underlying transport socket ever had any reads or
@@ -618,7 +617,8 @@ class NET_EXPORT SpdySession : public BufferedSpdyFramerVisitorInterface,
   // Advance the ReadState state machine. |expected_read_state| is the
   // expected starting read state.
   //
-  // This function must always be called via PumpReadLoop().
+  // 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();
diff --git a/net/spdy/spdy_session_pool.cc b/net/spdy/spdy_session_pool.cc
index 6ce0a93..093db1e 100644
--- a/net/spdy/spdy_session_pool.cc
+++ b/net/spdy/spdy_session_pool.cc
@@ -77,11 +77,12 @@ SpdySessionPool::~SpdySessionPool() {
   CertDatabase::GetInstance()->RemoveObserver(this);
 }
 
-base::WeakPtr<SpdySession> SpdySessionPool::CreateAvailableSessionFromSocket(
+net::Error SpdySessionPool::CreateAvailableSessionFromSocket(
     const SpdySessionKey& key,
     scoped_ptr<ClientSocketHandle> connection,
     const BoundNetLog& net_log,
     int certificate_error_code,
+    base::WeakPtr<SpdySession>* available_session,
     bool is_secure) {
   DCHECK_GE(default_protocol_, kProtoSPDYMinimumVersion);
   DCHECK_LE(default_protocol_, kProtoSPDYMaximumVersion);
@@ -104,16 +105,22 @@ base::WeakPtr<SpdySession> SpdySessionPool::CreateAvailableSessionFromSocket(
                       trusted_spdy_proxy_,
                       net_log.net_log()));
 
-  new_session->InitializeWithSocket(
+  Error error =  new_session->InitializeWithSocket(
       connection.Pass(), this, is_secure, certificate_error_code);
+  DCHECK_NE(error, ERR_IO_PENDING);
 
-  base::WeakPtr<SpdySession> available_session = new_session->GetWeakPtr();
+  if (error != OK) {
+    available_session->reset();
+    return error;
+  }
+
+  *available_session = new_session->GetWeakPtr();
   sessions_.insert(new_session.release());
-  MapKeyToAvailableSession(key, available_session);
+  MapKeyToAvailableSession(key, *available_session);
 
   net_log.AddEvent(
       NetLog::TYPE_SPDY_SESSION_POOL_IMPORTED_SESSION_FROM_SOCKET,
-      available_session->net_log().source().ToEventParametersCallback());
+      (*available_session)->net_log().source().ToEventParametersCallback());
 
   // Look up the IP address for this session so that we can match
   // future sessions (potentially to different domains) which can
@@ -122,11 +129,11 @@ base::WeakPtr<SpdySession> SpdySessionPool::CreateAvailableSessionFromSocket(
   // to see if this is a direct connection.
   if (key.proxy_server().is_direct()) {
     IPEndPoint address;
-    if (available_session->GetPeerAddress(&address) == OK)
+    if ((*available_session)->GetPeerAddress(&address) == OK)
       aliases_[address] = key;
   }
 
-  return available_session;
+  return error;
 }
 
 base::WeakPtr<SpdySession> SpdySessionPool::FindAvailableSession(
diff --git a/net/spdy/spdy_session_pool.h b/net/spdy/spdy_session_pool.h
index 0fad5d2..c44e9de 100644
--- a/net/spdy/spdy_session_pool.h
+++ b/net/spdy/spdy_session_pool.h
@@ -79,14 +79,15 @@ class NET_EXPORT SpdySessionPool
   // encountered when connecting the SSL socket, with OK meaning there
   // was no error.
   //
-  // Returns the new SpdySession. Note that the SpdySession begins reading from
-  // |connection| on a subsequent event loop iteration, so it may be closed
-  // immediately afterwards if the first read of |connection| fails.
-  base::WeakPtr<SpdySession> CreateAvailableSessionFromSocket(
+  // If successful, OK is returned and |available_session| will be
+  // non-NULL and available. Otherwise, an error is returned and
+  // |available_session| will be NULL.
+  net::Error CreateAvailableSessionFromSocket(
       const SpdySessionKey& key,
       scoped_ptr<ClientSocketHandle> connection,
       const BoundNetLog& net_log,
       int certificate_error_code,
+      base::WeakPtr<SpdySession>* available_session,
       bool is_secure);
 
   // Find an available session for the given key, or NULL if there isn't one.
diff --git a/net/spdy/spdy_session_unittest.cc b/net/spdy/spdy_session_unittest.cc
index a5d9292..0e8b370 100644
--- a/net/spdy/spdy_session_unittest.cc
+++ b/net/spdy/spdy_session_unittest.cc
@@ -190,12 +190,8 @@ INSTANTIATE_TEST_CASE_P(
 TEST_P(SpdySessionTest, InitialReadError) {
   CreateDeterministicNetworkSession();
 
-  base::WeakPtr<SpdySession> session = TryCreateFakeSpdySessionExpectingFailure(
+  TryCreateFakeSpdySessionExpectingFailure(
       spdy_session_pool_, key_, ERR_FAILED);
-  EXPECT_TRUE(session);
-  // Flush the read.
-  base::RunLoop().RunUntilIdle();
-  EXPECT_FALSE(session);
 }
 
 namespace {
@@ -281,6 +277,8 @@ TEST_P(SpdySessionTest, PendingStreamCancellingAnother) {
   session->CloseSessionOnError(ERR_ABORTED, "Aborting session");
 
   EXPECT_EQ(ERR_ABORTED, callback1.WaitForResult());
+
+  data.RunFor(1);
 }
 
 // A session receiving a GOAWAY frame with no active streams should
@@ -332,12 +330,9 @@ TEST_P(SpdySessionTest, GoAwayImmediatelyWithNoActiveStreams) {
 
   data.StopAfter(1);
 
-  base::WeakPtr<SpdySession> session =
-      TryCreateInsecureSpdySessionExpectingFailure(
-          http_session_, key_, ERR_CONNECTION_CLOSED, BoundNetLog());
-  base::RunLoop().RunUntilIdle();
+  TryCreateInsecureSpdySessionExpectingFailure(
+      http_session_, key_, ERR_CONNECTION_CLOSED, BoundNetLog());
 
-  EXPECT_FALSE(session);
   EXPECT_FALSE(HasSpdySession(spdy_session_pool_, key_));
 }
 
@@ -1050,6 +1045,8 @@ TEST_P(SpdySessionTest, FailedPing) {
 
   EXPECT_TRUE(session == NULL);
   EXPECT_FALSE(HasSpdySession(spdy_session_pool_, key_));
+
+  data.RunFor(1);
   EXPECT_EQ(NULL, spdy_stream1.get());
 }
 
diff --git a/net/spdy/spdy_test_util_common.cc b/net/spdy/spdy_test_util_common.cc
index 22ff5f6..66a6f8c3 100644
--- a/net/spdy/spdy_test_util_common.cc
+++ b/net/spdy/spdy_test_util_common.cc
@@ -540,12 +540,15 @@ base::WeakPtr<SpdySession> CreateSpdySessionHelper(
 
   EXPECT_EQ(OK, rv);
 
-  base::WeakPtr<SpdySession> spdy_session =
+  base::WeakPtr<SpdySession> spdy_session;
+  EXPECT_EQ(
+      expected_status,
       http_session->spdy_session_pool()->CreateAvailableSessionFromSocket(
-          key, connection.Pass(), net_log, OK, is_secure);
-  // Failure is reported asynchronously.
-  EXPECT_TRUE(spdy_session != NULL);
-  EXPECT_TRUE(HasSpdySession(http_session->spdy_session_pool(), key));
+          key, connection.Pass(), net_log, OK, &spdy_session,
+          is_secure));
+  EXPECT_EQ(expected_status == OK, spdy_session != NULL);
+  EXPECT_EQ(expected_status == OK,
+            HasSpdySession(http_session->spdy_session_pool(), key));
   return spdy_session;
 }
 
@@ -559,14 +562,14 @@ base::WeakPtr<SpdySession> CreateInsecureSpdySession(
                                  OK, false /* is_secure */);
 }
 
-base::WeakPtr<SpdySession> TryCreateInsecureSpdySessionExpectingFailure(
+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);
-  return CreateSpdySessionHelper(http_session, key, net_log,
-                                 expected_error, false /* is_secure */);
+  CreateSpdySessionHelper(http_session, key, net_log,
+                          expected_error, false /* is_secure */);
 }
 
 base::WeakPtr<SpdySession> CreateSecureSpdySession(
@@ -640,15 +643,17 @@ base::WeakPtr<SpdySession> CreateFakeSpdySessionHelper(
     Error expected_status) {
   EXPECT_NE(expected_status, ERR_IO_PENDING);
   EXPECT_FALSE(HasSpdySession(pool, key));
+  base::WeakPtr<SpdySession> spdy_session;
   scoped_ptr<ClientSocketHandle> handle(new ClientSocketHandle());
   handle->SetSocket(scoped_ptr<StreamSocket>(new FakeSpdySessionClientSocket(
       expected_status == OK ? ERR_IO_PENDING : expected_status)));
-  base::WeakPtr<SpdySession> spdy_session =
+  EXPECT_EQ(
+      expected_status,
       pool->CreateAvailableSessionFromSocket(
-          key, handle.Pass(), BoundNetLog(), OK, true /* is_secure */);
-  // Failure is reported asynchronously.
-  EXPECT_TRUE(spdy_session != NULL);
-  EXPECT_TRUE(HasSpdySession(pool, key));
+          key, handle.Pass(), BoundNetLog(), OK, &spdy_session,
+          true /* is_secure */));
+  EXPECT_EQ(expected_status == OK, spdy_session != NULL);
+  EXPECT_EQ(expected_status == OK, HasSpdySession(pool, key));
   return spdy_session;
 }
 
@@ -659,12 +664,11 @@ base::WeakPtr<SpdySession> CreateFakeSpdySession(SpdySessionPool* pool,
   return CreateFakeSpdySessionHelper(pool, key, OK);
 }
 
-base::WeakPtr<SpdySession> TryCreateFakeSpdySessionExpectingFailure(
-    SpdySessionPool* pool,
-    const SpdySessionKey& key,
-    Error expected_error) {
+void TryCreateFakeSpdySessionExpectingFailure(SpdySessionPool* pool,
+                                              const SpdySessionKey& key,
+                                              Error expected_error) {
   DCHECK_LT(expected_error, ERR_IO_PENDING);
-  return CreateFakeSpdySessionHelper(pool, key, expected_error);
+  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 668a7e8..42b595b 100644
--- a/net/spdy/spdy_test_util_common.h
+++ b/net/spdy/spdy_test_util_common.h
@@ -247,9 +247,8 @@ base::WeakPtr<SpdySession> CreateInsecureSpdySession(
 
 // 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. The session will be created but close in the
-// next event loop iteration.
-base::WeakPtr<SpdySession> TryCreateInsecureSpdySessionExpectingFailure(
+// not already exist.
+void TryCreateInsecureSpdySessionExpectingFailure(
     const scoped_refptr<HttpNetworkSession>& http_session,
     const SpdySessionKey& key,
     Error expected_error,
@@ -270,12 +269,10 @@ base::WeakPtr<SpdySession> CreateFakeSpdySession(SpdySessionPool* pool,
 // 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. The session will be created but close in the
-// next event loop iteration.
-base::WeakPtr<SpdySession> TryCreateFakeSpdySessionExpectingFailure(
-    SpdySessionPool* pool,
-    const SpdySessionKey& key,
-    Error expected_error);
+// not already exist.
+void TryCreateFakeSpdySessionExpectingFailure(SpdySessionPool* pool,
+                                              const SpdySessionKey& key,
+                                              Error expected_error);
 
 class SpdySessionPoolPeer {
  public: