diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc index e94e4a326f1cab..e9723e67b33d88 100644 --- a/net/http/http_network_transaction_unittest.cc +++ b/net/http/http_network_transaction_unittest.cc @@ -63,6 +63,7 @@ #include "net/test/cert_test_util.h" #include "testing/gtest/include/gtest/gtest.h" #include "testing/platform_test.h" +#include "url/gurl.h" //----------------------------------------------------------------------------- @@ -8855,7 +8856,7 @@ TEST_P(HttpNetworkTransactionTest, HostPortPair host_port_pair("www.google.com", 443); SpdySessionKey key(host_port_pair, ProxyServer::Direct(), kPrivacyModeDisabled); - scoped_refptr spdy_session = + base::WeakPtr spdy_session = CreateSecureSpdySession(session, key, BoundNetLog()); trans.reset(new HttpNetworkTransaction(DEFAULT_PRIORITY, session.get())); @@ -9578,6 +9579,29 @@ TEST_P(HttpNetworkTransactionTest, SpdyPostNPNServerHangup) { EXPECT_EQ(ERR_CONNECTION_CLOSED, callback.WaitForResult()); } +// A subclass of HttpAuthHandlerMock that records the request URL when +// it gets it. This is needed since the auth handler may get destroyed +// before we get a chance to query it. +class UrlRecordingHttpAuthHandlerMock : public HttpAuthHandlerMock { + public: + explicit UrlRecordingHttpAuthHandlerMock(GURL* url) : url_(url) {} + + virtual ~UrlRecordingHttpAuthHandlerMock() {} + + protected: + virtual int GenerateAuthTokenImpl(const AuthCredentials* credentials, + const HttpRequestInfo* request, + const CompletionCallback& callback, + std::string* auth_token) OVERRIDE { + *url_ = request->url; + return HttpAuthHandlerMock::GenerateAuthTokenImpl( + credentials, request, callback, auth_token); + } + + private: + GURL* url_; +}; + TEST_P(HttpNetworkTransactionTest, SpdyAlternateProtocolThroughProxy) { // This test ensures that the URL passed into the proxy is upgraded // to https when doing an Alternate Protocol upgrade. @@ -9588,12 +9612,16 @@ TEST_P(HttpNetworkTransactionTest, SpdyAlternateProtocolThroughProxy) { ProxyService::CreateFixedFromPacResult("PROXY myproxy:70")); CapturingNetLog net_log; session_deps_.net_log = &net_log; - HttpAuthHandlerMock::Factory* auth_factory = - new HttpAuthHandlerMock::Factory(); - HttpAuthHandlerMock* auth_handler = new HttpAuthHandlerMock(); - auth_factory->AddMockHandler(auth_handler, HttpAuth::AUTH_PROXY); - auth_factory->set_do_init_from_challenge(true); - session_deps_.http_auth_handler_factory.reset(auth_factory); + GURL request_url; + { + HttpAuthHandlerMock::Factory* auth_factory = + new HttpAuthHandlerMock::Factory(); + UrlRecordingHttpAuthHandlerMock* auth_handler = + new UrlRecordingHttpAuthHandlerMock(&request_url); + auth_factory->AddMockHandler(auth_handler, HttpAuth::AUTH_PROXY); + auth_factory->set_do_init_from_challenge(true); + session_deps_.http_auth_handler_factory.reset(auth_factory); + } HttpRequestInfo request; request.method = "GET"; @@ -9724,7 +9752,6 @@ TEST_P(HttpNetworkTransactionTest, SpdyAlternateProtocolThroughProxy) { // After all that work, these two lines (or actually, just the scheme) are // what this test is all about. Make sure it happens correctly. - const GURL& request_url = auth_handler->request_url(); EXPECT_EQ("https", request_url.scheme()); EXPECT_EQ("www.google.com", request_url.host()); @@ -9987,7 +10014,7 @@ TEST_P(HttpNetworkTransactionTest, PreconnectWithExistingSpdySession) { HostPortPair host_port_pair("www.google.com", 443); SpdySessionKey key(host_port_pair, ProxyServer::Direct(), kPrivacyModeDisabled); - scoped_refptr spdy_session = + base::WeakPtr spdy_session = CreateInsecureSpdySession(session, key, BoundNetLog()); HttpRequestInfo request; diff --git a/net/http/http_proxy_client_socket_pool.cc b/net/http/http_proxy_client_socket_pool.cc index 5249f5b4072376..b80df37b3dd0f3 100644 --- a/net/http/http_proxy_client_socket_pool.cc +++ b/net/http/http_proxy_client_socket_pool.cc @@ -302,7 +302,7 @@ int HttpProxyConnectJob::DoSpdyProxyCreateStream() { ProxyServer::Direct(), kPrivacyModeDisabled); SpdySessionPool* spdy_pool = params_->spdy_session_pool(); - scoped_refptr spdy_session = + base::WeakPtr spdy_session = spdy_pool->FindAvailableSession(key, net_log()); // It's possible that a session to the proxy has recently been created if (spdy_session) { diff --git a/net/http/http_stream_factory_impl.cc b/net/http/http_stream_factory_impl.cc index c54d92e0c89a73..35d94d779f8cef 100644 --- a/net/http/http_stream_factory_impl.cc +++ b/net/http/http_stream_factory_impl.cc @@ -263,7 +263,7 @@ void HttpStreamFactoryImpl::OrphanJob(Job* job, const Request* request) { } void HttpStreamFactoryImpl::OnNewSpdySessionReady( - scoped_refptr spdy_session, + const base::WeakPtr& spdy_session, bool direct, const SSLConfig& used_ssl_config, const ProxyInfo& used_proxy_info, @@ -271,15 +271,16 @@ void HttpStreamFactoryImpl::OnNewSpdySessionReady( NextProto protocol_negotiated, bool using_spdy, const BoundNetLog& net_log) { - const SpdySessionKey& spdy_session_key = - spdy_session->spdy_session_key(); - while (!spdy_session->IsClosed()) { - // Each iteration may empty out the RequestSet for |spdy_session_key_ in + while (true) { + if (!spdy_session) + break; + const SpdySessionKey& spdy_session_key = spdy_session->spdy_session_key(); + // Each iteration may empty out the RequestSet for |spdy_session_key| in // |spdy_session_request_map_|. So each time, check for RequestSet and use // the first one. // // TODO(willchan): If it's important, switch RequestSet out for a FIFO - // pqueue (Order by priority first, then FIFO within same priority). Unclear + // queue (Order by priority first, then FIFO within same priority). Unclear // that it matters here. if (!ContainsKey(spdy_session_request_map_, spdy_session_key)) break; @@ -297,14 +298,14 @@ void HttpStreamFactoryImpl::OnNewSpdySessionReady( NULL, used_ssl_config, used_proxy_info, - factory->CreateSpdyStream(spdy_session.get(), use_relative_url)); + factory->CreateSpdyStream(spdy_session, use_relative_url)); } else { bool use_relative_url = direct || request->url().SchemeIs("https"); request->OnStreamReady( NULL, used_ssl_config, used_proxy_info, - new SpdyHttpStream(spdy_session.get(), use_relative_url)); + new SpdyHttpStream(spdy_session, use_relative_url)); } } // TODO(mbelshe): Alert other valid requests. diff --git a/net/http/http_stream_factory_impl.h b/net/http/http_stream_factory_impl.h index 3cfda3c41fedac..3949f3839ee21a 100644 --- a/net/http/http_stream_factory_impl.h +++ b/net/http/http_stream_factory_impl.h @@ -94,7 +94,7 @@ class NET_EXPORT_PRIVATE HttpStreamFactoryImpl : // Called when a SpdySession is ready. It will find appropriate Requests and // fulfill them. |direct| indicates whether or not |spdy_session| uses a // proxy. - void OnNewSpdySessionReady(scoped_refptr spdy_session, + void OnNewSpdySessionReady(const base::WeakPtr& spdy_session, bool direct, const SSLConfig& used_ssl_config, const ProxyInfo& used_proxy_info, diff --git a/net/http/http_stream_factory_impl_job.cc b/net/http/http_stream_factory_impl_job.cc index 937d7db54dbae7..b2eee3b0fbeff5 100644 --- a/net/http/http_stream_factory_impl_job.cc +++ b/net/http/http_stream_factory_impl_job.cc @@ -4,6 +4,7 @@ #include "net/http/http_stream_factory_impl_job.h" +#include #include #include "base/bind.h" @@ -330,9 +331,10 @@ void HttpStreamFactoryImpl::Job::OnNewSpdySessionReadyCallback() { DCHECK(!stream_.get()); DCHECK(!IsPreconnecting()); DCHECK(using_spdy()); - DCHECK(new_spdy_session_.get()); - scoped_refptr spdy_session = new_spdy_session_; - new_spdy_session_ = NULL; + if (!new_spdy_session_) + return; + base::WeakPtr spdy_session = new_spdy_session_; + new_spdy_session_.reset(); if (IsOrphaned()) { stream_factory_->OnNewSpdySessionReady( spdy_session, spdy_session_direct_, server_ssl_config_, proxy_info_, @@ -762,7 +764,7 @@ int HttpStreamFactoryImpl::Job::DoInitConnection() { // Check first if we have a spdy session for this group. If so, then go // straight to using that. SpdySessionKey spdy_session_key = GetSpdySessionKey(); - scoped_refptr spdy_session = + base::WeakPtr spdy_session = session_->spdy_session_pool()->FindAvailableSession( spdy_session_key, net_log_); if (spdy_session && CanUseExistingSpdySession()) { @@ -1097,13 +1099,13 @@ int HttpStreamFactoryImpl::Job::DoCreateStream() { direct = false; } - scoped_refptr spdy_session; + base::WeakPtr spdy_session; if (existing_spdy_session_.get()) { // We picked up an existing session, so we don't need our socket. if (connection_->socket()) connection_->socket()->Disconnect(); connection_->Reset(); - spdy_session.swap(existing_spdy_session_); + std::swap(spdy_session, existing_spdy_session_); } else { SpdySessionPool* spdy_pool = session_->spdy_session_pool(); spdy_session = spdy_pool->FindAvailableSession(spdy_session_key, net_log_); @@ -1127,7 +1129,7 @@ int HttpStreamFactoryImpl::Job::DoCreateStream() { } } - if (spdy_session->IsClosed()) + if (!spdy_session) return ERR_CONNECTION_CLOSED; // TODO(willchan): Delete this code, because eventually, the @@ -1140,10 +1142,10 @@ int HttpStreamFactoryImpl::Job::DoCreateStream() { bool use_relative_url = direct || request_info_.url.SchemeIs("wss"); websocket_stream_.reset( request_->websocket_stream_factory()->CreateSpdyStream( - spdy_session.get(), use_relative_url)); + spdy_session, use_relative_url)); } else { bool use_relative_url = direct || request_info_.url.SchemeIs("https"); - stream_.reset(new SpdyHttpStream(spdy_session.get(), use_relative_url)); + stream_.reset(new SpdyHttpStream(spdy_session, use_relative_url)); } return OK; } diff --git a/net/http/http_stream_factory_impl_job.h b/net/http/http_stream_factory_impl_job.h index 6e34f5fe085815..2c2eb349586069 100644 --- a/net/http/http_stream_factory_impl_job.h +++ b/net/http/http_stream_factory_impl_job.h @@ -307,10 +307,10 @@ class HttpStreamFactoryImpl::Job { int num_streams_; // Initialized when we create a new SpdySession. - scoped_refptr new_spdy_session_; + base::WeakPtr new_spdy_session_; // Initialized when we have an existing SpdySession. - scoped_refptr existing_spdy_session_; + base::WeakPtr existing_spdy_session_; // Only used if |new_spdy_session_| is non-NULL. bool spdy_session_direct_; diff --git a/net/http/http_stream_factory_impl_request.cc b/net/http/http_stream_factory_impl_request.cc index 9fb3931462b943..e73a897a528b35 100644 --- a/net/http/http_stream_factory_impl_request.cc +++ b/net/http/http_stream_factory_impl_request.cc @@ -279,7 +279,7 @@ HttpStreamFactoryImpl::Request::RemoveRequestFromHttpPipeliningRequestMap() { void HttpStreamFactoryImpl::Request::OnNewSpdySessionReady( Job* job, - scoped_refptr spdy_session, + const base::WeakPtr& spdy_session, bool direct) { DCHECK(job); DCHECK(job->using_spdy()); @@ -311,17 +311,17 @@ void HttpStreamFactoryImpl::Request::OnNewSpdySessionReady( delegate_->OnWebSocketStreamReady( job->server_ssl_config(), job->proxy_info(), - websocket_stream_factory_->CreateSpdyStream(spdy_session.get(), + websocket_stream_factory_->CreateSpdyStream(spdy_session, use_relative_url)); } else { bool use_relative_url = direct || url().SchemeIs("https"); delegate_->OnStreamReady( job->server_ssl_config(), job->proxy_info(), - new SpdyHttpStream(spdy_session.get(), use_relative_url)); + new SpdyHttpStream(spdy_session, use_relative_url)); } // |this| may be deleted after this point. - factory->OnNewSpdySessionReady(spdy_session.get(), + factory->OnNewSpdySessionReady(spdy_session, direct, used_ssl_config, used_proxy_info, diff --git a/net/http/http_stream_factory_impl_request.h b/net/http/http_stream_factory_impl_request.h index e87768361e423b..169e1f54ce91f0 100644 --- a/net/http/http_stream_factory_impl_request.h +++ b/net/http/http_stream_factory_impl_request.h @@ -63,7 +63,7 @@ class HttpStreamFactoryImpl::Request : public HttpStreamRequest { // Called by an attached Job if it sets up a SpdySession. void OnNewSpdySessionReady(Job* job, - scoped_refptr spdy_session, + const base::WeakPtr& spdy_session, bool direct); WebSocketStreamBase::Factory* websocket_stream_factory() { diff --git a/net/http/http_stream_factory_impl_unittest.cc b/net/http/http_stream_factory_impl_unittest.cc index a2d0990be4081e..be3476a97e147f 100644 --- a/net/http/http_stream_factory_impl_unittest.cc +++ b/net/http/http_stream_factory_impl_unittest.cc @@ -201,7 +201,7 @@ class StreamRequestWaiter : public HttpStreamRequest::Delegate { class WebSocketSpdyStream : public MockWebSocketStream { public: - explicit WebSocketSpdyStream(SpdySession* spdy_session) + explicit WebSocketSpdyStream(const base::WeakPtr& spdy_session) : MockWebSocketStream(kStreamTypeSpdy), spdy_session_(spdy_session) {} virtual ~WebSocketSpdyStream() {} @@ -209,7 +209,7 @@ class WebSocketSpdyStream : public MockWebSocketStream { SpdySession* spdy_session() { return spdy_session_.get(); } private: - scoped_refptr spdy_session_; + base::WeakPtr spdy_session_; }; class WebSocketBasicStream : public MockWebSocketStream { @@ -237,7 +237,7 @@ class WebSocketStreamFactory : public WebSocketStreamBase::Factory { } virtual WebSocketStreamBase* CreateSpdyStream( - SpdySession* spdy_session, + const base::WeakPtr& spdy_session, bool use_relative_url) OVERRIDE { return new WebSocketSpdyStream(spdy_session); } @@ -1022,17 +1022,20 @@ TEST_P(HttpStreamFactoryTest, RequestSpdyHttpStream) { ProxyService::CreateDirect()); MockRead mock_read(ASYNC, OK); - StaticSocketDataProvider socket_data(&mock_read, 1, NULL, 0); + DeterministicSocketData socket_data(&mock_read, 1, NULL, 0); socket_data.set_connect_data(MockConnect(ASYNC, OK)); - session_deps.socket_factory->AddSocketDataProvider(&socket_data); + session_deps.deterministic_socket_factory->AddSocketDataProvider( + &socket_data); SSLSocketDataProvider ssl_socket_data(ASYNC, OK); ssl_socket_data.SetNextProto(GetParam()); - session_deps.socket_factory->AddSSLSocketDataProvider(&ssl_socket_data); + session_deps.deterministic_socket_factory->AddSSLSocketDataProvider( + &ssl_socket_data); HostPortPair host_port_pair("www.google.com", 443); scoped_refptr - session(SpdySessionDependencies::SpdyCreateSession(&session_deps)); + session(SpdySessionDependencies::SpdyCreateSessionDeterministic( + &session_deps)); // Now request a stream. HttpRequestInfo request_info; @@ -1149,21 +1152,25 @@ TEST_P(HttpStreamFactoryTest, OrphanedWebSocketStream) { ProxyService::CreateDirect()); MockRead mock_read(ASYNC, OK); - StaticSocketDataProvider socket_data(&mock_read, 1, NULL, 0); + DeterministicSocketData socket_data(&mock_read, 1, NULL, 0); socket_data.set_connect_data(MockConnect(ASYNC, OK)); - session_deps.socket_factory->AddSocketDataProvider(&socket_data); + session_deps.deterministic_socket_factory->AddSocketDataProvider( + &socket_data); MockRead mock_read2(ASYNC, OK); - StaticSocketDataProvider socket_data2(&mock_read2, 1, NULL, 0); + DeterministicSocketData socket_data2(&mock_read2, 1, NULL, 0); socket_data2.set_connect_data(MockConnect(ASYNC, ERR_IO_PENDING)); - session_deps.socket_factory->AddSocketDataProvider(&socket_data2); + session_deps.deterministic_socket_factory->AddSocketDataProvider( + &socket_data2); SSLSocketDataProvider ssl_socket_data(ASYNC, OK); ssl_socket_data.SetNextProto(GetParam()); - session_deps.socket_factory->AddSSLSocketDataProvider(&ssl_socket_data); + session_deps.deterministic_socket_factory->AddSSLSocketDataProvider( + &ssl_socket_data); scoped_refptr - session(SpdySessionDependencies::SpdyCreateSession(&session_deps)); + session(SpdySessionDependencies::SpdyCreateSessionDeterministic( + &session_deps)); // Now request a stream. HttpRequestInfo request_info; diff --git a/net/socket/ssl_client_socket_pool_unittest.cc b/net/socket/ssl_client_socket_pool_unittest.cc index 4652de8a868237..d237528a28578b 100644 --- a/net/socket/ssl_client_socket_pool_unittest.cc +++ b/net/socket/ssl_client_socket_pool_unittest.cc @@ -759,7 +759,7 @@ TEST_F(SSLClientSocketPoolTest, IPPooling) { socket_factory_.AddSSLSocketDataProvider(&ssl); CreatePool(true /* tcp pool */, false, false); - scoped_refptr spdy_session = + base::WeakPtr spdy_session = CreateSecureSpdySession(session_, test_hosts[0].key, BoundNetLog()); EXPECT_TRUE( @@ -812,7 +812,7 @@ void SSLClientSocketPoolTest::TestIPPoolingDisabled( socket_factory_.AddSSLSocketDataProvider(ssl); CreatePool(true /* tcp pool */, false, false); - scoped_refptr spdy_session = + base::WeakPtr spdy_session = CreateSecureSpdySession(session_, test_hosts[0].key, BoundNetLog()); EXPECT_TRUE( diff --git a/net/spdy/spdy_http_stream.cc b/net/spdy/spdy_http_stream.cc index 69127d107e7714..8f0f1713e872f1 100644 --- a/net/spdy/spdy_http_stream.cc +++ b/net/spdy/spdy_http_stream.cc @@ -25,9 +25,11 @@ namespace net { -SpdyHttpStream::SpdyHttpStream(SpdySession* spdy_session, bool direct) +SpdyHttpStream::SpdyHttpStream(const base::WeakPtr& spdy_session, + bool direct) : weak_factory_(this), spdy_session_(spdy_session), + is_reused_(spdy_session_->IsReused()), stream_closed_(false), closed_stream_status_(ERR_FAILED), closed_stream_id_(0), @@ -53,7 +55,9 @@ int SpdyHttpStream::InitializeStream(const HttpRequestInfo* request_info, RequestPriority priority, const BoundNetLog& stream_net_log, const CompletionCallback& callback) { - DCHECK(!stream_.get()); + DCHECK(!stream_); + if (!spdy_session_) + return ERR_CONNECTION_CLOSED; request_info_ = request_info; if (request_info_->method == "GET") { @@ -161,7 +165,7 @@ bool SpdyHttpStream::CanFindEndOfResponse() const { } bool SpdyHttpStream::IsConnectionReused() const { - return spdy_session_->IsReused(); + return is_reused_; } void SpdyHttpStream::SetConnectionReused() { @@ -174,19 +178,21 @@ bool SpdyHttpStream::IsConnectionReusable() const { } bool SpdyHttpStream::GetLoadTimingInfo(LoadTimingInfo* load_timing_info) const { + if (stream_closed_) { + if (!closed_stream_has_load_timing_info_) + return false; + *load_timing_info = closed_stream_load_timing_info_; + return true; + } + // If |stream_| has yet to be created, or does not yet have an ID, fail. // The reused flag can only be correctly set once a stream has an ID. Streams // get their IDs once the request has been successfully sent, so this does not // behave that differently from other stream types. - if (!spdy_session_.get() || (!stream_.get() && !stream_closed_)) - return false; - - SpdyStreamId stream_id = - stream_closed_ ? closed_stream_id_ : stream_->stream_id(); - if (stream_id == 0) + if (!stream_ || stream_->stream_id() == 0) return false; - return spdy_session_->GetLoadTimingInfo(stream_id, load_timing_info); + return stream_->GetLoadTimingInfo(load_timing_info); } int SpdyHttpStream::SendRequest(const HttpRequestHeaders& request_headers, @@ -317,7 +323,7 @@ SpdyResponseHeadersStatus SpdyHttpStream::OnResponseHeadersUpdated( response_info_->npn_negotiated_protocol = SSLClientSocket::NextProtoToString(protocol_negotiated); response_info_->request_time = stream_->GetRequestTime(); - switch (spdy_session_->GetProtocolVersion()) { + switch (stream_->GetProtocolVersion()) { case SPDY2: response_info_->connection_info = HttpResponseInfo::CONNECTION_INFO_SPDY2; break; @@ -368,6 +374,8 @@ void SpdyHttpStream::OnClose(int status) { stream_closed_ = true; closed_stream_status_ = status; closed_stream_id_ = stream_->stream_id(); + closed_stream_has_load_timing_info_ = + stream_->GetLoadTimingInfo(&closed_stream_load_timing_info_); } stream_.reset(); bool invoked_callback = false; diff --git a/net/spdy/spdy_http_stream.h b/net/spdy/spdy_http_stream.h index 33c253c55b0114..65d98784181f45 100644 --- a/net/spdy/spdy_http_stream.h +++ b/net/spdy/spdy_http_stream.h @@ -31,7 +31,7 @@ class NET_EXPORT_PRIVATE SpdyHttpStream : public SpdyStream::Delegate, public HttpStream { public: // |spdy_session| must not be NULL. - SpdyHttpStream(SpdySession* spdy_session, bool direct); + SpdyHttpStream(const base::WeakPtr& spdy_session, bool direct); virtual ~SpdyHttpStream(); SpdyStream* stream() { return stream_.get(); } @@ -109,7 +109,8 @@ class NET_EXPORT_PRIVATE SpdyHttpStream : public SpdyStream::Delegate, base::WeakPtrFactory weak_factory_; - const scoped_refptr spdy_session_; + const base::WeakPtr spdy_session_; + bool is_reused_; SpdyStreamRequest stream_request_; base::WeakPtr stream_; @@ -118,6 +119,8 @@ class NET_EXPORT_PRIVATE SpdyHttpStream : public SpdyStream::Delegate, // Set only when |stream_closed_| is true. int closed_stream_status_; SpdyStreamId closed_stream_id_; + bool closed_stream_has_load_timing_info_; + LoadTimingInfo closed_stream_load_timing_info_; // The request to send. const HttpRequestInfo* request_info_; diff --git a/net/spdy/spdy_http_stream_unittest.cc b/net/spdy/spdy_http_stream_unittest.cc index c6703fc767928e..26d87d85e4248c 100644 --- a/net/spdy/spdy_http_stream_unittest.cc +++ b/net/spdy/spdy_http_stream_unittest.cc @@ -121,7 +121,7 @@ class SpdyHttpStreamTest : public testing::Test, scoped_ptr data_; scoped_ptr deterministic_data_; scoped_refptr http_session_; - scoped_refptr session_; + base::WeakPtr session_; private: MockECSignatureCreatorFactory ec_signature_creator_factory_; @@ -144,7 +144,7 @@ TEST_P(SpdyHttpStreamTest, GetUploadProgressBeforeInitialization) { kPrivacyModeDisabled); InitSession(reads, arraysize(reads), NULL, 0, key); - SpdyHttpStream stream(session_.get(), false); + SpdyHttpStream stream(session_, false); UploadProgress progress = stream.GetUploadProgress(); EXPECT_EQ(0u, progress.size()); EXPECT_EQ(0u, progress.position()); @@ -174,8 +174,7 @@ TEST_P(SpdyHttpStreamTest, SendRequest) { HttpResponseInfo response; HttpRequestHeaders headers; BoundNetLog net_log; - scoped_ptr http_stream( - new SpdyHttpStream(session_.get(), true)); + scoped_ptr http_stream(new SpdyHttpStream(session_, true)); // Make sure getting load timing information the stream early does not crash. LoadTimingInfo load_timing_info; EXPECT_FALSE(http_stream->GetLoadTimingInfo(&load_timing_info)); @@ -251,8 +250,7 @@ TEST_P(SpdyHttpStreamTest, LoadTimingTwoRequests) { TestCompletionCallback callback1; HttpResponseInfo response1; HttpRequestHeaders headers1; - scoped_ptr http_stream1( - new SpdyHttpStream(session_.get(), true)); + scoped_ptr http_stream1(new SpdyHttpStream(session_, true)); HttpRequestInfo request2; request2.method = "GET"; @@ -260,8 +258,7 @@ TEST_P(SpdyHttpStreamTest, LoadTimingTwoRequests) { TestCompletionCallback callback2; HttpResponseInfo response2; HttpRequestHeaders headers2; - scoped_ptr http_stream2( - new SpdyHttpStream(session_.get(), true)); + scoped_ptr http_stream2(new SpdyHttpStream(session_, true)); // First write. ASSERT_EQ(OK, @@ -354,7 +351,7 @@ TEST_P(SpdyHttpStreamTest, SendChunkedPost) { HttpResponseInfo response; HttpRequestHeaders headers; BoundNetLog net_log; - SpdyHttpStream http_stream(session_.get(), true); + SpdyHttpStream http_stream(session_, true); ASSERT_EQ( OK, http_stream.InitializeStream(&request, DEFAULT_PRIORITY, @@ -423,8 +420,7 @@ TEST_P(SpdyHttpStreamTest, DelayedSendChunkedPost) { upload_stream.AppendChunk(kUploadData, kUploadDataSize, false); BoundNetLog net_log; - scoped_ptr http_stream( - new SpdyHttpStream(session_.get(), true)); + scoped_ptr http_stream(new SpdyHttpStream(session_, true)); ASSERT_EQ(OK, http_stream->InitializeStream(&request, DEFAULT_PRIORITY, net_log, CompletionCallback())); @@ -512,8 +508,7 @@ TEST_P(SpdyHttpStreamTest, SpdyURLTest) { HttpResponseInfo response; HttpRequestHeaders headers; BoundNetLog net_log; - scoped_ptr http_stream( - new SpdyHttpStream(session_.get(), true)); + scoped_ptr http_stream(new SpdyHttpStream(session_, true)); ASSERT_EQ(OK, http_stream->InitializeStream( &request, DEFAULT_PRIORITY, net_log, CompletionCallback())); @@ -656,8 +651,7 @@ void SpdyHttpStreamTest::TestSendCredentials( HttpResponseInfo response; HttpRequestHeaders headers; BoundNetLog net_log; - scoped_ptr http_stream( - new SpdyHttpStream(session_.get(), true)); + scoped_ptr http_stream(new SpdyHttpStream(session_, true)); ASSERT_EQ( OK, http_stream->InitializeStream(&request, DEFAULT_PRIORITY, @@ -676,8 +670,7 @@ void SpdyHttpStreamTest::TestSendCredentials( callback.WaitForResult(); // Start up second request for resource on a new origin. - scoped_ptr http_stream2( - new SpdyHttpStream(session_.get(), true)); + scoped_ptr http_stream2(new SpdyHttpStream(session_, true)); request.url = GURL(kUrl2); ASSERT_EQ( OK, @@ -739,8 +732,7 @@ TEST_P(SpdyHttpStreamTest, DelayedSendChunkedPostWithWindowUpdate) { upload_stream.AppendChunk(kUploadData, kUploadDataSize, true); BoundNetLog net_log; - scoped_ptr http_stream( - new SpdyHttpStream(session_.get(), true)); + scoped_ptr http_stream(new SpdyHttpStream(session_, true)); ASSERT_EQ(OK, http_stream->InitializeStream(&request, DEFAULT_PRIORITY, net_log, CompletionCallback())); @@ -866,8 +858,7 @@ TEST_P(SpdyHttpStreamTest, DontSendCredentialsForHttpUrlsEC) { HttpResponseInfo response; HttpRequestHeaders headers; BoundNetLog net_log; - scoped_ptr http_stream( - new SpdyHttpStream(session_.get(), true)); + scoped_ptr http_stream(new SpdyHttpStream(session_, true)); ASSERT_EQ( OK, http_stream->InitializeStream(&request, DEFAULT_PRIORITY, @@ -882,8 +873,7 @@ TEST_P(SpdyHttpStreamTest, DontSendCredentialsForHttpUrlsEC) { EXPECT_EQ(OK, callback.WaitForResult()); // Start up second request for resource on a new origin. - scoped_ptr http_stream2( - new SpdyHttpStream(session_.get(), true)); + scoped_ptr http_stream2(new SpdyHttpStream(session_, true)); request.url = GURL(kUrl2); ASSERT_EQ( OK, diff --git a/net/spdy/spdy_network_transaction_unittest.cc b/net/spdy/spdy_network_transaction_unittest.cc index 2a67cb31dac969..ba6a60706edd3a 100644 --- a/net/spdy/spdy_network_transaction_unittest.cc +++ b/net/spdy/spdy_network_transaction_unittest.cc @@ -564,7 +564,7 @@ class SpdyNetworkTransactionTest kPrivacyModeDisabled); BoundNetLog log; const scoped_refptr& session = helper.session(); - scoped_refptr spdy_session = + base::WeakPtr spdy_session = session->spdy_session_pool()->FindAvailableSession(key, log); ASSERT_TRUE(spdy_session != NULL); EXPECT_EQ(0u, spdy_session->num_active_streams()); diff --git a/net/spdy/spdy_proxy_client_socket_unittest.cc b/net/spdy/spdy_proxy_client_socket_unittest.cc index 95ca4b59c21603..b2bdb3e8f510a4 100644 --- a/net/spdy/spdy_proxy_client_socket_unittest.cc +++ b/net/spdy/spdy_proxy_client_socket_unittest.cc @@ -108,6 +108,10 @@ class SpdyProxyClientSocketTest data_->Run(); } + void CloseSpdySession(net::Error error, const std::string& description) { + spdy_session_->CloseSessionOnError(error, description); + } + SpdyTestUtil spdy_util_; scoped_ptr sock_; TestCompletionCallback read_callback_; @@ -120,7 +124,7 @@ class SpdyProxyClientSocketTest scoped_refptr read_buf_; SpdySessionDependencies session_deps_; MockConnect connect_data_; - scoped_refptr spdy_session_; + base::WeakPtr spdy_session_; BufferedSpdyFramer framer_; std::string user_agent_; @@ -144,7 +148,6 @@ SpdyProxyClientSocketTest::SpdyProxyClientSocketTest() read_buf_(NULL), session_deps_(GetParam()), connect_data_(SYNCHRONOUS, OK), - spdy_session_(NULL), framer_(spdy_util_.spdy_version(), false), user_agent_(kUserAgent), url_(kRequestUrl), @@ -1195,7 +1198,7 @@ TEST_P(SpdyProxyClientSocketTest, WritePendingOnClose) { EXPECT_EQ(ERR_IO_PENDING, sock_->Write(buf.get(), buf->size(), write_callback_.callback())); - Run(1); + CloseSpdySession(ERR_ABORTED, std::string()); EXPECT_EQ(ERR_CONNECTION_CLOSED, write_callback_.WaitForResult()); } @@ -1267,7 +1270,7 @@ TEST_P(SpdyProxyClientSocketTest, RstWithReadAndWritePending) { scoped_ptr conn(ConstructConnectRequestFrame()); MockWrite writes[] = { CreateMockWrite(*conn, 0, SYNCHRONOUS), - MockWrite(ASYNC, ERR_ABORTED, 2), + MockWrite(ASYNC, ERR_ABORTED, 3), }; scoped_ptr resp(ConstructConnectReplyFrame()); @@ -1275,7 +1278,8 @@ TEST_P(SpdyProxyClientSocketTest, RstWithReadAndWritePending) { spdy_util_.ConstructSpdyRstStream(1, RST_STREAM_CANCEL)); MockRead reads[] = { CreateMockRead(*resp, 1, ASYNC), - CreateMockRead(*rst, 3, ASYNC), + CreateMockRead(*rst, 2, ASYNC), + MockRead(ASYNC, 0, 4) // EOF }; Initialize(reads, arraysize(reads), writes, arraysize(writes)); @@ -1390,7 +1394,7 @@ TEST_P(SpdyProxyClientSocketTest, RstWithReadAndWritePendingDelete) { scoped_ptr conn(ConstructConnectRequestFrame()); MockWrite writes[] = { CreateMockWrite(*conn, 0, SYNCHRONOUS), - MockWrite(ASYNC, ERR_ABORTED, 2), + MockWrite(ASYNC, ERR_ABORTED, 3), }; scoped_ptr resp(ConstructConnectReplyFrame()); @@ -1398,7 +1402,8 @@ TEST_P(SpdyProxyClientSocketTest, RstWithReadAndWritePendingDelete) { spdy_util_.ConstructSpdyRstStream(1, RST_STREAM_CANCEL)); MockRead reads[] = { CreateMockRead(*resp, 1, ASYNC), - CreateMockRead(*rst, 3, ASYNC), + CreateMockRead(*rst, 2, ASYNC), + MockRead(ASYNC, 0, 4), // EOF }; Initialize(reads, arraysize(reads), writes, arraysize(writes)); @@ -1419,7 +1424,7 @@ TEST_P(SpdyProxyClientSocketTest, RstWithReadAndWritePendingDelete) { sock_->Write( write_buf.get(), write_buf->size(), write_callback_.callback())); - Run(2); + Run(1); EXPECT_FALSE(sock_.get()); EXPECT_TRUE(read_callback.have_result()); diff --git a/net/spdy/spdy_session.cc b/net/spdy/spdy_session.cc index 3a56a90186e281..e80a938343d304 100644 --- a/net/spdy/spdy_session.cc +++ b/net/spdy/spdy_session.cc @@ -240,7 +240,7 @@ SpdyStreamRequest::~SpdyStreamRequest() { int SpdyStreamRequest::StartRequest( SpdyStreamType type, - const scoped_refptr& session, + const base::WeakPtr& session, const GURL& url, RequestPriority priority, const BoundNetLog& net_log, @@ -304,7 +304,7 @@ void SpdyStreamRequest::OnRequestCompleteFailure(int rv) { void SpdyStreamRequest::Reset() { type_ = SPDY_BIDIRECTIONAL_STREAM; - session_ = NULL; + session_.reset(); stream_.reset(); url_ = GURL(); priority_ = MINIMUM_PRIORITY; @@ -632,7 +632,8 @@ int SpdySession::CreateStream(const SpdyStreamRequest& request, } scoped_ptr new_stream( - new SpdyStream(request.type(), this, request.url(), request.priority(), + new SpdyStream(request.type(), GetWeakPtr(), request.url(), + request.priority(), stream_initial_send_window_size_, stream_initial_recv_window_size_, request.net_log())); @@ -723,6 +724,10 @@ int SpdySession::GetProtocolVersion() const { return buffered_spdy_framer_->protocol_version(); } +base::WeakPtr SpdySession::GetWeakPtr() { + return weak_factory_.GetWeakPtr(); +} + bool SpdySession::CloseOneIdleConnection() { CHECK(!in_io_loop_); DCHECK(pool_); @@ -1325,7 +1330,7 @@ int SpdySession::DoWrite() { int SpdySession::DoWriteComplete(int result) { CHECK(in_io_loop_); DCHECK_NE(availability_state_, STATE_CLOSED); - + DCHECK_NE(result, ERR_IO_PENDING); DCHECK_GT(in_flight_write_->GetRemainingSize(), 0u); last_activity_time_ = time_func_(); @@ -1466,7 +1471,7 @@ SpdySession::CloseSessionResult SpdySession::DoCloseSession( // |pool_| will be NULL when |InitializeWithSocket()| is in the // call stack. if (pool_ && availability_state_ != STATE_GOING_AWAY) - pool_->MakeSessionUnavailable(make_scoped_refptr(this)); + pool_->MakeSessionUnavailable(GetWeakPtr()); availability_state_ = STATE_CLOSED; error_on_close_ = err; @@ -1489,7 +1494,7 @@ void SpdySession::RemoveFromPool() { SpdySessionPool* pool = pool_; pool_ = NULL; - pool->RemoveUnavailableSession(make_scoped_refptr(this)); + pool->RemoveUnavailableSession(GetWeakPtr()); } void SpdySession::LogAbandonedStream(SpdyStream* stream, Error status) { @@ -2028,7 +2033,7 @@ void SpdySession::OnSynStream(SpdyStreamId stream_id, } scoped_ptr stream( - new SpdyStream(SPDY_PUSH_STREAM, this, gurl, + new SpdyStream(SPDY_PUSH_STREAM, GetWeakPtr(), gurl, request_priority, stream_initial_send_window_size_, stream_initial_recv_window_size_, @@ -2225,7 +2230,7 @@ void SpdySession::OnGoAway(SpdyStreamId last_accepted_stream_id, // |pool_| will be NULL when |InitializeWithSocket()| is in the // call stack. if (pool_) - pool_->MakeSessionUnavailable(make_scoped_refptr(this)); + pool_->MakeSessionUnavailable(GetWeakPtr()); } StartGoingAway(last_accepted_stream_id, ERR_ABORTED); // This is to handle the case when we already don't have any active diff --git a/net/spdy/spdy_session.h b/net/spdy/spdy_session.h index 5f6f4e0e6d0888..dd2e0dafadd971 100644 --- a/net/spdy/spdy_session.h +++ b/net/spdy/spdy_session.h @@ -133,7 +133,7 @@ class NET_EXPORT_PRIVATE SpdyStreamRequest { // ReleaseStream() being called first. Otherwise, in case of an // immediate error, this may be called again. int StartRequest(SpdyStreamType type, - const scoped_refptr& session, + const base::WeakPtr& session, const GURL& url, RequestPriority priority, const BoundNetLog& net_log, @@ -170,7 +170,7 @@ class NET_EXPORT_PRIVATE SpdyStreamRequest { void Reset(); SpdyStreamType type_; - scoped_refptr session_; + base::WeakPtr session_; base::WeakPtr stream_; GURL url_; RequestPriority priority_; @@ -344,6 +344,9 @@ class NET_EXPORT SpdySession : public base::RefCounted, // Whether the stream is closed, i.e. it has stopped processing data // and is about to be destroyed. + // + // TODO(akalin): This is only used in tests. Remove this function + // and have tests test the WeakPtr instead. bool IsClosed() const { return availability_state_ == STATE_CLOSED; } // Closes this session. This will close all active streams and mark @@ -463,6 +466,9 @@ class NET_EXPORT SpdySession : public base::RefCounted, return buffered_spdy_framer_->GetDataFrameMaximumPayload(); } + // Must be used only by |pool_|. + base::WeakPtr GetWeakPtr(); + // LayeredPool implementation: virtual bool CloseOneIdleConnection() OVERRIDE; diff --git a/net/spdy/spdy_session_pool.cc b/net/spdy/spdy_session_pool.cc index 4b786e3901f65b..6825cfd033b55e 100644 --- a/net/spdy/spdy_session_pool.cc +++ b/net/spdy/spdy_session_pool.cc @@ -80,7 +80,7 @@ net::Error SpdySessionPool::CreateAvailableSessionFromSocket( scoped_ptr connection, const BoundNetLog& net_log, int certificate_error_code, - scoped_refptr* available_session, + base::WeakPtr* available_session, bool is_secure) { UMA_HISTOGRAM_ENUMERATION( "Net.SpdySessionGet", IMPORTED_FROM_SOCKET, SPDY_SESSION_GET_MAX); @@ -107,12 +107,12 @@ net::Error SpdySessionPool::CreateAvailableSessionFromSocket( if (error != OK) { new_session = NULL; - *available_session = NULL; + available_session->reset(); return error; } sessions_.insert(new_session); - available_session->swap(new_session); + *available_session = new_session->GetWeakPtr(); MapKeyToAvailableSession(key, *available_session); net_log.AddEvent( @@ -133,7 +133,7 @@ net::Error SpdySessionPool::CreateAvailableSessionFromSocket( return error; } -scoped_refptr SpdySessionPool::FindAvailableSession( +base::WeakPtr SpdySessionPool::FindAvailableSession( const SpdySessionKey& key, const BoundNetLog& net_log) { AvailableSessionMap::iterator it = LookupAvailableSessionByKey(key); @@ -147,7 +147,7 @@ scoped_refptr SpdySessionPool::FindAvailableSession( } if (!enable_ip_pooling_) - return scoped_refptr(); + return base::WeakPtr(); // Look up the key's from the resolver's cache. net::HostResolver::RequestInfo resolve_info(key.host_port_pair()); @@ -155,7 +155,7 @@ scoped_refptr SpdySessionPool::FindAvailableSession( int rv = resolver_->ResolveFromCache(resolve_info, &addresses, net_log); DCHECK_NE(rv, ERR_IO_PENDING); if (rv != OK) - return scoped_refptr(); + return base::WeakPtr(); // Check if we have a session through a domain alias. for (AddressList::const_iterator address_it = addresses.begin(); @@ -181,9 +181,9 @@ scoped_refptr SpdySessionPool::FindAvailableSession( continue; } - const scoped_refptr& available_session = + const base::WeakPtr& available_session = available_session_it->second; - DCHECK(ContainsKey(sessions_, available_session)); + DCHECK(ContainsKey(sessions_, available_session.get())); // If the session is a secure one, we need to verify that the // server is authenticated to serve traffic for |host_port_proxy_pair| too. if (!available_session->VerifyDomainAuthentication( @@ -205,11 +205,11 @@ scoped_refptr SpdySessionPool::FindAvailableSession( return available_session; } - return scoped_refptr(); + return base::WeakPtr(); } void SpdySessionPool::MakeSessionUnavailable( - const scoped_refptr& available_session) { + const base::WeakPtr& available_session) { UnmapKey(available_session->spdy_session_key()); RemoveAliases(available_session->spdy_session_key()); const std::set& aliases = available_session->pooled_aliases(); @@ -222,14 +222,14 @@ void SpdySessionPool::MakeSessionUnavailable( } void SpdySessionPool::RemoveUnavailableSession( - const scoped_refptr& unavailable_session) { + const base::WeakPtr& unavailable_session) { DCHECK(!IsSessionAvailable(unavailable_session)); unavailable_session->net_log().AddEvent( NetLog::TYPE_SPDY_SESSION_POOL_REMOVE_SESSION, unavailable_session->net_log().source().ToEventParametersCallback()); - SessionSet::iterator it = sessions_.find(unavailable_session); + SessionSet::iterator it = sessions_.find(unavailable_session.get()); CHECK(it != sessions_.end()); sessions_.erase(it); } @@ -293,10 +293,10 @@ void SpdySessionPool::OnCertTrustChanged(const X509Certificate* cert) { } bool SpdySessionPool::IsSessionAvailable( - const scoped_refptr& session) const { + const base::WeakPtr& session) const { for (AvailableSessionMap::const_iterator it = available_sessions_.begin(); it != available_sessions_.end(); ++it) { - if (it->second == session) + if (it->second.get() == session.get()) return true; } return false; @@ -319,8 +319,8 @@ const SpdySessionKey& SpdySessionPool::NormalizeListKey( void SpdySessionPool::MapKeyToAvailableSession( const SpdySessionKey& key, - const scoped_refptr& session) { - DCHECK(ContainsKey(sessions_, session)); + const base::WeakPtr& session) { + DCHECK(ContainsKey(sessions_, scoped_refptr(session.get()))); const SpdySessionKey& normalized_key = NormalizeListKey(key); std::pair result = available_sessions_.insert(std::make_pair(normalized_key, session)); @@ -368,7 +368,7 @@ void SpdySessionPool::CloseCurrentSessionsHelper( continue; (*it)->CloseSessionOnError(error, description); - DCHECK(!IsSessionAvailable(*it)); + DCHECK(!IsSessionAvailable((*it)->GetWeakPtr())); DCHECK(!ContainsKey(sessions_, *it)); } } diff --git a/net/spdy/spdy_session_pool.h b/net/spdy/spdy_session_pool.h index a7562344f03c8e..d2301a0a745a23 100644 --- a/net/spdy/spdy_session_pool.h +++ b/net/spdy/spdy_session_pool.h @@ -84,23 +84,23 @@ class NET_EXPORT SpdySessionPool scoped_ptr connection, const BoundNetLog& net_log, int certificate_error_code, - scoped_refptr* available_session, + base::WeakPtr* available_session, bool is_secure); // Find an available session for the given key, or NULL if there isn't one. - scoped_refptr FindAvailableSession(const SpdySessionKey& key, + base::WeakPtr FindAvailableSession(const SpdySessionKey& key, const BoundNetLog& net_log); // Remove all mappings and aliases for the given session, which must // still be available. Except for in tests, this must be called by // the given session itself. void MakeSessionUnavailable( - const scoped_refptr& available_session); + const base::WeakPtr& available_session); // Removes an unavailable session from the pool. Except for in // tests, this must be called by the given session itself. void RemoveUnavailableSession( - const scoped_refptr& unavailable_session); + const base::WeakPtr& unavailable_session); // Close only the currently existing SpdySessions with |error|. // Let any new ones created while this method is running continue to @@ -144,12 +144,12 @@ class NET_EXPORT SpdySessionPool friend class SpdySessionPoolPeer; // For testing. typedef std::set > SessionSet; - typedef std::map > + typedef std::map > AvailableSessionMap; typedef std::map AliasMap; // Returns true iff |session| is in |available_sessions_|. - bool IsSessionAvailable(const scoped_refptr& session) const; + bool IsSessionAvailable(const base::WeakPtr& session) const; // Returns a normalized version of the given key suitable for lookup // into |available_sessions_|. @@ -158,7 +158,7 @@ class NET_EXPORT SpdySessionPool // Map the given key to the given session. There must not already be // a mapping for |key|. void MapKeyToAvailableSession(const SpdySessionKey& key, - const scoped_refptr& session); + const base::WeakPtr& session); // Returns an iterator into |available_sessions_| for the given key, // which may be equal to |available_sessions_.end()|. diff --git a/net/spdy/spdy_session_pool_unittest.cc b/net/spdy/spdy_session_pool_unittest.cc index 58fedcbb911088..69ba11b584d382 100644 --- a/net/spdy/spdy_session_pool_unittest.cc +++ b/net/spdy/spdy_session_pool_unittest.cc @@ -111,7 +111,7 @@ TEST_P(SpdySessionPoolTest, CloseCurrentSessions) { CreateNetworkSession(); // Setup the first session to the first host. - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, test_key, BoundNetLog()); // Flush the SpdySession::OnReadComplete() task. @@ -156,7 +156,7 @@ TEST_P(SpdySessionPoolTest, CloseCurrentIdleSessions) { HostPortPair test_host_port_pair1(kTestHost1, 80); SpdySessionKey key1(test_host_port_pair1, ProxyServer::Direct(), kPrivacyModeDisabled); - scoped_refptr session1 = + base::WeakPtr session1 = CreateInsecureSpdySession(http_session_, key1, BoundNetLog()); GURL url1(kTestHost1); base::WeakPtr spdy_stream1 = @@ -170,7 +170,7 @@ TEST_P(SpdySessionPoolTest, CloseCurrentIdleSessions) { HostPortPair test_host_port_pair2(kTestHost2, 80); SpdySessionKey key2(test_host_port_pair2, ProxyServer::Direct(), kPrivacyModeDisabled); - scoped_refptr session2 = + base::WeakPtr session2 = CreateInsecureSpdySession(http_session_, key2, BoundNetLog()); GURL url2(kTestHost2); base::WeakPtr spdy_stream2 = @@ -184,7 +184,7 @@ TEST_P(SpdySessionPoolTest, CloseCurrentIdleSessions) { HostPortPair test_host_port_pair3(kTestHost3, 80); SpdySessionKey key3(test_host_port_pair3, ProxyServer::Direct(), kPrivacyModeDisabled); - scoped_refptr session3 = + base::WeakPtr session3 = CreateInsecureSpdySession(http_session_, key3, BoundNetLog()); GURL url3(kTestHost3); base::WeakPtr spdy_stream3 = @@ -224,12 +224,10 @@ TEST_P(SpdySessionPoolTest, CloseCurrentIdleSessions) { // Should close session 1 and 3, 2 should be left open spdy_session_pool_->CloseCurrentIdleSessions(); - EXPECT_FALSE(session1->is_active()); - EXPECT_TRUE(session1->IsClosed()); + EXPECT_TRUE(session1 == NULL); EXPECT_TRUE(session2->is_active()); EXPECT_FALSE(session2->IsClosed()); - EXPECT_FALSE(session3->is_active()); - EXPECT_TRUE(session3->IsClosed()); + EXPECT_TRUE(session3 == NULL); // Should not do anything spdy_session_pool_->CloseCurrentIdleSessions(); @@ -244,8 +242,7 @@ TEST_P(SpdySessionPoolTest, CloseCurrentIdleSessions) { // This should close session 2 spdy_session_pool_->CloseCurrentIdleSessions(); - EXPECT_FALSE(session2->is_active()); - EXPECT_TRUE(session2->IsClosed()); + EXPECT_TRUE(session2 == NULL); } // Set up a SpdyStream to create a new session when it is closed. @@ -277,7 +274,7 @@ TEST_P(SpdySessionPoolTest, CloseAllSessions) { CreateNetworkSession(); // Setup the first session to the first host. - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, test_key, BoundNetLog()); // Flush the SpdySession::OnReadComplete() task. @@ -364,7 +361,7 @@ void SpdySessionPoolTest::RunIPPoolingTest( CreateNetworkSession(); // Setup the first session to the first host. - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession( http_session_, test_hosts[0].key, BoundNetLog()); @@ -388,7 +385,7 @@ void SpdySessionPoolTest::RunIPPoolingTest( // Create a new session to host 2. session_deps_.socket_factory->AddSocketDataProvider(&data); - scoped_refptr session2 = + base::WeakPtr session2 = CreateInsecureSpdySession( http_session_, test_hosts[2].key, BoundNetLog()); @@ -399,7 +396,7 @@ void SpdySessionPoolTest::RunIPPoolingTest( // Grab the session to host 1 and verify that it is the same session // we got with host 0, and that is a different from host 2's session. - scoped_refptr session1 = + base::WeakPtr session1 = spdy_session_pool_->FindAvailableSession( test_hosts[1].key, BoundNetLog()); EXPECT_EQ(session.get(), session1.get()); @@ -419,9 +416,9 @@ void SpdySessionPoolTest::RunIPPoolingTest( switch (close_sessions_type) { case SPDY_POOL_CLOSE_SESSIONS_MANUALLY: session->CloseSessionOnError(ERR_ABORTED, std::string()); - session = NULL; + EXPECT_TRUE(session == NULL); session2->CloseSessionOnError(ERR_ABORTED, std::string()); - session2 = NULL; + EXPECT_TRUE(session2 == NULL); break; case SPDY_POOL_CLOSE_CURRENT_SESSIONS: spdy_session_pool_->CloseCurrentSessions(ERR_ABORTED); @@ -459,10 +456,8 @@ void SpdySessionPoolTest::RunIPPoolingTest( spdy_session_pool_->CloseCurrentIdleSessions(); // Verify spdy_session and spdy_session1 are closed. - EXPECT_FALSE(session->is_active()); - EXPECT_TRUE(session->IsClosed()); - EXPECT_FALSE(session1->is_active()); - EXPECT_TRUE(session1->IsClosed()); + EXPECT_TRUE(session == NULL); + EXPECT_TRUE(session1 == NULL); EXPECT_TRUE(session2->is_active()); EXPECT_FALSE(session2->IsClosed()); @@ -471,7 +466,7 @@ void SpdySessionPoolTest::RunIPPoolingTest( EXPECT_EQ(NULL, spdy_stream1.get()); EXPECT_EQ(NULL, spdy_stream2.get()); session2->CloseSessionOnError(ERR_ABORTED, std::string()); - session2 = NULL; + EXPECT_TRUE(session2 == NULL); break; } diff --git a/net/spdy/spdy_session_unittest.cc b/net/spdy/spdy_session_unittest.cc index 2de35ffdd183ae..40028e74f0d21f 100644 --- a/net/spdy/spdy_session_unittest.cc +++ b/net/spdy/spdy_session_unittest.cc @@ -211,7 +211,7 @@ TEST_P(SpdySessionTest, GoAwayWithNoActiveStreams) { CreateDeterministicNetworkSession(); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, BoundNetLog()); EXPECT_EQ(spdy_util_.spdy_version(), session->GetProtocolVersion()); @@ -223,12 +223,7 @@ TEST_P(SpdySessionTest, GoAwayWithNoActiveStreams) { EXPECT_FALSE(HasSpdySession(spdy_session_pool_, key_)); - // TODO(akalin): Once |session| is a WeakPtr, then simply assert - // that it's NULL here instead. - EXPECT_TRUE(session->IsClosed()); - - // Delete the session. - session = NULL; + EXPECT_TRUE(session == NULL); } // A session receiving a GOAWAY frame immediately with no active @@ -287,7 +282,7 @@ TEST_P(SpdySessionTest, GoAwayWithActiveStreams) { CreateDeterministicNetworkSession(); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, BoundNetLog()); EXPECT_EQ(spdy_util_.spdy_version(), session->GetProtocolVersion()); @@ -336,12 +331,7 @@ TEST_P(SpdySessionTest, GoAwayWithActiveStreams) { spdy_stream1->Close(); EXPECT_EQ(NULL, spdy_stream1.get()); - // TODO(akalin): Once |session| is a WeakPtr, then simply assert - // that it's NULL here instead. - EXPECT_TRUE(session->IsClosed()); - - // Delete the session. - session = NULL; + EXPECT_TRUE(session == NULL); } // Have a session receive two GOAWAY frames, with the last one causing @@ -376,7 +366,7 @@ TEST_P(SpdySessionTest, GoAwayTwice) { CreateDeterministicNetworkSession(); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, BoundNetLog()); EXPECT_EQ(spdy_util_.spdy_version(), session->GetProtocolVersion()); @@ -425,12 +415,7 @@ TEST_P(SpdySessionTest, GoAwayTwice) { // session. data.RunFor(1); - // TODO(akalin): Once |session| is a WeakPtr, then simply assert - // that it's NULL here instead. - EXPECT_TRUE(session->IsClosed()); - - // Delete the session. - session = NULL; + EXPECT_TRUE(session == NULL); } // Have a session with active streams receive a GOAWAY frame and then @@ -463,7 +448,7 @@ TEST_P(SpdySessionTest, GoAwayWithActiveStreamsThenClose) { CreateDeterministicNetworkSession(); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, BoundNetLog()); EXPECT_EQ(spdy_util_.spdy_version(), session->GetProtocolVersion()); @@ -511,10 +496,7 @@ TEST_P(SpdySessionTest, GoAwayWithActiveStreamsThenClose) { session->CloseSessionOnError(ERR_ABORTED, "Aborting session"); EXPECT_EQ(NULL, spdy_stream1.get()); - EXPECT_TRUE(session->IsClosed()); - - // Delete the session. - session = NULL; + EXPECT_TRUE(session == NULL); } // Try to create a stream after receiving a GOAWAY frame. It should @@ -544,7 +526,7 @@ TEST_P(SpdySessionTest, CreateStreamAfterGoAway) { CreateDeterministicNetworkSession(); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, BoundNetLog()); EXPECT_EQ(spdy_util_.spdy_version(), session->GetProtocolVersion()); @@ -582,8 +564,7 @@ TEST_P(SpdySessionTest, CreateStreamAfterGoAway) { // Read and process EOF. data.RunFor(1); - // Delete the first session. - session = NULL; + EXPECT_TRUE(session == NULL); } // Receiving a SYN_STREAM frame after a GOAWAY frame should result in @@ -619,7 +600,7 @@ TEST_P(SpdySessionTest, SynStreamAfterGoAway) { CreateDeterministicNetworkSession(); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, BoundNetLog()); EXPECT_EQ(spdy_util_.spdy_version(), session->GetProtocolVersion()); @@ -652,8 +633,7 @@ TEST_P(SpdySessionTest, SynStreamAfterGoAway) { // and EOF. data.RunFor(3); - // Delete the first session. - session = NULL; + EXPECT_TRUE(session == NULL); } TEST_P(SpdySessionTest, ClientPing) { @@ -663,24 +643,24 @@ TEST_P(SpdySessionTest, ClientPing) { MockConnect connect_data(SYNCHRONOUS, OK); scoped_ptr read_ping(spdy_util_.ConstructSpdyPing(1)); MockRead reads[] = { - CreateMockRead(*read_ping), - MockRead(SYNCHRONOUS, 0, 0) // EOF + CreateMockRead(*read_ping, 1), + MockRead(ASYNC, 0, 0, 2) // EOF }; scoped_ptr write_ping(spdy_util_.ConstructSpdyPing(1)); MockWrite writes[] = { - CreateMockWrite(*write_ping), + CreateMockWrite(*write_ping, 0), }; - StaticSocketDataProvider data( + DeterministicSocketData data( reads, arraysize(reads), writes, arraysize(writes)); data.set_connect_data(connect_data); - session_deps_.socket_factory->AddSocketDataProvider(&data); + session_deps_.deterministic_socket_factory->AddSocketDataProvider(&data); SSLSocketDataProvider ssl(SYNCHRONOUS, OK); - session_deps_.socket_factory->AddSSLSocketDataProvider(&ssl); + session_deps_.deterministic_socket_factory->AddSSLSocketDataProvider(&ssl); - CreateNetworkSession(); + CreateDeterministicNetworkSession(); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, BoundNetLog()); base::WeakPtr spdy_stream1 = @@ -698,7 +678,7 @@ TEST_P(SpdySessionTest, ClientPing) { session->SendPrefacePingIfNoneInFlight(); - EXPECT_EQ(ERR_CONNECTION_CLOSED, delegate.WaitForClose()); + data.RunFor(2); session->CheckPingStatus(before_ping_time); @@ -707,10 +687,12 @@ TEST_P(SpdySessionTest, ClientPing) { EXPECT_FALSE(session->check_ping_status_pending()); EXPECT_GE(session->last_activity_time(), before_ping_time); - EXPECT_FALSE(HasSpdySession(spdy_session_pool_, key_)); + data.RunFor(1); + + EXPECT_EQ(ERR_CONNECTION_CLOSED, delegate.WaitForClose()); - // Delete the first session. - session = NULL; + EXPECT_FALSE(HasSpdySession(spdy_session_pool_, key_)); + EXPECT_TRUE(session == NULL); } TEST_P(SpdySessionTest, ServerPing) { @@ -736,7 +718,7 @@ TEST_P(SpdySessionTest, ServerPing) { CreateNetworkSession(); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, BoundNetLog()); base::WeakPtr spdy_stream1 = @@ -751,8 +733,7 @@ TEST_P(SpdySessionTest, ServerPing) { EXPECT_FALSE(HasSpdySession(spdy_session_pool_, key_)); - // Delete the session. - session = NULL; + EXPECT_TRUE(session == NULL); EXPECT_EQ(NULL, spdy_stream1.get()); } @@ -789,7 +770,7 @@ TEST_P(SpdySessionTest, PingAndWriteLoop) { CreateDeterministicNetworkSession(); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, BoundNetLog()); GURL url("http://www.google.com"); @@ -809,8 +790,6 @@ TEST_P(SpdySessionTest, PingAndWriteLoop) { data.RunFor(2); session->CloseSessionOnError(ERR_ABORTED, "Aborting"); - - data.RunFor(1); } TEST_P(SpdySessionTest, DeleteExpiredPushStreams) { @@ -822,7 +801,7 @@ TEST_P(SpdySessionTest, DeleteExpiredPushStreams) { CreateNetworkSession(); - scoped_refptr session = + base::WeakPtr session = CreateFakeSpdySession(spdy_session_pool_, key_); session->buffered_spdy_framer_.reset( @@ -833,7 +812,7 @@ TEST_P(SpdySessionTest, DeleteExpiredPushStreams) { spdy_util_.ConstructGetHeaderBlock("http://www.google.com/")); scoped_ptr stream(new SpdyStream(SPDY_REQUEST_RESPONSE_STREAM, - session.get(), + session, GURL(), DEFAULT_PRIORITY, kSpdyStreamInitialWindowSize, @@ -873,35 +852,26 @@ TEST_P(SpdySessionTest, DeleteExpiredPushStreams) { iter = session->unclaimed_pushed_streams_.find( GURL("http://www.google.com/b.dat")); EXPECT_TRUE(session->unclaimed_pushed_streams_.end() != iter); - - // Delete the session. - session = NULL; } TEST_P(SpdySessionTest, FailedPing) { session_deps_.host_resolver->set_synchronous_mode(true); MockConnect connect_data(SYNCHRONOUS, OK); - scoped_ptr read_ping(spdy_util_.ConstructSpdyPing(1)); MockRead reads[] = { - CreateMockRead(*read_ping), - MockRead(SYNCHRONOUS, 0, 0) // EOF + MockRead(ASYNC, 0, 0, 0) // EOF }; scoped_ptr write_ping(spdy_util_.ConstructSpdyPing(1)); - MockWrite writes[] = { - CreateMockWrite(*write_ping), - }; - StaticSocketDataProvider data( - reads, arraysize(reads), writes, arraysize(writes)); + DeterministicSocketData data(reads, arraysize(reads), NULL, 0); data.set_connect_data(connect_data); - session_deps_.socket_factory->AddSocketDataProvider(&data); + session_deps_.deterministic_socket_factory->AddSocketDataProvider(&data); SSLSocketDataProvider ssl(SYNCHRONOUS, OK); - session_deps_.socket_factory->AddSSLSocketDataProvider(&ssl); + session_deps_.deterministic_socket_factory->AddSSLSocketDataProvider(&ssl); - CreateNetworkSession(); + CreateDeterministicNetworkSession(); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, BoundNetLog()); base::WeakPtr spdy_stream1 = @@ -931,13 +901,10 @@ TEST_P(SpdySessionTest, FailedPing) { session->last_activity_time_ = now - base::TimeDelta::FromSeconds(1); session->CheckPingStatus(now); - EXPECT_TRUE(session->IsClosed()); - EXPECT_EQ(0u, session->num_active_streams()); - EXPECT_EQ(0u, session->num_unclaimed_pushed_streams()); + EXPECT_TRUE(session == NULL); EXPECT_FALSE(HasSpdySession(spdy_session_pool_, key_)); - // Delete the first session. - session = NULL; + data.RunFor(1); EXPECT_EQ(NULL, spdy_stream1.get()); } @@ -949,39 +916,52 @@ TEST_P(SpdySessionTest, FailedPing) { TEST_P(SpdySessionTest, OnSettings) { session_deps_.host_resolver->set_synchronous_mode(true); + const SpdySettingsIds kSpdySettingsIds = SETTINGS_MAX_CONCURRENT_STREAMS; + + SettingsMap initial_settings; + const uint32 initial_max_concurrent_streams = 1; + initial_settings[kSpdySettingsIds] = + SettingsFlagsAndValue(SETTINGS_FLAG_PERSISTED, + initial_max_concurrent_streams); + scoped_ptr initial_settings_frame( + spdy_util_.ConstructSpdySettings(initial_settings)); + MockWrite writes[] = { + CreateMockWrite(*initial_settings_frame, 0), + }; + SettingsMap new_settings; - const SpdySettingsIds kSpdySettingsIds1 = SETTINGS_MAX_CONCURRENT_STREAMS; const uint32 max_concurrent_streams = 2; - new_settings[kSpdySettingsIds1] = + new_settings[kSpdySettingsIds] = SettingsFlagsAndValue(SETTINGS_FLAG_NONE, max_concurrent_streams); // Set up the socket so we read a SETTINGS frame that raises max concurrent // streams to 2. - MockConnect connect_data(SYNCHRONOUS, OK); scoped_ptr settings_frame( spdy_util_.ConstructSpdySettings(new_settings)); MockRead reads[] = { - CreateMockRead(*settings_frame), - MockRead(SYNCHRONOUS, 0, 0) // EOF + CreateMockRead(*settings_frame, 1), + MockRead(ASYNC, 0, 2), }; - StaticSocketDataProvider data(reads, arraysize(reads), NULL, 0); + DeterministicSocketData data( + reads, arraysize(reads), writes, arraysize(writes)); + MockConnect connect_data(SYNCHRONOUS, OK); data.set_connect_data(connect_data); - session_deps_.socket_factory->AddSocketDataProvider(&data); + session_deps_.deterministic_socket_factory->AddSocketDataProvider(&data); SSLSocketDataProvider ssl(SYNCHRONOUS, OK); - session_deps_.socket_factory->AddSSLSocketDataProvider(&ssl); + session_deps_.deterministic_socket_factory->AddSSLSocketDataProvider(&ssl); - CreateNetworkSession(); + CreateDeterministicNetworkSession(); // Initialize the SpdySetting with 1 max concurrent streams. spdy_session_pool_->http_server_properties()->SetSpdySetting( test_host_port_pair_, - kSpdySettingsIds1, + kSpdySettingsIds, SETTINGS_FLAG_PLEASE_PERSIST, - 1); + initial_max_concurrent_streams); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, BoundNetLog()); // Create 2 streams. First will succeed. Second will be pending. @@ -997,9 +977,13 @@ TEST_P(SpdySessionTest, OnSettings) { SPDY_BIDIRECTIONAL_STREAM, session, test_url_, MEDIUM, BoundNetLog(), stream_releaser.MakeCallback(&request))); - session = NULL; - EXPECT_EQ(ERR_ABORTED, stream_releaser.WaitForResult()); + data.RunFor(2); + + EXPECT_EQ(OK, stream_releaser.WaitForResult()); + + data.RunFor(1); + EXPECT_TRUE(session == NULL); } // Start with max concurrent streams set to 1 (that is persisted). Receive a @@ -1008,44 +992,57 @@ TEST_P(SpdySessionTest, OnSettings) { TEST_P(SpdySessionTest, ClearSettings) { session_deps_.host_resolver->set_synchronous_mode(true); + const SpdySettingsIds kSpdySettingsIds = SETTINGS_MAX_CONCURRENT_STREAMS; + + SettingsMap initial_settings; + const uint32 initial_max_concurrent_streams = 1; + initial_settings[kSpdySettingsIds] = + SettingsFlagsAndValue(SETTINGS_FLAG_PERSISTED, + initial_max_concurrent_streams); + scoped_ptr initial_settings_frame( + spdy_util_.ConstructSpdySettings(initial_settings)); + MockWrite writes[] = { + CreateMockWrite(*initial_settings_frame, 0), + }; + SettingsMap new_settings; - const SpdySettingsIds kSpdySettingsIds1 = SETTINGS_MAX_CONCURRENT_STREAMS; const uint32 max_concurrent_streams = 2; - new_settings[kSpdySettingsIds1] = + new_settings[kSpdySettingsIds] = SettingsFlagsAndValue(SETTINGS_FLAG_NONE, max_concurrent_streams); // Set up the socket so we read a SETTINGS frame that raises max concurrent - // streams to 2 and clears previously persisted data. - MockConnect connect_data(SYNCHRONOUS, OK); + // streams to 2. scoped_ptr settings_frame( spdy_util_.ConstructSpdySettings(new_settings)); uint8 flags = SETTINGS_FLAG_CLEAR_PREVIOUSLY_PERSISTED_SETTINGS; test::SetFrameFlags(settings_frame.get(), flags, spdy_util_.spdy_version()); MockRead reads[] = { - CreateMockRead(*settings_frame), - MockRead(SYNCHRONOUS, 0, 0) // EOF + CreateMockRead(*settings_frame, 1), + MockRead(ASYNC, 0, 2), }; - StaticSocketDataProvider data(reads, arraysize(reads), NULL, 0); + DeterministicSocketData data( + reads, arraysize(reads), writes, arraysize(writes)); + MockConnect connect_data(SYNCHRONOUS, OK); data.set_connect_data(connect_data); - session_deps_.socket_factory->AddSocketDataProvider(&data); + session_deps_.deterministic_socket_factory->AddSocketDataProvider(&data); SSLSocketDataProvider ssl(SYNCHRONOUS, OK); - session_deps_.socket_factory->AddSSLSocketDataProvider(&ssl); + session_deps_.deterministic_socket_factory->AddSSLSocketDataProvider(&ssl); - CreateNetworkSession(); + CreateDeterministicNetworkSession(); // Initialize the SpdySetting with 1 max concurrent streams. spdy_session_pool_->http_server_properties()->SetSpdySetting( test_host_port_pair_, - kSpdySettingsIds1, + kSpdySettingsIds, SETTINGS_FLAG_PLEASE_PERSIST, - 1); + initial_max_concurrent_streams); EXPECT_EQ(1u, spdy_session_pool_->http_server_properties()->GetSpdySettings( test_host_port_pair_).size()); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, BoundNetLog()); // Create 2 streams. First will succeed. Second will be pending. @@ -1063,7 +1060,9 @@ TEST_P(SpdySessionTest, ClearSettings) { BoundNetLog(), stream_releaser.MakeCallback(&request))); - EXPECT_EQ(ERR_ABORTED, stream_releaser.WaitForResult()); + data.RunFor(2); + + EXPECT_EQ(OK, stream_releaser.WaitForResult()); // Make sure that persisted data is cleared. EXPECT_EQ(0u, spdy_session_pool_->http_server_properties()->GetSpdySettings( @@ -1072,7 +1071,8 @@ TEST_P(SpdySessionTest, ClearSettings) { // Make sure session's max_concurrent_streams is 2. EXPECT_EQ(2u, session->max_concurrent_streams()); - session = NULL; + data.RunFor(1); + EXPECT_TRUE(session == NULL); } // Start with max concurrent streams set to 1. Request two streams. When the @@ -1104,7 +1104,7 @@ TEST_P(SpdySessionTest, CancelPendingCreateStream) { SETTINGS_FLAG_PLEASE_PERSIST, 1); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, BoundNetLog()); // Create 2 streams. First will succeed. Second will be pending. @@ -1182,7 +1182,7 @@ TEST_P(SpdySessionTest, SendInitialSettingsOnNewSession) { SpdySessionPoolPeer pool_peer(spdy_session_pool_); pool_peer.EnableSendingInitialSettings(true); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, BoundNetLog()); base::MessageLoop::current()->RunUntilIdle(); @@ -1228,7 +1228,7 @@ TEST_P(SpdySessionTest, SendSettingsOnNewSession) { SETTINGS_FLAG_PLEASE_PERSIST, kBogusSettingValue); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, BoundNetLog()); base::MessageLoop::current()->RunUntilIdle(); @@ -1272,7 +1272,7 @@ TEST_P(SpdySessionTest, Initialize) { CreateNetworkSession(); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, log.bound()); EXPECT_TRUE(HasSpdySession(spdy_session_pool_, key_)); @@ -1296,8 +1296,6 @@ TEST_P(SpdySessionTest, Initialize) { &socket_source)); EXPECT_TRUE(socket_source.IsValid()); EXPECT_NE(log.bound().source().id, socket_source.id); - - session->CloseSessionOnError(ERR_ABORTED, std::string()); } TEST_P(SpdySessionTest, CloseSessionOnError) { @@ -1320,7 +1318,7 @@ TEST_P(SpdySessionTest, CloseSessionOnError) { CreateNetworkSession(); CapturingBoundNetLog log; - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, log.bound()); EXPECT_TRUE(HasSpdySession(spdy_session_pool_, key_)); @@ -1328,6 +1326,7 @@ TEST_P(SpdySessionTest, CloseSessionOnError) { base::MessageLoop::current()->RunUntilIdle(); EXPECT_FALSE(HasSpdySession(spdy_session_pool_, key_)); + EXPECT_TRUE(session == NULL); // Check that the NetLog was filled reasonably. net::CapturingNetLog::CapturedEntryList entries; @@ -1348,11 +1347,6 @@ 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 @@ -1398,7 +1392,7 @@ TEST_P(SpdySessionTest, OutOfOrderSynStreams) { CreateDeterministicNetworkSession(); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, BoundNetLog()); GURL url("http://www.google.com"); @@ -1471,7 +1465,7 @@ TEST_P(SpdySessionTest, CancelStream) { CreateDeterministicNetworkSession(); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, BoundNetLog()); GURL url1("http://www.google.com"); @@ -1544,7 +1538,7 @@ TEST_P(SpdySessionTest, CloseSessionWithTwoCreatedSelfClosingStreams) { CreateDeterministicNetworkSession(); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, BoundNetLog()); GURL url1("http://www.google.com"); @@ -1590,7 +1584,7 @@ TEST_P(SpdySessionTest, CloseSessionWithTwoCreatedSelfClosingStreams) { EXPECT_TRUE(delegate1.StreamIsClosed()); EXPECT_TRUE(delegate2.StreamIsClosed()); - session = NULL; + EXPECT_TRUE(session == NULL); } // Create two streams that are set to close each other on close, and @@ -1618,7 +1612,7 @@ TEST_P(SpdySessionTest, CloseSessionWithTwoCreatedMutuallyClosingStreams) { CreateDeterministicNetworkSession(); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, BoundNetLog()); GURL url1("http://www.google.com"); @@ -1666,7 +1660,7 @@ TEST_P(SpdySessionTest, CloseSessionWithTwoCreatedMutuallyClosingStreams) { EXPECT_TRUE(delegate1.StreamIsClosed()); EXPECT_TRUE(delegate2.StreamIsClosed()); - session = NULL; + EXPECT_TRUE(session == NULL); } // Create two streams that are set to re-close themselves on close, @@ -1699,7 +1693,7 @@ TEST_P(SpdySessionTest, CloseSessionWithTwoActivatedSelfClosingStreams) { CreateDeterministicNetworkSession(); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, BoundNetLog()); GURL url1("http://www.google.com"); @@ -1750,7 +1744,7 @@ TEST_P(SpdySessionTest, CloseSessionWithTwoActivatedSelfClosingStreams) { EXPECT_TRUE(delegate1.StreamIsClosed()); EXPECT_TRUE(delegate2.StreamIsClosed()); - session = NULL; + EXPECT_TRUE(session == NULL); } // Create two streams that are set to close each other on close, @@ -1783,7 +1777,7 @@ TEST_P(SpdySessionTest, CloseSessionWithTwoActivatedMutuallyClosingStreams) { CreateDeterministicNetworkSession(); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, BoundNetLog()); GURL url1("http://www.google.com"); @@ -1836,7 +1830,7 @@ TEST_P(SpdySessionTest, CloseSessionWithTwoActivatedMutuallyClosingStreams) { EXPECT_TRUE(delegate1.StreamIsClosed()); EXPECT_TRUE(delegate2.StreamIsClosed()); - session = NULL; + EXPECT_TRUE(session == NULL); } TEST_P(SpdySessionTest, VerifyDomainAuthentication) { @@ -1872,7 +1866,7 @@ TEST_P(SpdySessionTest, VerifyDomainAuthentication) { CreateDeterministicNetworkSession(); - scoped_refptr session = + base::WeakPtr session = CreateSecureSpdySession(http_session_, key_, BoundNetLog()); EXPECT_TRUE(session->VerifyDomainAuthentication("www.example.org")); @@ -1915,7 +1909,7 @@ TEST_P(SpdySessionTest, ConnectionPooledWithTlsChannelId) { CreateDeterministicNetworkSession(); - scoped_refptr session = + base::WeakPtr session = CreateSecureSpdySession(http_session_, key_, BoundNetLog()); EXPECT_TRUE(session->VerifyDomainAuthentication("www.example.org")); @@ -1982,7 +1976,7 @@ TEST_P(SpdySessionTest, CloseTwoStalledCreateStream) { CreateDeterministicNetworkSession(); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, BoundNetLog()); // Read the settings frame. @@ -2056,11 +2050,13 @@ TEST_P(SpdySessionTest, CloseTwoStalledCreateStream) { EXPECT_TRUE(stream3->HasUrlFromHeaders()); EXPECT_EQ(0u, delegate3.stream_id()); - data.RunFor(4); + data.RunFor(3); EXPECT_EQ(NULL, stream3.get()); EXPECT_EQ(5u, delegate3.stream_id()); EXPECT_EQ(0u, session->num_active_streams() + session->num_created_streams()); EXPECT_EQ(0u, session->pending_create_stream_queue_size(LOWEST)); + + data.RunFor(1); } TEST_P(SpdySessionTest, CancelTwoStalledCreateStream) { @@ -2088,7 +2084,7 @@ TEST_P(SpdySessionTest, CancelTwoStalledCreateStream) { SETTINGS_FLAG_PLEASE_PERSIST, 1); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, BoundNetLog()); GURL url1("http://www.google.com"); @@ -2162,7 +2158,7 @@ TEST_P(SpdySessionTest, NeedsCredentials) { SpdySessionKey key(test_host_port_pair, ProxyServer::Direct(), kPrivacyModeDisabled); - scoped_refptr session = + base::WeakPtr session = CreateSecureSpdySession(http_session_, key, BoundNetLog()); EXPECT_EQ(spdy_util_.spdy_version() >= SPDY3, session->NeedsCredentials()); @@ -2227,7 +2223,7 @@ TEST_P(SpdySessionTest, ReadDataWithoutYielding) { CreateDeterministicNetworkSession(); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, BoundNetLog()); GURL url1("http://www.google.com"); @@ -2321,7 +2317,7 @@ TEST_P(SpdySessionTest, TestYieldingDuringReadData) { CreateDeterministicNetworkSession(); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, BoundNetLog()); GURL url1("http://www.google.com"); @@ -2437,7 +2433,7 @@ TEST_P(SpdySessionTest, TestYieldingDuringAsyncReadData) { CreateDeterministicNetworkSession(); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, BoundNetLog()); GURL url1("http://www.google.com"); @@ -2477,11 +2473,8 @@ TEST_P(SpdySessionTest, TestYieldingDuringAsyncReadData) { EXPECT_TRUE(data.at_read_eof()); } -// Send a GoAway frame when SpdySession is in DoReadLoop. If -// scoped_refptr to is deleted from -// SpdySession::DoReadLoop(), we get a crash because GoAway could -// delete the SpdySession from the SpdySessionPool and the last -// reference to SpdySession. +// Send a GoAway frame when SpdySession is in DoReadLoop. Make sure +// nothing blows up. TEST_P(SpdySessionTest, GoAwayWhileInDoReadLoop) { MockConnect connect_data(SYNCHRONOUS, OK); BufferedSpdyFramer framer(spdy_util_.spdy_version(), false); @@ -2514,7 +2507,7 @@ TEST_P(SpdySessionTest, GoAwayWhileInDoReadLoop) { CreateDeterministicNetworkSession(); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, BoundNetLog()); GURL url1("http://www.google.com"); @@ -2523,7 +2516,6 @@ TEST_P(SpdySessionTest, GoAwayWhileInDoReadLoop) { session, url1, MEDIUM, BoundNetLog()); test::StreamDelegateDoNothing delegate1(spdy_stream1); spdy_stream1->SetDelegate(&delegate1); - session = NULL; ASSERT_TRUE(spdy_stream1.get() != NULL); EXPECT_EQ(0u, spdy_stream1->stream_id()); @@ -2537,15 +2529,12 @@ TEST_P(SpdySessionTest, GoAwayWhileInDoReadLoop) { data.RunFor(1); EXPECT_EQ(1u, spdy_stream1->stream_id()); - // Only references to SpdySession are held by DoReadLoop and - // SpdySessionPool. If DoReadLoop doesn't hold the reference, we get a - // crash if SpdySession is deleted from the SpdySessionPool. - // Run until GoAway. data.RunFor(3); EXPECT_EQ(NULL, spdy_stream1.get()); EXPECT_TRUE(data.at_write_eof()); EXPECT_TRUE(data.at_read_eof()); + EXPECT_TRUE(session == NULL); } // Within this framework, a SpdySession should be initialized with @@ -2564,7 +2553,7 @@ TEST_P(SpdySessionTest, ProtocolNegotiation) { session_deps_.socket_factory->AddSocketDataProvider(&data); CreateNetworkSession(); - scoped_refptr session = + base::WeakPtr session = CreateFakeSpdySession(spdy_session_pool_, key_); EXPECT_EQ(spdy_util_.spdy_version(), @@ -2614,11 +2603,9 @@ TEST_P(SpdySessionTest, CloseOneIdleConnection) { // Create an idle SPDY session. SpdySessionKey key1(HostPortPair("1.com", 80), ProxyServer::Direct(), kPrivacyModeDisabled); - scoped_refptr session1 = + base::WeakPtr session1 = CreateInsecureSpdySession(http_session_, key1, BoundNetLog()); EXPECT_FALSE(pool->IsStalled()); - // Release the pointer to the session so it can be closed. - session1 = NULL; // Trying to create a new connection should cause the pool to be stalled, and // post a task asynchronously to try and close the session. @@ -2637,6 +2624,7 @@ TEST_P(SpdySessionTest, CloseOneIdleConnection) { // new connection. EXPECT_EQ(OK, callback2.WaitForResult()); EXPECT_FALSE(pool->IsStalled()); + EXPECT_TRUE(session1 == NULL); } // Tests the case of a non-SPDY request closing an idle SPDY session when no @@ -2675,7 +2663,7 @@ TEST_P(SpdySessionTest, CloseOneIdleConnectionWithAlias) { // Create an idle SPDY session. SpdySessionKey key1(HostPortPair("1.com", 80), ProxyServer::Direct(), kPrivacyModeDisabled); - scoped_refptr session1 = + base::WeakPtr session1 = CreateInsecureSpdySession(http_session_, key1, BoundNetLog()); EXPECT_FALSE(pool->IsStalled()); @@ -2689,15 +2677,11 @@ TEST_P(SpdySessionTest, CloseOneIdleConnectionWithAlias) { session_deps_.host_resolver->Resolve( info, &addresses, CompletionCallback(), NULL, BoundNetLog()); // Get a session for |key2|, which should return the session created earlier. - scoped_refptr session2 = + base::WeakPtr session2 = spdy_session_pool_->FindAvailableSession(key2, BoundNetLog()); ASSERT_EQ(session1.get(), session2.get()); EXPECT_FALSE(pool->IsStalled()); - // Release both the pointers to the session so it can be closed. - session1 = NULL; - session2 = NULL; - // Trying to create a new connection should cause the pool to be stalled, and // post a task asynchronously to try and close the session. TestCompletionCallback callback3; @@ -2715,62 +2699,8 @@ TEST_P(SpdySessionTest, CloseOneIdleConnectionWithAlias) { // new connection. EXPECT_EQ(OK, callback3.WaitForResult()); EXPECT_FALSE(pool->IsStalled()); -} - -// Tests the case of a non-SPDY request closing an idle SPDY session when a -// pointer to the idle session is still held. -TEST_P(SpdySessionTest, CloseOneIdleConnectionSessionStillHeld) { - ClientSocketPoolManager::set_max_sockets_per_group( - HttpNetworkSession::NORMAL_SOCKET_POOL, 1); - ClientSocketPoolManager::set_max_sockets_per_pool( - HttpNetworkSession::NORMAL_SOCKET_POOL, 1); - - MockConnect connect_data(SYNCHRONOUS, OK); - MockRead reads[] = { - MockRead(SYNCHRONOUS, ERR_IO_PENDING) // Stall forever. - }; - StaticSocketDataProvider data(reads, arraysize(reads), NULL, 0); - data.set_connect_data(connect_data); - session_deps_.socket_factory->AddSocketDataProvider(&data); - session_deps_.socket_factory->AddSocketDataProvider(&data); - - CreateNetworkSession(); - - TransportClientSocketPool* pool = - http_session_->GetTransportSocketPool( - HttpNetworkSession::NORMAL_SOCKET_POOL); - - // Create an idle SPDY session. - SpdySessionKey key1(HostPortPair("1.com", 80), ProxyServer::Direct(), - kPrivacyModeDisabled); - scoped_refptr session1 = - CreateInsecureSpdySession(http_session_, key1, BoundNetLog()); - EXPECT_FALSE(pool->IsStalled()); - - // Trying to create a new connection should cause the pool to be stalled, and - // post a task asynchronously to try and close the session. - TestCompletionCallback callback2; - HostPortPair host_port2("2.com", 80); - scoped_refptr params2( - new TransportSocketParams(host_port2, DEFAULT_PRIORITY, false, false, - OnHostResolutionCallback())); - scoped_ptr connection2(new ClientSocketHandle); - EXPECT_EQ(ERR_IO_PENDING, - connection2->Init(host_port2.ToString(), params2, DEFAULT_PRIORITY, - callback2.callback(), pool, BoundNetLog())); - EXPECT_TRUE(pool->IsStalled()); - - // Running the message loop should cause the session to prepare to be closed, - // but since there's still an outstanding reference, it should not be closed - // yet. - base::RunLoop().RunUntilIdle(); - EXPECT_TRUE(pool->IsStalled()); - EXPECT_FALSE(callback2.have_result()); - - // Release the pointer to the session so it can be closed. - session1 = NULL; - EXPECT_EQ(OK, callback2.WaitForResult()); - EXPECT_FALSE(pool->IsStalled()); + EXPECT_TRUE(session1 == NULL); + EXPECT_TRUE(session2 == NULL); } // Tests that a non-SPDY request can't close a SPDY session that's currently in @@ -2808,7 +2738,7 @@ TEST_P(SpdySessionTest, CloseOneIdleConnectionFailsWhenSessionInUse) { GURL url1("http://www.google.com"); SpdySessionKey key1(HostPortPair(url1.host(), 80), ProxyServer::Direct(), kPrivacyModeDisabled); - scoped_refptr session1 = + base::WeakPtr session1 = CreateInsecureSpdySession(http_session_, key1, BoundNetLog()); EXPECT_FALSE(pool->IsStalled()); @@ -2832,10 +2762,6 @@ TEST_P(SpdySessionTest, CloseOneIdleConnectionFailsWhenSessionInUse) { base::MessageLoop::current()->RunUntilIdle(); - // Release the session, so holding onto a pointer here does not affect - // anything. - session1 = NULL; - // Trying to create a new connection should cause the pool to be stalled, and // post a task asynchronously to try and close the session. TestCompletionCallback callback2; @@ -2863,6 +2789,7 @@ TEST_P(SpdySessionTest, CloseOneIdleConnectionFailsWhenSessionInUse) { base::RunLoop().RunUntilIdle(); EXPECT_TRUE(pool->IsStalled()); EXPECT_FALSE(callback2.have_result()); + EXPECT_TRUE(session1 != NULL); } // Verify that SpdySessionKey and therefore SpdySession is different when @@ -2880,14 +2807,14 @@ TEST_P(SpdySessionTest, SpdySessionKeyPrivacyMode) { EXPECT_FALSE(HasSpdySession(spdy_session_pool_, key_privacy_disabled)); // Add SpdySession with PrivacyMode Enabled to the pool. - scoped_refptr session_privacy_enabled = + base::WeakPtr session_privacy_enabled = CreateFakeSpdySession(spdy_session_pool_, key_privacy_enabled); EXPECT_TRUE(HasSpdySession(spdy_session_pool_, key_privacy_enabled)); EXPECT_FALSE(HasSpdySession(spdy_session_pool_, key_privacy_disabled)); // Add SpdySession with PrivacyMode Disabled to the pool. - scoped_refptr session_privacy_disabled = + base::WeakPtr session_privacy_disabled = CreateFakeSpdySession(spdy_session_pool_, key_privacy_disabled); EXPECT_TRUE(HasSpdySession(spdy_session_pool_, key_privacy_enabled)); @@ -2935,7 +2862,7 @@ TEST_P(SpdySessionTest, SendCredentials) { SpdySessionKey key(test_host_port_pair, ProxyServer::Direct(), kPrivacyModeDisabled); - scoped_refptr session = + base::WeakPtr session = CreateSecureSpdySession(http_session_, key, BoundNetLog()); EXPECT_TRUE(session->NeedsCredentials()); @@ -2980,7 +2907,7 @@ TEST_P(SpdySessionTest, UpdateStreamsSendWindowSize) { CreateDeterministicNetworkSession(); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, BoundNetLog()); base::WeakPtr spdy_stream1 = CreateStreamSynchronously(SPDY_BIDIRECTIONAL_STREAM, @@ -3041,7 +2968,7 @@ TEST_P(SpdySessionTest, AdjustRecvWindowSize) { session_deps_.deterministic_socket_factory->AddSSLSocketDataProvider(&ssl); CreateDeterministicNetworkSession(); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, BoundNetLog()); EXPECT_EQ(SpdySession::FLOW_CONTROL_STREAM_AND_SESSION, session->flow_control_state()); @@ -3091,7 +3018,7 @@ TEST_P(SpdySessionTest, AdjustSendWindowSize) { session_deps_.socket_factory->AddSocketDataProvider(&data); CreateNetworkSession(); - scoped_refptr session = + base::WeakPtr session = CreateFakeSpdySession(spdy_session_pool_, key_); EXPECT_EQ(SpdySession::FLOW_CONTROL_STREAM_AND_SESSION, session->flow_control_state()); @@ -3130,7 +3057,7 @@ TEST_P(SpdySessionTest, SessionFlowControlInactiveStream) { session_deps_.deterministic_socket_factory->AddSSLSocketDataProvider(&ssl); CreateDeterministicNetworkSession(); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, BoundNetLog()); EXPECT_EQ(SpdySession::FLOW_CONTROL_STREAM_AND_SESSION, session->flow_control_state()); @@ -3138,10 +3065,12 @@ TEST_P(SpdySessionTest, SessionFlowControlInactiveStream) { EXPECT_EQ(kSpdySessionInitialWindowSize, session->session_recv_window_size_); EXPECT_EQ(0, session->session_unacked_recv_window_bytes_); - data.RunFor(2); + data.RunFor(1); EXPECT_EQ(kSpdySessionInitialWindowSize, session->session_recv_window_size_); EXPECT_EQ(0, session->session_unacked_recv_window_bytes_); + + data.RunFor(1); } // A delegate that drops any received data. @@ -3207,7 +3136,7 @@ TEST_P(SpdySessionTest, SessionFlowControlNoReceiveLeaks) { CreateDeterministicNetworkSession(); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, BoundNetLog()); GURL url(kStreamUrl); @@ -3285,7 +3214,7 @@ TEST_P(SpdySessionTest, SessionFlowControlNoSendLeaks) { CreateDeterministicNetworkSession(); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, BoundNetLog()); GURL url(kStreamUrl); @@ -3377,7 +3306,7 @@ TEST_P(SpdySessionTest, SessionFlowControlEndToEnd) { CreateDeterministicNetworkSession(); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, BoundNetLog()); GURL url(kStreamUrl); @@ -3494,7 +3423,7 @@ void SpdySessionTest::RunResumeAfterUnstallTest( session_deps_.deterministic_socket_factory->AddSocketDataProvider(&data); CreateDeterministicNetworkSession(); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, BoundNetLog()); EXPECT_EQ(SpdySession::FLOW_CONTROL_STREAM_AND_SESSION, session->flow_control_state()); @@ -3654,7 +3583,7 @@ TEST_P(SpdySessionTest, ResumeByPriorityAfterSendWindowSizeIncrease) { session_deps_.deterministic_socket_factory->AddSocketDataProvider(&data); CreateDeterministicNetworkSession(); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, BoundNetLog()); EXPECT_EQ(SpdySession::FLOW_CONTROL_STREAM_AND_SESSION, session->flow_control_state()); @@ -3809,7 +3738,7 @@ TEST_P(SpdySessionTest, SendWindowSizeIncreaseWithDeletedStreams) { session_deps_.deterministic_socket_factory->AddSocketDataProvider(&data); CreateDeterministicNetworkSession(); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, BoundNetLog()); EXPECT_EQ(SpdySession::FLOW_CONTROL_STREAM_AND_SESSION, session->flow_control_state()); @@ -3965,7 +3894,7 @@ TEST_P(SpdySessionTest, SendWindowSizeIncreaseWithDeletedSession) { session_deps_.deterministic_socket_factory->AddSocketDataProvider(&data); CreateDeterministicNetworkSession(); - scoped_refptr session = + base::WeakPtr session = CreateInsecureSpdySession(http_session_, key_, BoundNetLog()); EXPECT_EQ(SpdySession::FLOW_CONTROL_STREAM_AND_SESSION, session->flow_control_state()); @@ -4025,7 +3954,7 @@ TEST_P(SpdySessionTest, SendWindowSizeIncreaseWithDeletedSession) { // Close the session (since we can't do it from within the delegate // method, since it's in the stream's loop). session->CloseSessionOnError(ERR_CONNECTION_CLOSED, "Closing session"); - session = NULL; + EXPECT_TRUE(session == NULL); EXPECT_FALSE(HasSpdySession(spdy_session_pool_, key_)); diff --git a/net/spdy/spdy_stream.cc b/net/spdy/spdy_stream.cc index 8b5718dcd4aa21..5b44fe8302042b 100644 --- a/net/spdy/spdy_stream.cc +++ b/net/spdy/spdy_stream.cc @@ -78,7 +78,7 @@ class SpdyStream::SynStreamBufferProducer : public SpdyBufferProducer { }; SpdyStream::SpdyStream(SpdyStreamType type, - SpdySession* session, + const base::WeakPtr& session, const GURL& url, RequestPriority priority, int32 initial_send_window_size, diff --git a/net/spdy/spdy_stream.h b/net/spdy/spdy_stream.h index 185d3e46688604..1db1217c87aa5b 100644 --- a/net/spdy/spdy_stream.h +++ b/net/spdy/spdy_stream.h @@ -157,7 +157,7 @@ class NET_EXPORT_PRIVATE SpdyStream { // SpdyStream constructor SpdyStream(SpdyStreamType type, - SpdySession* session, + const base::WeakPtr& session, const GURL& url, RequestPriority priority, int32 initial_send_window_size, @@ -334,7 +334,7 @@ class NET_EXPORT_PRIVATE SpdyStream { // it. void Close(); - // Must be used only by the SpdySession. + // Must be used only by |session_|. base::WeakPtr GetWeakPtr(); // Interface for the delegate to use. @@ -490,7 +490,7 @@ class NET_EXPORT_PRIVATE SpdyStream { ScopedBandwidthMetrics metrics_; - scoped_refptr session_; + const base::WeakPtr session_; // The transaction should own the delegate. SpdyStream::Delegate* delegate_; diff --git a/net/spdy/spdy_stream_unittest.cc b/net/spdy/spdy_stream_unittest.cc index 623674f7c175e8..6a14d67d95277d 100644 --- a/net/spdy/spdy_stream_unittest.cc +++ b/net/spdy/spdy_stream_unittest.cc @@ -50,7 +50,7 @@ class SpdyStreamTest : public ::testing::Test, session_deps_(GetParam()), offset_(0) {} - scoped_refptr CreateDefaultSpdySession() { + base::WeakPtr CreateDefaultSpdySession() { SpdySessionKey key(HostPortPair("www.google.com", 80), ProxyServer::Direct(), kPrivacyModeDisabled); @@ -145,7 +145,7 @@ TEST_P(SpdyStreamTest, SendDataAfterOpen) { session_deps_.socket_factory->AddSocketDataProvider(&data); - scoped_refptr session(CreateDefaultSpdySession()); + base::WeakPtr session(CreateDefaultSpdySession()); base::WeakPtr stream = CreateStreamSynchronously( @@ -187,11 +187,11 @@ TEST_P(SpdyStreamTest, PushedStream) { session_deps_.socket_factory->AddSocketDataProvider(&data); - scoped_refptr spdy_session(CreateDefaultSpdySession()); + base::WeakPtr spdy_session(CreateDefaultSpdySession()); // Conjure up a stream. SpdyStream stream(SPDY_PUSH_STREAM, - spdy_session.get(), + spdy_session, GURL(), DEFAULT_PRIORITY, kSpdyStreamInitialWindowSize, @@ -222,7 +222,7 @@ TEST_P(SpdyStreamTest, PushedStream) { EXPECT_EQ("200", delegate.GetResponseHeaderValue(spdy_util_.GetStatusKey())); - spdy_session->CloseSessionOnError(ERR_CONNECTION_CLOSED, "Closing session"); + EXPECT_TRUE(spdy_session == NULL); } TEST_P(SpdyStreamTest, StreamError) { @@ -258,7 +258,7 @@ TEST_P(SpdyStreamTest, StreamError) { session_deps_.socket_factory->AddSocketDataProvider(&data); - scoped_refptr session(CreateDefaultSpdySession()); + base::WeakPtr session(CreateDefaultSpdySession()); base::WeakPtr stream = CreateStreamSynchronously( @@ -342,7 +342,7 @@ TEST_P(SpdyStreamTest, SendLargeDataAfterOpenRequestResponse) { session_deps_.socket_factory->AddSocketDataProvider(&data); - scoped_refptr session(CreateDefaultSpdySession()); + base::WeakPtr session(CreateDefaultSpdySession()); base::WeakPtr stream = CreateStreamSynchronously( @@ -405,7 +405,7 @@ TEST_P(SpdyStreamTest, SendLargeDataAfterOpenBidirectional) { session_deps_.socket_factory->AddSocketDataProvider(&data); - scoped_refptr session(CreateDefaultSpdySession()); + base::WeakPtr session(CreateDefaultSpdySession()); base::WeakPtr stream = CreateStreamSynchronously( @@ -465,7 +465,7 @@ TEST_P(SpdyStreamTest, UpperCaseHeaders) { session_deps_.deterministic_socket_factory->AddSocketDataProvider(&data); - scoped_refptr session(CreateDefaultSpdySession()); + base::WeakPtr session(CreateDefaultSpdySession()); base::WeakPtr stream = CreateStreamSynchronously( @@ -523,7 +523,7 @@ TEST_P(SpdyStreamTest, UpperCaseHeadersOnPush) { session_deps_.deterministic_socket_factory->AddSocketDataProvider(&data); - scoped_refptr session(CreateDefaultSpdySession()); + base::WeakPtr session(CreateDefaultSpdySession()); base::WeakPtr stream = CreateStreamSynchronously( @@ -598,7 +598,7 @@ TEST_P(SpdyStreamTest, UpperCaseHeadersInHeadersFrame) { session_deps_.deterministic_socket_factory->AddSocketDataProvider(&data); - scoped_refptr session(CreateDefaultSpdySession()); + base::WeakPtr session(CreateDefaultSpdySession()); base::WeakPtr stream = CreateStreamSynchronously( @@ -678,7 +678,7 @@ TEST_P(SpdyStreamTest, DuplicateHeaders) { session_deps_.deterministic_socket_factory->AddSocketDataProvider(&data); - scoped_refptr session(CreateDefaultSpdySession()); + base::WeakPtr session(CreateDefaultSpdySession()); base::WeakPtr stream = CreateStreamSynchronously( @@ -747,7 +747,7 @@ TEST_P(SpdyStreamTest, IncreaseSendWindowSizeOverflow) { session_deps_.deterministic_socket_factory->AddSocketDataProvider(&data); - scoped_refptr session(CreateDefaultSpdySession()); + base::WeakPtr session(CreateDefaultSpdySession()); GURL url(kStreamUrl); base::WeakPtr stream = @@ -838,7 +838,7 @@ void SpdyStreamTest::RunResumeAfterUnstallRequestResponseTest( session_deps_.deterministic_socket_factory->AddSocketDataProvider(&data); - scoped_refptr session(CreateDefaultSpdySession()); + base::WeakPtr session(CreateDefaultSpdySession()); base::WeakPtr stream = CreateStreamSynchronously( @@ -930,7 +930,7 @@ void SpdyStreamTest::RunResumeAfterUnstallBidirectionalTest( session_deps_.deterministic_socket_factory->AddSocketDataProvider(&data); - scoped_refptr session(CreateDefaultSpdySession()); + base::WeakPtr session(CreateDefaultSpdySession()); base::WeakPtr stream = CreateStreamSynchronously( diff --git a/net/spdy/spdy_test_util_common.cc b/net/spdy/spdy_test_util_common.cc index ef603fbf2ec1db..2881844a79ae22 100644 --- a/net/spdy/spdy_test_util_common.cc +++ b/net/spdy/spdy_test_util_common.cc @@ -269,7 +269,7 @@ bool GetSpdyPriority(SpdyMajorVersion version, base::WeakPtr CreateStreamSynchronously( SpdyStreamType type, - const scoped_refptr& session, + const base::WeakPtr& session, const GURL& url, RequestPriority priority, const BoundNetLog& net_log) { @@ -489,7 +489,7 @@ bool HasSpdySession(SpdySessionPool* pool, const SpdySessionKey& key) { namespace { -scoped_refptr CreateSpdySessionHelper( +base::WeakPtr CreateSpdySessionHelper( const scoped_refptr& http_session, const SpdySessionKey& key, const BoundNetLog& net_log, @@ -543,7 +543,7 @@ scoped_refptr CreateSpdySessionHelper( EXPECT_EQ(OK, rv); - scoped_refptr spdy_session; + base::WeakPtr spdy_session; EXPECT_EQ( expected_status, http_session->spdy_session_pool()->CreateAvailableSessionFromSocket( @@ -557,7 +557,7 @@ scoped_refptr CreateSpdySessionHelper( } // namespace -scoped_refptr CreateInsecureSpdySession( +base::WeakPtr CreateInsecureSpdySession( const scoped_refptr& http_session, const SpdySessionKey& key, const BoundNetLog& net_log) { @@ -575,7 +575,7 @@ void TryCreateInsecureSpdySessionExpectingFailure( expected_error, false /* is_secure */); } -scoped_refptr CreateSecureSpdySession( +base::WeakPtr CreateSecureSpdySession( const scoped_refptr& http_session, const SpdySessionKey& key, const BoundNetLog& net_log) { @@ -640,13 +640,13 @@ class FakeSpdySessionClientSocket : public MockClientSocket { int read_result_; }; -scoped_refptr CreateFakeSpdySessionHelper( +base::WeakPtr CreateFakeSpdySessionHelper( SpdySessionPool* pool, const SpdySessionKey& key, Error expected_status) { EXPECT_NE(expected_status, ERR_IO_PENDING); EXPECT_FALSE(HasSpdySession(pool, key)); - scoped_refptr spdy_session; + base::WeakPtr spdy_session; scoped_ptr handle(new ClientSocketHandle()); handle->set_socket(new FakeSpdySessionClientSocket( expected_status == OK ? ERR_IO_PENDING : expected_status)); @@ -662,7 +662,7 @@ scoped_refptr CreateFakeSpdySessionHelper( } // namespace -scoped_refptr CreateFakeSpdySession(SpdySessionPool* pool, +base::WeakPtr CreateFakeSpdySession(SpdySessionPool* pool, const SpdySessionKey& key) { return CreateFakeSpdySessionHelper(pool, key, OK); } diff --git a/net/spdy/spdy_test_util_common.h b/net/spdy/spdy_test_util_common.h index a65efc4ed394f1..92efc4917df07b 100644 --- a/net/spdy/spdy_test_util_common.h +++ b/net/spdy/spdy_test_util_common.h @@ -114,7 +114,7 @@ bool GetSpdyPriority(SpdyMajorVersion version, // on failure. base::WeakPtr CreateStreamSynchronously( SpdyStreamType type, - const scoped_refptr& session, + const base::WeakPtr& session, const GURL& url, RequestPriority priority, const BoundNetLog& net_log); @@ -240,7 +240,7 @@ bool HasSpdySession(SpdySessionPool* pool, const SpdySessionKey& key); // Creates a SPDY session for the given key and puts it in the SPDY // session pool in |http_session|. A SPDY session for |key| must not // already exist. -scoped_refptr CreateInsecureSpdySession( +base::WeakPtr CreateInsecureSpdySession( const scoped_refptr& http_session, const SpdySessionKey& key, const BoundNetLog& net_log); @@ -255,7 +255,7 @@ void TryCreateInsecureSpdySessionExpectingFailure( const BoundNetLog& net_log); // Like CreateInsecureSpdySession(), but uses TLS. -scoped_refptr CreateSecureSpdySession( +base::WeakPtr CreateSecureSpdySession( const scoped_refptr& http_session, const SpdySessionKey& key, const BoundNetLog& net_log); @@ -263,7 +263,7 @@ scoped_refptr CreateSecureSpdySession( // Creates an insecure SPDY session for the given key and puts it in // |pool|. The returned session will neither receive nor send any // data. A SPDY session for |key| must not already exist. -scoped_refptr CreateFakeSpdySession(SpdySessionPool* pool, +base::WeakPtr CreateFakeSpdySession(SpdySessionPool* pool, const SpdySessionKey& key); // Tries to create an insecure SPDY session for the given key but diff --git a/net/spdy/spdy_websocket_stream.cc b/net/spdy/spdy_websocket_stream.cc index 3a3022af90cbf1..c0eac28b6fb4e6 100644 --- a/net/spdy/spdy_websocket_stream.cc +++ b/net/spdy/spdy_websocket_stream.cc @@ -18,7 +18,7 @@ namespace net { SpdyWebSocketStream::SpdyWebSocketStream( - SpdySession* spdy_session, Delegate* delegate) + const base::WeakPtr& spdy_session, Delegate* delegate) : weak_ptr_factory_(this), spdy_session_(spdy_session), pending_send_data_length_(0), @@ -35,7 +35,7 @@ SpdyWebSocketStream::~SpdyWebSocketStream() { int SpdyWebSocketStream::InitializeStream(const GURL& url, RequestPriority request_priority, const BoundNetLog& net_log) { - if (spdy_session_->IsClosed()) + if (!spdy_session_) return ERR_SOCKET_NOT_CONNECTED; int rv = stream_request_.StartRequest( diff --git a/net/spdy/spdy_websocket_stream.h b/net/spdy/spdy_websocket_stream.h index d4a71969d9fb01..169d1e481a8711 100644 --- a/net/spdy/spdy_websocket_stream.h +++ b/net/spdy/spdy_websocket_stream.h @@ -58,7 +58,8 @@ class NET_EXPORT_PRIVATE SpdyWebSocketStream virtual ~Delegate() {} }; - SpdyWebSocketStream(SpdySession* spdy_session, Delegate* delegate); + SpdyWebSocketStream(const base::WeakPtr& spdy_session, + Delegate* delegate); virtual ~SpdyWebSocketStream(); // Initializes SPDY stream for the WebSocket. @@ -90,7 +91,7 @@ class NET_EXPORT_PRIVATE SpdyWebSocketStream base::WeakPtrFactory weak_ptr_factory_; SpdyStreamRequest stream_request_; base::WeakPtr stream_; - scoped_refptr spdy_session_; + const base::WeakPtr spdy_session_; size_t pending_send_data_length_; Delegate* delegate_; diff --git a/net/spdy/spdy_websocket_stream_unittest.cc b/net/spdy/spdy_websocket_stream_unittest.cc index 54a71009de08a7..42dd95c434d9d5 100644 --- a/net/spdy/spdy_websocket_stream_unittest.cc +++ b/net/spdy/spdy_websocket_stream_unittest.cc @@ -277,7 +277,7 @@ class SpdyWebSocketStreamTest SpdySessionDependencies session_deps_; scoped_ptr data_; scoped_refptr http_session_; - scoped_refptr session_; + base::WeakPtr session_; scoped_ptr websocket_stream_; SpdyStreamId stream_id_; SpdyStreamId created_stream_id_; @@ -336,7 +336,7 @@ TEST_P(SpdyWebSocketStreamTest, Basic) { base::Bind(&SpdyWebSocketStreamTest::DoSendClosingFrame, base::Unretained(this))); - websocket_stream_.reset(new SpdyWebSocketStream(session_.get(), &delegate)); + websocket_stream_.reset(new SpdyWebSocketStream(session_, &delegate)); BoundNetLog net_log; GURL url("ws://example.com/echo"); @@ -408,7 +408,7 @@ TEST_P(SpdyWebSocketStreamTest, DestructionBeforeClose) { base::Bind(&SpdyWebSocketStreamTest::DoSync, base::Unretained(this))); - websocket_stream_.reset(new SpdyWebSocketStream(session_.get(), &delegate)); + websocket_stream_.reset(new SpdyWebSocketStream(session_, &delegate)); BoundNetLog net_log; GURL url("ws://example.com/echo"); @@ -470,7 +470,7 @@ TEST_P(SpdyWebSocketStreamTest, DestructionAfterExplicitClose) { base::Bind(&SpdyWebSocketStreamTest::DoClose, base::Unretained(this))); - websocket_stream_.reset(new SpdyWebSocketStream(session_.get(), &delegate)); + websocket_stream_.reset(new SpdyWebSocketStream(session_, &delegate)); BoundNetLog net_log; GURL url("ws://example.com/echo"); @@ -535,7 +535,7 @@ TEST_P(SpdyWebSocketStreamTest, IOPending) { SpdyWebSocketStreamEventRecorder block_delegate((CompletionCallback())); scoped_ptr block_stream( - new SpdyWebSocketStream(session_.get(), &block_delegate)); + new SpdyWebSocketStream(session_, &block_delegate)); BoundNetLog block_net_log; GURL block_url("ws://example.com/block"); ASSERT_EQ(OK, @@ -553,7 +553,7 @@ TEST_P(SpdyWebSocketStreamTest, IOPending) { base::Bind(&SpdyWebSocketStreamTest::DoSendClosingFrame, base::Unretained(this))); - websocket_stream_.reset(new SpdyWebSocketStream(session_.get(), &delegate)); + websocket_stream_.reset(new SpdyWebSocketStream(session_, &delegate)); BoundNetLog net_log; GURL url("ws://example.com/echo"); ASSERT_EQ(ERR_IO_PENDING, websocket_stream_->InitializeStream( diff --git a/net/spdy/spdy_write_queue_unittest.cc b/net/spdy/spdy_write_queue_unittest.cc index f0ec14d82fb057..6d6cb3cd3b57fe 100644 --- a/net/spdy/spdy_write_queue_unittest.cc +++ b/net/spdy/spdy_write_queue_unittest.cc @@ -64,8 +64,8 @@ int ProducerToInt(scoped_ptr producer) { // be there. SpdyStream* MakeTestStream(RequestPriority priority) { return new SpdyStream( - SPDY_BIDIRECTIONAL_STREAM, NULL, GURL(), priority, - 0, 0, BoundNetLog()); + SPDY_BIDIRECTIONAL_STREAM, base::WeakPtr(), + GURL(), priority, 0, 0, BoundNetLog()); } // Add some frame producers of different priority. The producers diff --git a/net/websockets/websocket_job.cc b/net/websockets/websocket_job.cc index 3d7c11842770c1..50d121837ac72d 100644 --- a/net/websockets/websocket_job.cc +++ b/net/websockets/websocket_job.cc @@ -597,7 +597,7 @@ int WebSocketJob::TrySpdyStream() { socket_->proxy_server(), privacy_mode); // Forbid wss downgrade to SPDY without SSL. // TODO(toyoshim): Does it realize the same policy with HTTP? - scoped_refptr spdy_session = + base::WeakPtr spdy_session = spdy_pool->FindAvailableSession(key, *socket_->net_log()); if (!spdy_session) return OK; @@ -612,8 +612,7 @@ int WebSocketJob::TrySpdyStream() { // Create SpdyWebSocketStream. spdy_protocol_version_ = spdy_session->GetProtocolVersion(); - spdy_websocket_stream_.reset( - new SpdyWebSocketStream(spdy_session.get(), this)); + spdy_websocket_stream_.reset(new SpdyWebSocketStream(spdy_session, this)); int result = spdy_websocket_stream_->InitializeStream( socket_->url(), MEDIUM, *socket_->net_log()); diff --git a/net/websockets/websocket_job_unittest.cc b/net/websockets/websocket_job_unittest.cc index 3ce52790cba95a..4ee0132ffec9d6 100644 --- a/net/websockets/websocket_job_unittest.cc +++ b/net/websockets/websocket_job_unittest.cc @@ -297,7 +297,7 @@ class MockHttpTransactionFactory : public HttpTransactionFactory { OrderedSocketData* data_; scoped_ptr session_deps_; scoped_refptr http_session_; - scoped_refptr session_; + base::WeakPtr session_; HostPortPair host_port_pair_; SpdySessionKey spdy_session_key_; }; diff --git a/net/websockets/websocket_stream_base.h b/net/websockets/websocket_stream_base.h index fc9acc32c23b18..dc863d2000193e 100644 --- a/net/websockets/websocket_stream_base.h +++ b/net/websockets/websocket_stream_base.h @@ -34,7 +34,7 @@ class NET_EXPORT WebSocketStreamBase { // Create a WebSocketSpdyStream. virtual WebSocketStreamBase* CreateSpdyStream( - SpdySession* session, + const base::WeakPtr& session, bool use_relative_url) = 0; };