diff options
| -rw-r--r-- | net/http/http_network_transaction_unittest.cc | 122 | ||||
| -rw-r--r-- | net/http/http_proxy_client_socket_pool.cc | 10 | ||||
| -rw-r--r-- | net/http/http_stream_factory_impl_job.cc | 31 | ||||
| -rw-r--r-- | net/http/http_stream_factory_impl_request.cc | 30 | ||||
| -rw-r--r-- | net/http/http_stream_factory_impl_request.h | 2 | ||||
| -rw-r--r-- | net/spdy/spdy_session.cc | 40 | ||||
| -rw-r--r-- | net/spdy/spdy_session.h | 20 | ||||
| -rw-r--r-- | net/spdy/spdy_session_pool.cc | 21 | ||||
| -rw-r--r-- | net/spdy/spdy_session_pool.h | 9 | ||||
| -rw-r--r-- | net/spdy/spdy_session_unittest.cc | 17 | ||||
| -rw-r--r-- | net/spdy/spdy_test_util_common.cc | 40 | ||||
| -rw-r--r-- | net/spdy/spdy_test_util_common.h | 15 |
12 files changed, 213 insertions, 144 deletions
diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc index 7dda13e..ca06785 100644 --- a/net/http/http_network_transaction_unittest.cc +++ b/net/http/http_network_transaction_unittest.cc @@ -289,7 +289,8 @@ 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); + const MockRead* read_failure, + bool use_spdy); SimpleGetHelperResult SimpleGetHelperForData(StaticSocketDataProvider* data[], size_t data_count) { @@ -1308,46 +1309,81 @@ void HttpNetworkTransactionTest::KeepAliveConnectionResendRequestTest( void HttpNetworkTransactionTest::PreconnectErrorResendRequestTest( const MockWrite* write_failure, - const MockRead* read_failure) { + const MockRead* read_failure, + bool use_spdy) { HttpRequestInfo request; request.method = "GET"; - request.url = GURL("http://www.foo.com/"); + request.url = GURL("https://www.foo.com/"); request.load_flags = 0; CapturingNetLog net_log; session_deps_.net_log = &net_log; scoped_refptr<HttpNetworkSession> session(CreateSession(&session_deps_)); - // 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"), - }; + 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); - // Read results for the first request. - MockRead data1_reads[] = { - MockRead(ASYNC, OK), - }; + // 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)); + // 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"; + + std::vector<MockRead> data1_reads; + std::vector<MockWrite> data1_writes; if (write_failure) { ASSERT_FALSE(read_failure); - data1_writes[0] = *write_failure; + data1_writes.push_back(*write_failure); + data1_reads.push_back(MockRead(ASYNC, OK)); } else { ASSERT_TRUE(read_failure); - data1_reads[0] = *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); } - StaticSocketDataProvider data1(data1_reads, arraysize(data1_reads), - data1_writes, arraysize(data1_writes)); + StaticSocketDataProvider data1(&data1_reads[0], data1_reads.size(), + &data1_writes[0], data1_writes.size()); session_deps_.socket_factory->AddSocketDataProvider(&data1); - 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); + 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()); session_deps_.socket_factory->AddSocketDataProvider(&data2); // Preconnect a socket. @@ -1360,7 +1396,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, GetIdleSocketCountInTransportSocketPool(session.get())); + EXPECT_EQ(1, GetIdleSocketCountInSSLSocketPool(session.get())); // Make the request. TestCompletionCallback callback; @@ -1376,7 +1412,9 @@ void HttpNetworkTransactionTest::PreconnectErrorResendRequestTest( LoadTimingInfo load_timing_info; EXPECT_TRUE(trans->GetLoadTimingInfo(&load_timing_info)); - TestLoadTimingNotReused(load_timing_info, CONNECT_TIMING_HAS_DNS_TIMES); + TestLoadTimingNotReused( + load_timing_info, + CONNECT_TIMING_HAS_DNS_TIMES|CONNECT_TIMING_HAS_SSL_TIMES); const HttpResponseInfo* response = trans->GetResponseInfo(); ASSERT_TRUE(response != NULL); @@ -1387,7 +1425,7 @@ void HttpNetworkTransactionTest::PreconnectErrorResendRequestTest( std::string response_data; rv = ReadTransaction(trans.get(), &response_data); EXPECT_EQ(OK, rv); - EXPECT_EQ("hello", response_data); + EXPECT_EQ(kHttpData, response_data); } TEST_P(HttpNetworkTransactionTest, @@ -1409,17 +1447,43 @@ TEST_P(HttpNetworkTransactionTest, KeepAliveConnectionEOF) { TEST_P(HttpNetworkTransactionTest, PreconnectErrorNotConnectedOnWrite) { MockWrite write_failure(ASYNC, ERR_SOCKET_NOT_CONNECTED); - PreconnectErrorResendRequestTest(&write_failure, NULL); + PreconnectErrorResendRequestTest(&write_failure, NULL, false); } TEST_P(HttpNetworkTransactionTest, PreconnectErrorReset) { MockRead read_failure(ASYNC, ERR_CONNECTION_RESET); - PreconnectErrorResendRequestTest(NULL, &read_failure); + PreconnectErrorResendRequestTest(NULL, &read_failure, false); } TEST_P(HttpNetworkTransactionTest, PreconnectErrorEOF) { MockRead read_failure(SYNCHRONOUS, OK); // EOF - PreconnectErrorResendRequestTest(NULL, &read_failure); + 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); } 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 8d691b7..e2529f8 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 - int rv = spdy_pool->CreateAvailableSessionFromSocket( - key, transport_socket_handle_.Pass(), - net_log(), OK, &spdy_session, /*using_ssl_*/ true); - if (rv < 0) - return rv; + spdy_session = + spdy_pool->CreateAvailableSessionFromSocket( + key, transport_socket_handle_.Pass(), + net_log(), OK, /*using_ssl_*/ true); + DCHECK(spdy_session); } 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 960bc58..d595f2a 100644 --- a/net/http/http_stream_factory_impl_job.cc +++ b/net/http/http_stream_factory_impl_job.cc @@ -334,20 +334,23 @@ void HttpStreamFactoryImpl::Job::OnWebSocketHandshakeStreamReadyCallback() { } void HttpStreamFactoryImpl::Job::OnNewSpdySessionReadyCallback() { - DCHECK(!stream_.get()); + DCHECK(stream_.get()); DCHECK(!IsPreconnecting()); DCHECK(using_spdy()); - if (!new_spdy_session_) - return; + // Note: an event loop iteration has passed, so |new_spdy_session_| may be + // NULL at this point if the SpdySession closed immediately after creation. base::WeakPtr<SpdySession> spdy_session = new_spdy_session_; new_spdy_session_.reset(); if (IsOrphaned()) { - stream_factory_->OnNewSpdySessionReady( - spdy_session, spdy_session_direct_, server_ssl_config_, proxy_info_, - was_npn_negotiated(), protocol_negotiated(), using_spdy(), net_log_); + 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_->OnOrphanedJobComplete(this); } else { - request_->OnNewSpdySessionReady(this, spdy_session, spdy_session_direct_); + request_->OnNewSpdySessionReady( + this, stream_.Pass(), spdy_session, spdy_session_direct_); } // |this| may be deleted after this call. } @@ -1104,21 +1107,27 @@ int HttpStreamFactoryImpl::Job::DoCreateStream() { SpdySessionPool* spdy_pool = session_->spdy_session_pool(); spdy_session = spdy_pool->FindAvailableSession(spdy_session_key, net_log_); if (!spdy_session) { - int error = + new_spdy_session_ = 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 ff66f14..5f129ac 100644 --- a/net/http/http_stream_factory_impl_request.cc +++ b/net/http/http_stream_factory_impl_request.cc @@ -291,11 +291,16 @@ 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); @@ -322,21 +327,20 @@ void HttpStreamFactoryImpl::Request::OnNewSpdySessionReady( // implemented. NOTREACHED(); } else { - bool use_relative_url = direct || url().SchemeIs("https"); - delegate_->OnStreamReady( - job->server_ssl_config(), - job->proxy_info(), - new SpdyHttpStream(spdy_session, use_relative_url)); + delegate_->OnStreamReady(job->server_ssl_config(), job->proxy_info(), + stream.release()); } // |this| may be deleted after this point. - factory->OnNewSpdySessionReady(spdy_session, - direct, - used_ssl_config, - used_proxy_info, - was_npn_negotiated, - protocol_negotiated, - using_spdy, - net_log); + if (spdy_session) { + 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 3d3b2c8..22c8ff2 100644 --- a/net/http/http_stream_factory_impl_request.h +++ b/net/http/http_stream_factory_impl_request.h @@ -16,6 +16,7 @@ namespace net { class ClientSocketHandle; +class HttpStream; class SpdySession; class HttpStreamFactoryImpl::Request : public HttpStreamRequest { @@ -64,6 +65,7 @@ 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 c20d89e..5f0e285 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); } -Error SpdySession::InitializeWithSocket( +void SpdySession::InitializeWithSocket( scoped_ptr<ClientSocketHandle> connection, SpdySessionPool* pool, bool is_secure, @@ -593,19 +593,17 @@ Error SpdySession::InitializeWithSocket( NetLog::TYPE_SPDY_SESSION_INITIALIZED, connection_->socket()->NetLog().source().ToEventParametersCallback()); - 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); + 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)); } bool SpdySession::VerifyDomainAuthentication(const std::string& domain) { @@ -1554,9 +1552,8 @@ SpdySession::CloseSessionResult SpdySession::DoCloseSession( 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) + DCHECK(pool_); + if (availability_state_ != STATE_GOING_AWAY) pool_->MakeSessionUnavailable(GetWeakPtr()); availability_state_ = STATE_CLOSED; @@ -1628,10 +1625,8 @@ void SpdySession::CloseSessionOnError(Error err, void SpdySession::MakeUnavailable() { 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(GetWeakPtr()); + DCHECK(pool_); + pool_->MakeSessionUnavailable(GetWeakPtr()); } } @@ -1686,7 +1681,8 @@ base::Value* SpdySession::GetInfoAsValue() const { } bool SpdySession::IsReused() const { - return buffered_spdy_framer_->frames_received() > 0; + return buffered_spdy_framer_->frames_received() > 0 || + connection_->reuse_type() == ClientSocketHandle::UNUSED_IDLE; } bool SpdySession::GetLoadTimingInfo(SpdyStreamId stream_id, diff --git a/net/spdy/spdy_session.h b/net/spdy/spdy_session.h index cbb87bf..203f1ef 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. // - // 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); + // 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 the protocol used by this session. Always between // kProtoSPDYMinimumVersion and kProtoSPDYMaximumVersion. @@ -367,7 +367,8 @@ 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. + // used to send/receive data in the past or if the underlying socket was idle + // before being used for a SPDY session. bool IsReused() const; // Returns true if the underlying transport socket ever had any reads or @@ -617,8 +618,7 @@ 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() except for - // from InitializeWithSocket(). + // This function must always be called via PumpReadLoop(). 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 093db1e..6ce0a93 100644 --- a/net/spdy/spdy_session_pool.cc +++ b/net/spdy/spdy_session_pool.cc @@ -77,12 +77,11 @@ SpdySessionPool::~SpdySessionPool() { CertDatabase::GetInstance()->RemoveObserver(this); } -net::Error SpdySessionPool::CreateAvailableSessionFromSocket( +base::WeakPtr<SpdySession> 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); @@ -105,22 +104,16 @@ net::Error SpdySessionPool::CreateAvailableSessionFromSocket( trusted_spdy_proxy_, net_log.net_log())); - Error error = new_session->InitializeWithSocket( + new_session->InitializeWithSocket( connection.Pass(), this, is_secure, certificate_error_code); - DCHECK_NE(error, ERR_IO_PENDING); - if (error != OK) { - available_session->reset(); - return error; - } - - *available_session = new_session->GetWeakPtr(); + base::WeakPtr<SpdySession> 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 @@ -129,11 +122,11 @@ net::Error 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 error; + return available_session; } base::WeakPtr<SpdySession> SpdySessionPool::FindAvailableSession( diff --git a/net/spdy/spdy_session_pool.h b/net/spdy/spdy_session_pool.h index c44e9de..0fad5d2 100644 --- a/net/spdy/spdy_session_pool.h +++ b/net/spdy/spdy_session_pool.h @@ -79,15 +79,14 @@ class NET_EXPORT SpdySessionPool // encountered when connecting the SSL socket, with OK meaning there // was no error. // - // 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( + // 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( 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 0e8b370..a5d9292 100644 --- a/net/spdy/spdy_session_unittest.cc +++ b/net/spdy/spdy_session_unittest.cc @@ -190,8 +190,12 @@ INSTANTIATE_TEST_CASE_P( TEST_P(SpdySessionTest, InitialReadError) { CreateDeterministicNetworkSession(); - TryCreateFakeSpdySessionExpectingFailure( + base::WeakPtr<SpdySession> session = TryCreateFakeSpdySessionExpectingFailure( spdy_session_pool_, key_, ERR_FAILED); + EXPECT_TRUE(session); + // Flush the read. + base::RunLoop().RunUntilIdle(); + EXPECT_FALSE(session); } namespace { @@ -277,8 +281,6 @@ 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 @@ -330,9 +332,12 @@ TEST_P(SpdySessionTest, GoAwayImmediatelyWithNoActiveStreams) { data.StopAfter(1); - TryCreateInsecureSpdySessionExpectingFailure( - http_session_, key_, ERR_CONNECTION_CLOSED, BoundNetLog()); + base::WeakPtr<SpdySession> session = + TryCreateInsecureSpdySessionExpectingFailure( + http_session_, key_, ERR_CONNECTION_CLOSED, BoundNetLog()); + base::RunLoop().RunUntilIdle(); + EXPECT_FALSE(session); EXPECT_FALSE(HasSpdySession(spdy_session_pool_, key_)); } @@ -1045,8 +1050,6 @@ 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 66a6f8c3..22ff5f6 100644 --- a/net/spdy/spdy_test_util_common.cc +++ b/net/spdy/spdy_test_util_common.cc @@ -540,15 +540,12 @@ base::WeakPtr<SpdySession> CreateSpdySessionHelper( EXPECT_EQ(OK, rv); - base::WeakPtr<SpdySession> spdy_session; - EXPECT_EQ( - expected_status, + base::WeakPtr<SpdySession> spdy_session = http_session->spdy_session_pool()->CreateAvailableSessionFromSocket( - 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)); + 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)); return spdy_session; } @@ -562,14 +559,14 @@ base::WeakPtr<SpdySession> CreateInsecureSpdySession( OK, false /* is_secure */); } -void TryCreateInsecureSpdySessionExpectingFailure( +base::WeakPtr<SpdySession> 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 */); + return CreateSpdySessionHelper(http_session, key, net_log, + expected_error, false /* is_secure */); } base::WeakPtr<SpdySession> CreateSecureSpdySession( @@ -643,17 +640,15 @@ 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))); - EXPECT_EQ( - expected_status, + base::WeakPtr<SpdySession> spdy_session = pool->CreateAvailableSessionFromSocket( - 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)); + key, handle.Pass(), BoundNetLog(), OK, true /* is_secure */); + // Failure is reported asynchronously. + EXPECT_TRUE(spdy_session != NULL); + EXPECT_TRUE(HasSpdySession(pool, key)); return spdy_session; } @@ -664,11 +659,12 @@ base::WeakPtr<SpdySession> CreateFakeSpdySession(SpdySessionPool* pool, return CreateFakeSpdySessionHelper(pool, key, OK); } -void TryCreateFakeSpdySessionExpectingFailure(SpdySessionPool* pool, - const SpdySessionKey& key, - Error expected_error) { +base::WeakPtr<SpdySession> TryCreateFakeSpdySessionExpectingFailure( + SpdySessionPool* pool, + const SpdySessionKey& key, + Error expected_error) { DCHECK_LT(expected_error, ERR_IO_PENDING); - CreateFakeSpdySessionHelper(pool, key, expected_error); + return 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 42b595b..668a7e8 100644 --- a/net/spdy/spdy_test_util_common.h +++ b/net/spdy/spdy_test_util_common.h @@ -247,8 +247,9 @@ 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. -void TryCreateInsecureSpdySessionExpectingFailure( +// not already exist. The session will be created but close in the +// next event loop iteration. +base::WeakPtr<SpdySession> TryCreateInsecureSpdySessionExpectingFailure( const scoped_refptr<HttpNetworkSession>& http_session, const SpdySessionKey& key, Error expected_error, @@ -269,10 +270,12 @@ 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. -void TryCreateFakeSpdySessionExpectingFailure(SpdySessionPool* pool, - const SpdySessionKey& key, - Error expected_error); +// 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); class SpdySessionPoolPeer { public: |