From c4744cdbbe22df6848e354dbfa9dbb1dc36e76d8 Mon Sep 17 00:00:00 2001 From: "willchan@chromium.org" Date: Wed, 23 Feb 2011 00:08:48 +0000 Subject: Revert 75668 for breaking ChromeOS build - Refactor HttpStreamFactory. Rename StreamFactory and StreamRequest to HttpStreamFactory and HttpStreamRequest. Rename HttpStreamFactory to HttpStreamFactoryImpl. Create HttpStreamFactoryImpl::Request (inherits from HttpStreamRequest) and HttpStreamFactoryImpl::Job (most of the old HttpStreamRequest code, other than the interface, moved here). Currently there is still a strong binding within HttpStreamFactoryImpl between requests and jobs. This will be removed in a future changelist. Note that due to the preparation for late binding, information like HttpRequestInfo and SSLConfig and ProxyInfo are just copied. It's possible we can consider refcounting them to reduce copies, but I think it's not worth the effort / ugliness. I also did some minor cleanups like moving SpdySettingsStorage into SpdySessionPool and some CloseIdleConnections() cleanup. BUG=54371,42669 TEST=unit tests Review URL: http://codereview.chromium.org/6543004 TBR=willchan@chromium.org git-svn-id: svn://svn.chromium.org/chrome/trunk/src@75670 0039d316-1c4b-4281-b951-d872f2087c98 --- net/spdy/spdy_http_stream_unittest.cc | 3 ++- net/spdy/spdy_network_transaction_unittest.cc | 19 +++++++++---------- net/spdy/spdy_proxy_client_socket_unittest.cc | 17 +++++++++-------- net/spdy/spdy_session_pool.cc | 6 ++++-- net/spdy/spdy_session_pool.h | 9 +++------ net/spdy/spdy_session_unittest.cc | 27 +++++++++++++++------------ net/spdy/spdy_stream_unittest.cc | 4 +++- 7 files changed, 45 insertions(+), 40 deletions(-) (limited to 'net/spdy') diff --git a/net/spdy/spdy_http_stream_unittest.cc b/net/spdy/spdy_http_stream_unittest.cc index 1688871..df979ec 100644 --- a/net/spdy/spdy_http_stream_unittest.cc +++ b/net/spdy/spdy_http_stream_unittest.cc @@ -29,7 +29,8 @@ class SpdyHttpStreamTest : public testing::Test { data_ = new OrderedSocketData(reads, reads_count, writes, writes_count); session_deps_.socket_factory->AddSocketDataProvider(data_.get()); http_session_ = SpdySessionDependencies::SpdyCreateSession(&session_deps_); - session_ = http_session_->spdy_session_pool()->Get(pair, BoundNetLog()); + session_ = http_session_->spdy_session_pool()-> + Get(pair, http_session_->mutable_spdy_settings(), BoundNetLog()); tcp_params_ = new TCPSocketParams(host_port_pair.host(), host_port_pair.port(), MEDIUM, GURL(), false); diff --git a/net/spdy/spdy_network_transaction_unittest.cc b/net/spdy/spdy_network_transaction_unittest.cc index 019cc4c..402f2f7 100644 --- a/net/spdy/spdy_network_transaction_unittest.cc +++ b/net/spdy/spdy_network_transaction_unittest.cc @@ -393,7 +393,8 @@ class SpdyNetworkTransactionTest const scoped_refptr& session = helper.session(); SpdySessionPool* pool(session->spdy_session_pool()); EXPECT_TRUE(pool->HasSession(pair)); - scoped_refptr spdy_session(pool->Get(pair, log)); + scoped_refptr spdy_session( + pool->Get(pair, session->mutable_spdy_settings(), log)); ASSERT_TRUE(spdy_session.get() != NULL); EXPECT_EQ(0u, spdy_session->num_active_streams()); EXPECT_EQ(0u, spdy_session->num_unclaimed_pushed_streams()); @@ -4015,8 +4016,7 @@ TEST_P(SpdyNetworkTransactionTest, SettingsSaved) { // Verify that no settings exist initially. HostPortPair host_port_pair("www.google.com", helper.port()); - SpdySessionPool* spdy_session_pool = helper.session()->spdy_session_pool(); - EXPECT_TRUE(spdy_session_pool->spdy_settings().Get(host_port_pair).empty()); + EXPECT_TRUE(helper.session()->spdy_settings().Get(host_port_pair).empty()); // Construct the request. scoped_ptr req(ConstructSpdyGet(NULL, 0, false, 1, LOWEST)); @@ -4078,7 +4078,7 @@ TEST_P(SpdyNetworkTransactionTest, SettingsSaved) { { // Verify we had two persisted settings. spdy::SpdySettings saved_settings = - spdy_session_pool->spdy_settings().Get(host_port_pair); + helper.session()->spdy_settings().Get(host_port_pair); ASSERT_EQ(2u, saved_settings.size()); // Verify the first persisted setting. @@ -4124,8 +4124,7 @@ TEST_P(SpdyNetworkTransactionTest, SettingsPlayback) { // Verify that no settings exist initially. HostPortPair host_port_pair("www.google.com", helper.port()); - SpdySessionPool* spdy_session_pool = helper.session()->spdy_session_pool(); - EXPECT_TRUE(spdy_session_pool->spdy_settings().Get(host_port_pair).empty()); + EXPECT_TRUE(helper.session()->spdy_settings().Get(host_port_pair).empty()); unsigned int kSampleId1 = 0x1; unsigned int kSampleValue1 = 0x0a0a0a0a; @@ -4144,14 +4143,14 @@ TEST_P(SpdyNetworkTransactionTest, SettingsPlayback) { setting.set_id(kSampleId2); settings.push_back(std::make_pair(setting, kSampleValue2)); - spdy_session_pool->mutable_spdy_settings()->Set(host_port_pair, settings); + helper.session()->mutable_spdy_settings()->Set(host_port_pair, settings); } - EXPECT_EQ(2u, spdy_session_pool->spdy_settings().Get(host_port_pair).size()); + EXPECT_EQ(2u, helper.session()->spdy_settings().Get(host_port_pair).size()); // Construct the SETTINGS frame. const spdy::SpdySettings& settings = - spdy_session_pool->spdy_settings().Get(host_port_pair); + helper.session()->spdy_settings().Get(host_port_pair); scoped_ptr settings_frame(ConstructSpdySettings(settings)); // Construct the request. @@ -4191,7 +4190,7 @@ TEST_P(SpdyNetworkTransactionTest, SettingsPlayback) { { // Verify we had two persisted settings. spdy::SpdySettings saved_settings = - spdy_session_pool->spdy_settings().Get(host_port_pair); + helper.session()->spdy_settings().Get(host_port_pair); ASSERT_EQ(2u, saved_settings.size()); // Verify the first persisted setting. diff --git a/net/spdy/spdy_proxy_client_socket_unittest.cc b/net/spdy/spdy_proxy_client_socket_unittest.cc index 9023ca9..e4483bd 100644 --- a/net/spdy/spdy_proxy_client_socket_unittest.cc +++ b/net/spdy/spdy_proxy_client_socket_unittest.cc @@ -85,13 +85,13 @@ class SpdyProxyClientSocketTest : public PlatformTest { void AddAuthToCache() { const string16 kFoo(ASCIIToUTF16("foo")); const string16 kBar(ASCIIToUTF16("bar")); - session_->http_auth_cache()->Add(GURL(kProxyUrl), - "MyRealm1", - HttpAuth::AUTH_SCHEME_BASIC, - "Basic realm=MyRealm1", - kFoo, - kBar, - "/"); + session_->auth_cache()->Add(GURL(kProxyUrl), + "MyRealm1", + HttpAuth::AUTH_SCHEME_BASIC, + "Basic realm=MyRealm1", + kFoo, + kBar, + "/"); } void Run(int steps) { @@ -175,6 +175,7 @@ void SpdyProxyClientSocketTest::Initialize(MockRead* reads, // Creates a new spdy session spdy_session_ = session_->spdy_session_pool()->Get(endpoint_host_port_proxy_pair_, + session_->mutable_spdy_settings(), BoundNetLog()); // Perform the TCP connect @@ -195,7 +196,7 @@ void SpdyProxyClientSocketTest::Initialize(MockRead* reads, sock_.reset( new SpdyProxyClientSocket(spdy_stream_, user_agent_, endpoint_host_port_pair_, url_, - proxy_host_port_, session_->http_auth_cache(), + proxy_host_port_, session_->auth_cache(), session_->http_auth_handler_factory())); } diff --git a/net/spdy/spdy_session_pool.cc b/net/spdy/spdy_session_pool.cc index b32495e..f4ec575 100644 --- a/net/spdy/spdy_session_pool.cc +++ b/net/spdy/spdy_session_pool.cc @@ -34,6 +34,7 @@ SpdySessionPool::~SpdySessionPool() { scoped_refptr SpdySessionPool::Get( const HostPortProxyPair& host_port_proxy_pair, + SpdySettingsStorage* spdy_settings, const BoundNetLog& net_log) { scoped_refptr spdy_session; SpdySessionList* list = GetSessionList(host_port_proxy_pair); @@ -52,7 +53,7 @@ scoped_refptr SpdySessionPool::Get( DCHECK(list); if (!spdy_session) { - spdy_session = new SpdySession(host_port_proxy_pair, this, &spdy_settings_, + spdy_session = new SpdySession(host_port_proxy_pair, this, spdy_settings, net_log.net_log()); net_log.AddEvent( NetLog::TYPE_SPDY_SESSION_POOL_CREATED_NEW_SESSION, @@ -68,13 +69,14 @@ scoped_refptr SpdySessionPool::Get( net::Error SpdySessionPool::GetSpdySessionFromSocket( const HostPortProxyPair& host_port_proxy_pair, + SpdySettingsStorage* spdy_settings, ClientSocketHandle* connection, const BoundNetLog& net_log, int certificate_error_code, scoped_refptr* spdy_session, bool is_secure) { // Create the SPDY session and add it to the pool. - *spdy_session = new SpdySession(host_port_proxy_pair, this, &spdy_settings_, + *spdy_session = new SpdySession(host_port_proxy_pair, this, spdy_settings, net_log.net_log()); SpdySessionList* list = GetSessionList(host_port_proxy_pair); if (!list) diff --git a/net/spdy/spdy_session_pool.h b/net/spdy/spdy_session_pool.h index 10da747..7f30b20 100644 --- a/net/spdy/spdy_session_pool.h +++ b/net/spdy/spdy_session_pool.h @@ -20,7 +20,6 @@ #include "net/base/ssl_config_service.h" #include "net/proxy/proxy_config.h" #include "net/proxy/proxy_server.h" -#include "net/spdy/spdy_settings_storage.h" namespace net { // Sessions are uniquely identified by their HostPortPair and the proxy server @@ -31,6 +30,7 @@ class BoundNetLog; class ClientSocketHandle; class HttpNetworkSession; class SpdySession; +class SpdySettingsStorage; // This is a very simple pool for open SpdySessions. // TODO(mbelshe): Make this production ready. @@ -45,6 +45,7 @@ class SpdySessionPool // use. scoped_refptr Get( const HostPortProxyPair& host_port_proxy_pair, + SpdySettingsStorage* spdy_settings, const BoundNetLog& net_log); // Set the maximum concurrent sessions per domain. @@ -65,6 +66,7 @@ class SpdySessionPool // Returns an error on failure, and |spdy_session| will be NULL. net::Error GetSpdySessionFromSocket( const HostPortProxyPair& host_port_proxy_pair, + SpdySettingsStorage* spdy_settings, ClientSocketHandle* connection, const BoundNetLog& net_log, int certificate_error_code, @@ -90,9 +92,6 @@ class SpdySessionPool // responsible for deleting the returned value. Value* SpdySessionPoolInfoToValue() const; - SpdySettingsStorage* mutable_spdy_settings() { return &spdy_settings_; } - const SpdySettingsStorage& spdy_settings() const { return spdy_settings_; } - // NetworkChangeNotifier::Observer methods: // We flush all idle sessions and release references to the active ones so @@ -125,8 +124,6 @@ class SpdySessionPool const HostPortProxyPair& host_port_proxy_pair) const; void RemoveSessionList(const HostPortProxyPair& host_port_proxy_pair); - SpdySettingsStorage spdy_settings_; - // This is our weak session pool - one session per domain. SpdySessionsMap sessions_; diff --git a/net/spdy/spdy_session_unittest.cc b/net/spdy/spdy_session_unittest.cc index 0f3b601..44848e3 100644 --- a/net/spdy/spdy_session_unittest.cc +++ b/net/spdy/spdy_session_unittest.cc @@ -88,7 +88,8 @@ TEST_F(SpdySessionTest, GoAway) { SpdySessionPool* spdy_session_pool(http_session->spdy_session_pool()); EXPECT_FALSE(spdy_session_pool->HasSession(pair)); scoped_refptr session = - spdy_session_pool->Get(pair, BoundNetLog()); + spdy_session_pool->Get(pair, http_session->mutable_spdy_settings(), + BoundNetLog()); EXPECT_TRUE(spdy_session_pool->HasSession(pair)); scoped_refptr tcp_params( @@ -106,7 +107,8 @@ TEST_F(SpdySessionTest, GoAway) { EXPECT_FALSE(spdy_session_pool->HasSession(pair)); scoped_refptr session2 = - spdy_session_pool->Get(pair, BoundNetLog()); + spdy_session_pool->Get(pair, http_session->mutable_spdy_settings(), + BoundNetLog()); // Delete the first session. session = NULL; @@ -185,17 +187,18 @@ TEST_F(SpdySessionTest, OnSettings) { HostPortProxyPair pair(test_host_port_pair, ProxyServer::Direct()); // Initialize the SpdySettingsStorage with 1 max concurrent streams. - SpdySessionPool* spdy_session_pool(http_session->spdy_session_pool()); spdy::SpdySettings old_settings; id.set_flags(spdy::SETTINGS_FLAG_PLEASE_PERSIST); old_settings.push_back(spdy::SpdySetting(id, 1)); - spdy_session_pool->mutable_spdy_settings()->Set( + http_session->mutable_spdy_settings()->Set( test_host_port_pair, old_settings); // Create a session. + SpdySessionPool* spdy_session_pool(http_session->spdy_session_pool()); EXPECT_FALSE(spdy_session_pool->HasSession(pair)); scoped_refptr session = - spdy_session_pool->Get(pair, BoundNetLog()); + spdy_session_pool->Get(pair, http_session->mutable_spdy_settings(), + BoundNetLog()); ASSERT_TRUE(spdy_session_pool->HasSession(pair)); scoped_refptr tcp_params( @@ -264,19 +267,19 @@ TEST_F(SpdySessionTest, CancelPendingCreateStream) { HostPortProxyPair pair(test_host_port_pair, ProxyServer::Direct()); // Initialize the SpdySettingsStorage with 1 max concurrent streams. - SpdySessionPool* spdy_session_pool(http_session->spdy_session_pool()); spdy::SpdySettings settings; spdy::SettingsFlagsAndId id(spdy::SETTINGS_MAX_CONCURRENT_STREAMS); id.set_id(spdy::SETTINGS_MAX_CONCURRENT_STREAMS); id.set_flags(spdy::SETTINGS_FLAG_PLEASE_PERSIST); settings.push_back(spdy::SpdySetting(id, 1)); - spdy_session_pool->mutable_spdy_settings()->Set( - test_host_port_pair, settings); + http_session->mutable_spdy_settings()->Set(test_host_port_pair, settings); // Create a session. + SpdySessionPool* spdy_session_pool(http_session->spdy_session_pool()); EXPECT_FALSE(spdy_session_pool->HasSession(pair)); scoped_refptr session = - spdy_session_pool->Get(pair, BoundNetLog()); + spdy_session_pool->Get(pair, http_session->mutable_spdy_settings(), + BoundNetLog()); ASSERT_TRUE(spdy_session_pool->HasSession(pair)); scoped_refptr tcp_params( @@ -366,12 +369,12 @@ TEST_F(SpdySessionTest, SendSettingsOnNewSession) { id.set_flags(spdy::SETTINGS_FLAG_PLEASE_PERSIST); settings.clear(); settings.push_back(spdy::SpdySetting(id, kBogusSettingValue)); + http_session->mutable_spdy_settings()->Set(test_host_port_pair, settings); SpdySessionPool* spdy_session_pool(http_session->spdy_session_pool()); - spdy_session_pool->mutable_spdy_settings()->Set( - test_host_port_pair, settings); EXPECT_FALSE(spdy_session_pool->HasSession(pair)); scoped_refptr session = - spdy_session_pool->Get(pair, BoundNetLog()); + spdy_session_pool->Get(pair, http_session->mutable_spdy_settings(), + BoundNetLog()); EXPECT_TRUE(spdy_session_pool->HasSession(pair)); scoped_refptr tcp_params( diff --git a/net/spdy/spdy_stream_unittest.cc b/net/spdy/spdy_stream_unittest.cc index 532c79f..9c7b934 100644 --- a/net/spdy/spdy_stream_unittest.cc +++ b/net/spdy/spdy_stream_unittest.cc @@ -117,7 +117,9 @@ class SpdyStreamTest : public testing::Test { HostPortPair host_port_pair("www.google.com", 80); HostPortProxyPair pair(host_port_pair, ProxyServer::Direct()); scoped_refptr session( - session_->spdy_session_pool()->Get(pair, BoundNetLog())); + session_->spdy_session_pool()->Get(pair, + session_->mutable_spdy_settings(), + BoundNetLog())); return session; } -- cgit v1.1