diff options
author | willchan@chromium.org <willchan@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-06-28 23:55:46 +0000 |
---|---|---|
committer | willchan@chromium.org <willchan@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-06-28 23:55:46 +0000 |
commit | e3ceb68c8b7176d838a071abdb028a652fa9570d (patch) | |
tree | 72b7ce9d23670aaac5c3cd449618712768d0595d /net | |
parent | 37ebcd63f367449cadde58365bc840aec9ae1a9e (diff) | |
download | chromium_src-e3ceb68c8b7176d838a071abdb028a652fa9570d.zip chromium_src-e3ceb68c8b7176d838a071abdb028a652fa9570d.tar.gz chromium_src-e3ceb68c8b7176d838a071abdb028a652fa9570d.tar.bz2 |
Speculative fix for bug 80095.
Based off previous CHECKs added, it appears that SpdySessionPool::HasSession() may return true, but later on, may return false. I hypothesize that this is because of HostCache entries expiring in between calls to HasSession().
The solution is not to use HasSession(), but instead just call a new SpdySessionPool::GetIfExists() function. If we acquire it, then cache it in a member variable of HttpStreamFactoryImpl::Job.
BUG=80095
TEST=New tests in HttpNetworkTransactionTest. Because it's timing related, it's hard to come up with a solid repro case.
Review URL: http://codereview.chromium.org/7260014
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@90886 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'net')
-rw-r--r-- | net/http/http_network_transaction_unittest.cc | 270 | ||||
-rw-r--r-- | net/http/http_stream_factory_impl_job.cc | 66 | ||||
-rw-r--r-- | net/http/http_stream_factory_impl_job.h | 5 | ||||
-rw-r--r-- | net/spdy/spdy_session.cc | 12 | ||||
-rw-r--r-- | net/spdy/spdy_session.h | 4 | ||||
-rw-r--r-- | net/spdy/spdy_session_pool.cc | 17 | ||||
-rw-r--r-- | net/spdy/spdy_session_pool.h | 15 |
7 files changed, 339 insertions, 50 deletions
diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc index a1de71f..ac86082 100644 --- a/net/http/http_network_transaction_unittest.cc +++ b/net/http/http_network_transaction_unittest.cc @@ -116,8 +116,15 @@ HttpNetworkSession* CreateSession(SessionDependencies* session_deps) { } class HttpNetworkTransactionTest : public PlatformTest { - public: + protected: + struct SimpleGetHelperResult { + int rv; + std::string status_line; + std::string response_data; + }; + virtual void SetUp() { + disabled_spdy_session_domain_verification_ = false; NetworkChangeNotifier::NotifyObserversOfIPAddressChangeForTests(); MessageLoop::current()->RunAllPending(); spdy::SpdyFramer::set_enable_compression_default(false); @@ -132,17 +139,14 @@ class HttpNetworkTransactionTest : public PlatformTest { PlatformTest::TearDown(); NetworkChangeNotifier::NotifyObserversOfIPAddressChangeForTests(); MessageLoop::current()->RunAllPending(); + if (disabled_spdy_session_domain_verification_) { + SpdySession::SetDomainVerification( + spdy_session_domain_verification_old_value_); + } } - protected: void KeepAliveConnectionResendRequestTest(const MockRead& read_failure); - struct SimpleGetHelperResult { - int rv; - std::string status_line; - std::string response_data; - }; - SimpleGetHelperResult SimpleGetHelper(MockRead data_reads[], size_t reads_count) { SimpleGetHelperResult out; @@ -207,6 +211,15 @@ class HttpNetworkTransactionTest : public PlatformTest { int expected_status); void ConnectStatusHelper(const MockRead& status); + + void DisableSpdySessionDomainVerification() { + disabled_spdy_session_domain_verification_ = true; + spdy_session_domain_verification_old_value_ = + SpdySession::SetDomainVerification(false); + } + + bool disabled_spdy_session_domain_verification_; + bool spdy_session_domain_verification_old_value_; }; // Fill |str| with a long header list that consumes >= |size| bytes. @@ -8703,4 +8716,245 @@ TEST_F(HttpNetworkTransactionTest, ClientAuthCertCache_Proxy_Fail) { } } +TEST_F(HttpNetworkTransactionTest, UseIPConnectionPooling) { + HttpStreamFactory::set_use_alternate_protocols(true); + HttpStreamFactory::set_next_protos(kExpectedNPNString); + DisableSpdySessionDomainVerification(); + + // Set up a special HttpNetworkSession with a MockCachingHostResolver. + SessionDependencies session_deps; + MockCachingHostResolver host_resolver; + net::HttpNetworkSession::Params params; + params.client_socket_factory = &session_deps.socket_factory; + params.host_resolver = &host_resolver; + params.cert_verifier = session_deps.cert_verifier.get(); + params.proxy_service = session_deps.proxy_service.get(); + params.ssl_config_service = session_deps.ssl_config_service; + params.http_auth_handler_factory = + session_deps.http_auth_handler_factory.get(); + params.net_log = session_deps.net_log; + scoped_refptr<HttpNetworkSession> session(new HttpNetworkSession(params)); + + SSLSocketDataProvider ssl(true, OK); + ssl.next_proto_status = SSLClientSocket::kNextProtoNegotiated; + ssl.next_proto = "spdy/2"; + ssl.was_npn_negotiated = true; + session_deps.socket_factory.AddSSLSocketDataProvider(&ssl); + + scoped_ptr<spdy::SpdyFrame> host1_req(ConstructSpdyGet( + "https://www.google.com", false, 1, LOWEST)); + scoped_ptr<spdy::SpdyFrame> host2_req(ConstructSpdyGet( + "https://www.gmail.com", false, 3, LOWEST)); + MockWrite spdy_writes[] = { + CreateMockWrite(*host1_req, 1), + CreateMockWrite(*host2_req, 4), + }; + scoped_ptr<spdy::SpdyFrame> host1_resp(ConstructSpdyGetSynReply(NULL, 0, 1)); + scoped_ptr<spdy::SpdyFrame> host1_resp_body(ConstructSpdyBodyFrame(1, true)); + scoped_ptr<spdy::SpdyFrame> host2_resp(ConstructSpdyGetSynReply(NULL, 0, 3)); + scoped_ptr<spdy::SpdyFrame> host2_resp_body(ConstructSpdyBodyFrame(3, true)); + MockRead spdy_reads[] = { + CreateMockRead(*host1_resp, 2), + CreateMockRead(*host1_resp_body, 3), + CreateMockRead(*host2_resp, 5), + CreateMockRead(*host2_resp_body, 6), + MockRead(true, 0, 7), + }; + + scoped_refptr<OrderedSocketData> spdy_data( + new OrderedSocketData( + spdy_reads, arraysize(spdy_reads), + spdy_writes, arraysize(spdy_writes))); + session_deps.socket_factory.AddSocketDataProvider(spdy_data); + + TestCompletionCallback callback; + HttpRequestInfo request1; + request1.method = "GET"; + request1.url = GURL("https://www.google.com/"); + request1.load_flags = 0; + HttpNetworkTransaction trans1(session); + + int rv = trans1.Start(&request1, &callback, BoundNetLog()); + EXPECT_EQ(ERR_IO_PENDING, rv); + EXPECT_EQ(OK, callback.WaitForResult()); + + const HttpResponseInfo* response = trans1.GetResponseInfo(); + ASSERT_TRUE(response != NULL); + ASSERT_TRUE(response->headers != NULL); + EXPECT_EQ("HTTP/1.1 200 OK", response->headers->GetStatusLine()); + + std::string response_data; + ASSERT_EQ(OK, ReadTransaction(&trans1, &response_data)); + EXPECT_EQ("hello!", response_data); + + // Preload www.gmail.com into HostCache. + HostPortPair host_port("www.gmail.com", 443); + HostResolver::RequestInfo resolve_info(host_port); + AddressList ignored; + host_resolver.Resolve(resolve_info, &ignored, NULL, NULL, BoundNetLog()); + + HttpRequestInfo request2; + request2.method = "GET"; + request2.url = GURL("https://www.gmail.com/"); + request2.load_flags = 0; + HttpNetworkTransaction trans2(session); + + rv = trans2.Start(&request2, &callback, BoundNetLog()); + EXPECT_EQ(ERR_IO_PENDING, rv); + EXPECT_EQ(OK, callback.WaitForResult()); + + response = trans2.GetResponseInfo(); + ASSERT_TRUE(response != NULL); + ASSERT_TRUE(response->headers != NULL); + EXPECT_EQ("HTTP/1.1 200 OK", response->headers->GetStatusLine()); + EXPECT_TRUE(response->was_fetched_via_spdy); + EXPECT_TRUE(response->was_npn_negotiated); + ASSERT_EQ(OK, ReadTransaction(&trans2, &response_data)); + EXPECT_EQ("hello!", response_data); + + HttpStreamFactory::set_next_protos(""); + HttpStreamFactory::set_use_alternate_protocols(false); +} + +class OneTimeCachingHostResolver : public net::HostResolver { + public: + explicit OneTimeCachingHostResolver(const HostPortPair& host_port) + : host_port_(host_port) {} + virtual ~OneTimeCachingHostResolver() {} + + RuleBasedHostResolverProc* rules() { return host_resolver_.rules(); } + + // HostResolver methods: + virtual int Resolve(const RequestInfo& info, + AddressList* addresses, + CompletionCallback* callback, + RequestHandle* out_req, + const BoundNetLog& net_log) { + int rv = host_resolver_.Resolve( + info, addresses, callback, out_req, net_log); + if (rv == OK && info.only_use_cached_response() && + info.host_port_pair().Equals(host_port_)) { + // Discard cache. + host_resolver_.Reset(NULL); + } + return rv; + } + + virtual void CancelRequest(RequestHandle req) { + host_resolver_.CancelRequest(req); + } + + virtual void AddObserver(Observer* observer) { + return host_resolver_.AddObserver(observer); + } + + virtual void RemoveObserver(Observer* observer) { + return host_resolver_.RemoveObserver(observer); + } + + private: + MockCachingHostResolver host_resolver_; + const HostPortPair host_port_; +}; + +TEST_F(HttpNetworkTransactionTest, + UseIPConnectionPoolingWithHostCacheExpiration) { + HttpStreamFactory::set_use_alternate_protocols(true); + HttpStreamFactory::set_next_protos(kExpectedNPNString); + DisableSpdySessionDomainVerification(); + + // Set up a special HttpNetworkSession with a OneTimeCachingHostResolver. + SessionDependencies session_deps; + OneTimeCachingHostResolver host_resolver(HostPortPair("www.gmail.com", 443)); + net::HttpNetworkSession::Params params; + params.client_socket_factory = &session_deps.socket_factory; + params.host_resolver = &host_resolver; + params.cert_verifier = session_deps.cert_verifier.get(); + params.proxy_service = session_deps.proxy_service.get(); + params.ssl_config_service = session_deps.ssl_config_service; + params.http_auth_handler_factory = + session_deps.http_auth_handler_factory.get(); + params.net_log = session_deps.net_log; + scoped_refptr<HttpNetworkSession> session(new HttpNetworkSession(params)); + + SSLSocketDataProvider ssl(true, OK); + ssl.next_proto_status = SSLClientSocket::kNextProtoNegotiated; + ssl.next_proto = "spdy/2"; + ssl.was_npn_negotiated = true; + session_deps.socket_factory.AddSSLSocketDataProvider(&ssl); + + scoped_ptr<spdy::SpdyFrame> host1_req(ConstructSpdyGet( + "https://www.google.com", false, 1, LOWEST)); + scoped_ptr<spdy::SpdyFrame> host2_req(ConstructSpdyGet( + "https://www.gmail.com", false, 3, LOWEST)); + MockWrite spdy_writes[] = { + CreateMockWrite(*host1_req, 1), + CreateMockWrite(*host2_req, 4), + }; + scoped_ptr<spdy::SpdyFrame> host1_resp(ConstructSpdyGetSynReply(NULL, 0, 1)); + scoped_ptr<spdy::SpdyFrame> host1_resp_body(ConstructSpdyBodyFrame(1, true)); + scoped_ptr<spdy::SpdyFrame> host2_resp(ConstructSpdyGetSynReply(NULL, 0, 3)); + scoped_ptr<spdy::SpdyFrame> host2_resp_body(ConstructSpdyBodyFrame(3, true)); + MockRead spdy_reads[] = { + CreateMockRead(*host1_resp, 2), + CreateMockRead(*host1_resp_body, 3), + CreateMockRead(*host2_resp, 5), + CreateMockRead(*host2_resp_body, 6), + MockRead(true, 0, 7), + }; + + scoped_refptr<OrderedSocketData> spdy_data( + new OrderedSocketData( + spdy_reads, arraysize(spdy_reads), + spdy_writes, arraysize(spdy_writes))); + session_deps.socket_factory.AddSocketDataProvider(spdy_data); + + TestCompletionCallback callback; + HttpRequestInfo request1; + request1.method = "GET"; + request1.url = GURL("https://www.google.com/"); + request1.load_flags = 0; + HttpNetworkTransaction trans1(session); + + int rv = trans1.Start(&request1, &callback, BoundNetLog()); + EXPECT_EQ(ERR_IO_PENDING, rv); + EXPECT_EQ(OK, callback.WaitForResult()); + + const HttpResponseInfo* response = trans1.GetResponseInfo(); + ASSERT_TRUE(response != NULL); + ASSERT_TRUE(response->headers != NULL); + EXPECT_EQ("HTTP/1.1 200 OK", response->headers->GetStatusLine()); + + std::string response_data; + ASSERT_EQ(OK, ReadTransaction(&trans1, &response_data)); + EXPECT_EQ("hello!", response_data); + + // Preload cache entries into HostCache. + HostResolver::RequestInfo resolve_info(HostPortPair("www.gmail.com", 443)); + AddressList ignored; + host_resolver.Resolve(resolve_info, &ignored, NULL, NULL, BoundNetLog()); + + HttpRequestInfo request2; + request2.method = "GET"; + request2.url = GURL("https://www.gmail.com/"); + request2.load_flags = 0; + HttpNetworkTransaction trans2(session); + + rv = trans2.Start(&request2, &callback, BoundNetLog()); + EXPECT_EQ(ERR_IO_PENDING, rv); + EXPECT_EQ(OK, callback.WaitForResult()); + + response = trans2.GetResponseInfo(); + ASSERT_TRUE(response != NULL); + ASSERT_TRUE(response->headers != NULL); + EXPECT_EQ("HTTP/1.1 200 OK", response->headers->GetStatusLine()); + EXPECT_TRUE(response->was_fetched_via_spdy); + EXPECT_TRUE(response->was_npn_negotiated); + ASSERT_EQ(OK, ReadTransaction(&trans2, &response_data)); + EXPECT_EQ("hello!", response_data); + + HttpStreamFactory::set_next_protos(""); + HttpStreamFactory::set_use_alternate_protocols(false); +} + } // namespace net diff --git a/net/http/http_stream_factory_impl_job.cc b/net/http/http_stream_factory_impl_job.cc index 52dfa9f..8952668 100644 --- a/net/http/http_stream_factory_impl_job.cc +++ b/net/http/http_stream_factory_impl_job.cc @@ -85,7 +85,6 @@ HttpStreamFactoryImpl::Job::Job(HttpStreamFactoryImpl* stream_factory, was_npn_negotiated_(false), num_streams_(0), spdy_session_direct_(false), - expect_to_use_existing_spdy_session_(false), ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)) { DCHECK(stream_factory); DCHECK(session); @@ -567,14 +566,16 @@ int HttpStreamFactoryImpl::Job::DoInitConnection() { } else { spdy_session_key = HostPortProxyPair(origin_, proxy_info_.proxy_server()); } - if (session_->spdy_session_pool()->HasSession(spdy_session_key)) { + scoped_refptr<SpdySession> spdy_session = + session_->spdy_session_pool()->GetIfExists(spdy_session_key, net_log_); + if (spdy_session) { // If we're preconnecting, but we already have a SpdySession, we don't // actually need to preconnect any sockets, so we're done. if (IsPreconnecting()) return OK; using_spdy_ = true; next_state_ = STATE_CREATE_STREAM; - expect_to_use_existing_spdy_session_ = true; + existing_spdy_session_ = spdy_session; return OK; } else if (request_ && (using_ssl_ || ShouldForceSpdyWithoutSSL())) { // Update the spdy session key for the request that launched this job. @@ -756,9 +757,8 @@ int HttpStreamFactoryImpl::Job::DoWaitingUserAction(int result) { } int HttpStreamFactoryImpl::Job::DoCreateStream() { - if (!expect_to_use_existing_spdy_session_ && !connection_->socket()) { + if (!connection_->socket() && !existing_spdy_session_) HACKCrashHereToDebug80095(); - } next_state_ = STATE_CREATE_STREAM_COMPLETE; @@ -780,55 +780,44 @@ int HttpStreamFactoryImpl::Job::DoCreateStream() { CHECK(!stream_.get()); - if (!expect_to_use_existing_spdy_session_ && !connection_->socket()) { + if (!connection_->socket() && !existing_spdy_session_) HACKCrashHereToDebug80095(); - } bool direct = true; - SpdySessionPool* spdy_pool = session_->spdy_session_pool(); - scoped_refptr<SpdySession> spdy_session; - HostPortProxyPair pair(origin_, proxy_server); - if (spdy_pool->HasSession(pair)) { - // We have a SPDY session to the origin server. This might be a direct - // connection, or it might be a SPDY session through an HTTP or HTTPS proxy. - spdy_session = spdy_pool->Get(pair, net_log_); - } else if (IsHttpsProxyAndHttpUrl()) { + if (IsHttpsProxyAndHttpUrl()) { // If we don't have a direct SPDY session, and we're using an HTTPS // proxy, then we might have a SPDY session to the proxy. pair = HostPortProxyPair(proxy_server.host_port_pair(), ProxyServer::Direct()); - if (spdy_pool->HasSession(pair)) { - spdy_session = spdy_pool->Get(pair, net_log_); - } else if (expect_to_use_existing_spdy_session_) { - HACKCrashHereToDebug80095(); - } direct = false; - } else if (expect_to_use_existing_spdy_session_) { - HACKCrashHereToDebug80095(); } - if (spdy_session.get()) { + scoped_refptr<SpdySession> spdy_session; + if (existing_spdy_session_) { // 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_); } else { - // SPDY can be negotiated using the TLS next protocol negotiation (NPN) - // extension, or just directly using SSL. Either way, |connection_| must - // contain an SSLClientSocket. - if (!connection_->socket()) { - HACKCrashHereToDebug80095(); + SpdySessionPool* spdy_pool = session_->spdy_session_pool(); + spdy_session = spdy_pool->GetIfExists(pair, net_log_); + if (!spdy_session) { + // SPDY can be negotiated using the TLS next protocol negotiation (NPN) + // extension, or just directly using SSL. Either way, |connection_| must + // contain an SSLClientSocket. + if (!connection_->socket()) + HACKCrashHereToDebug80095(); + + int error = spdy_pool->GetSpdySessionFromSocket( + pair, connection_.release(), net_log_, spdy_certificate_error_, + &new_spdy_session_, using_ssl_); + if (error != OK) + return error; + spdy_session_direct_ = direct; + return OK; } - - int error = spdy_pool->GetSpdySessionFromSocket( - pair, connection_.release(), net_log_, spdy_certificate_error_, - &spdy_session, using_ssl_); - if (error != OK) - return error; - new_spdy_session_ = spdy_session; - spdy_session_direct_ = direct; - return OK; } if (spdy_session->IsClosed()) @@ -836,8 +825,7 @@ int HttpStreamFactoryImpl::Job::DoCreateStream() { // TODO(willchan): Delete this code, because eventually, the // HttpStreamFactoryImpl will be creating all the SpdyHttpStreams, since it - // will know when SpdySessions become available. The above HasSession() checks - // will be able to be deleted too. + // will know when SpdySessions become available. bool use_relative_url = direct || request_info_.url.SchemeIs("https"); stream_.reset(new SpdyHttpStream(spdy_session, use_relative_url)); diff --git a/net/http/http_stream_factory_impl_job.h b/net/http/http_stream_factory_impl_job.h index 7d43d4d..ef0893d 100644 --- a/net/http/http_stream_factory_impl_job.h +++ b/net/http/http_stream_factory_impl_job.h @@ -268,11 +268,12 @@ class HttpStreamFactoryImpl::Job { // Initialized when we create a new SpdySession. scoped_refptr<SpdySession> new_spdy_session_; + // Initialized when we have an existing SpdySession. + scoped_refptr<SpdySession> existing_spdy_session_; + // Only used if |new_spdy_session_| is non-NULL. bool spdy_session_direct_; - bool expect_to_use_existing_spdy_session_; - ScopedRunnableMethodFactory<Job> method_factory_; DISALLOW_COPY_AND_ASSIGN(Job); diff --git a/net/spdy/spdy_session.cc b/net/spdy/spdy_session.cc index e88d587..d4aace1 100644 --- a/net/spdy/spdy_session.cc +++ b/net/spdy/spdy_session.cc @@ -210,6 +210,9 @@ bool SpdySession::use_flow_control_ = false; // static size_t SpdySession::max_concurrent_stream_limit_ = 256; +// static +bool SpdySession::verify_domain_authentication_ = true; + SpdySession::SpdySession(const HostPortProxyPair& host_port_proxy_pair, SpdySessionPool* spdy_session_pool, SpdySettingsStorage* spdy_settings, @@ -303,6 +306,9 @@ net::Error SpdySession::InitializeWithSocket( } bool SpdySession::VerifyDomainAuthentication(const std::string& domain) { + if (!verify_domain_authentication_) + return true; + if (state_ != CONNECTED) return false; @@ -1521,4 +1527,10 @@ void SpdySession::InvokeUserStreamCreationCallback( callback->Run(result); } +bool SpdySession::SetDomainVerification(bool value) { + bool old_value = verify_domain_authentication_; + verify_domain_authentication_ = value; + return old_value; +} + } // namespace net diff --git a/net/spdy/spdy_session.h b/net/spdy/spdy_session.h index f26d256..1fffc9f 100644 --- a/net/spdy/spdy_session.h +++ b/net/spdy/spdy_session.h @@ -204,6 +204,7 @@ class NET_API SpdySession : public base::RefCounted<SpdySession>, private: friend class base::RefCounted<SpdySession>; FRIEND_TEST_ALL_PREFIXES(SpdySessionTest, GetActivePushStream); + friend class HttpNetworkTransactionTest; struct PendingCreateStream { PendingCreateStream(const GURL& url, RequestPriority priority, @@ -330,6 +331,8 @@ class NET_API SpdySession : public base::RefCounted<SpdySession>, size_t len); virtual void OnControl(const spdy::SpdyControlFrame* frame); + static bool SetDomainVerification(bool value); + // Callbacks for the Spdy session. CompletionCallbackImpl<SpdySession> read_callback_; CompletionCallbackImpl<SpdySession> write_callback_; @@ -435,6 +438,7 @@ class NET_API SpdySession : public base::RefCounted<SpdySession>, static bool use_ssl_; static bool use_flow_control_; static size_t max_concurrent_stream_limit_; + static bool verify_domain_authentication_; }; class NetLogSpdySynParameter : public NetLog::EventParameters { diff --git a/net/spdy/spdy_session_pool.cc b/net/spdy/spdy_session_pool.cc index da2d331..adaeabf 100644 --- a/net/spdy/spdy_session_pool.cc +++ b/net/spdy/spdy_session_pool.cc @@ -61,6 +61,19 @@ SpdySessionPool::~SpdySessionPool() { scoped_refptr<SpdySession> SpdySessionPool::Get( const HostPortProxyPair& host_port_proxy_pair, const BoundNetLog& net_log) { + return GetInternal(host_port_proxy_pair, net_log, false); +} + +scoped_refptr<SpdySession> SpdySessionPool::GetIfExists( + const HostPortProxyPair& host_port_proxy_pair, + const BoundNetLog& net_log) { + return GetInternal(host_port_proxy_pair, net_log, true); +} + +scoped_refptr<SpdySession> SpdySessionPool::GetInternal( + const HostPortProxyPair& host_port_proxy_pair, + const BoundNetLog& net_log, + bool only_use_existing_sessions) { scoped_refptr<SpdySession> spdy_session; SpdySessionList* list = GetSessionList(host_port_proxy_pair); if (!list) { @@ -75,6 +88,8 @@ scoped_refptr<SpdySession> SpdySessionPool::Get( make_scoped_refptr(new NetLogSourceParameter( "session", spdy_session->net_log().source()))); return spdy_session; + } else if (only_use_existing_sessions) { + return NULL; } list = AddSessionList(host_port_proxy_pair); } @@ -92,6 +107,8 @@ scoped_refptr<SpdySession> SpdySessionPool::Get( return spdy_session; } + DCHECK(!only_use_existing_sessions); + spdy_session = new SpdySession(host_port_proxy_pair, this, &spdy_settings_, net_log.net_log()); UMA_HISTOGRAM_ENUMERATION("Net.SpdySessionGet", diff --git a/net/spdy/spdy_session_pool.h b/net/spdy/spdy_session_pool.h index 0fd1a17..4ef8c47 100644 --- a/net/spdy/spdy_session_pool.h +++ b/net/spdy/spdy_session_pool.h @@ -50,6 +50,11 @@ class NET_API SpdySessionPool const HostPortProxyPair& host_port_proxy_pair, const BoundNetLog& net_log); + // Only returns a SpdySession if it already exists. + scoped_refptr<SpdySession> GetIfExists( + const HostPortProxyPair& host_port_proxy_pair, + const BoundNetLog& net_log); + // Set the maximum concurrent sessions per domain. static void set_max_sessions_per_domain(int max) { if (max >= 1) @@ -75,7 +80,10 @@ class NET_API SpdySessionPool bool is_secure); // TODO(willchan): Consider renaming to HasReusableSession, since perhaps we - // should be creating a new session. + // should be creating a new session. WARNING: Because of IP connection pooling + // using the HostCache, if HasSession() returns true at one point, it does not + // imply the SpdySessionPool will still have a matching session in the near + // future, since the HostCache's entry may have expired. bool HasSession(const HostPortProxyPair& host_port_proxy_pair) const; // Close all SpdySessions, including any new ones created in the process of @@ -128,6 +136,11 @@ class NET_API SpdySessionPool typedef std::map<HostPortProxyPair, SpdySessionList*> SpdySessionsMap; typedef std::map<IPEndPoint, HostPortProxyPair> SpdyAliasMap; + + scoped_refptr<SpdySession> GetInternal( + const HostPortProxyPair& host_port_proxy_pair, + const BoundNetLog& net_log, + bool only_use_existing_sessions); scoped_refptr<SpdySession> GetExistingSession( SpdySessionList* list, const BoundNetLog& net_log) const; |