diff options
-rw-r--r-- | net/http/http_stream_factory_impl_job.cc | 16 | ||||
-rw-r--r-- | net/quic/quic_network_transaction_unittest.cc | 36 | ||||
-rw-r--r-- | net/quic/quic_stream_factory.cc | 63 | ||||
-rw-r--r-- | net/quic/quic_stream_factory.h | 6 | ||||
-rw-r--r-- | net/quic/quic_stream_factory_test.cc | 4 | ||||
-rw-r--r-- | net/socket/socket_test_util.cc | 2 |
6 files changed, 89 insertions, 38 deletions
diff --git a/net/http/http_stream_factory_impl_job.cc b/net/http/http_stream_factory_impl_job.cc index ed8f1da..8cd2de0 100644 --- a/net/http/http_stream_factory_impl_job.cc +++ b/net/http/http_stream_factory_impl_job.cc @@ -977,6 +977,14 @@ int HttpStreamFactoryImpl::Job::DoInitConnectionComplete(int result) { return result; } + if (using_quic_) { + if (result < 0) + return result; + stream_ = quic_request_.ReleaseStream(); + next_state_ = STATE_NONE; + return OK; + } + if (!ssl_started && result < 0 && original_url_.get()) { HistogramBrokenAlternateProtocolLocation( BROKEN_ALTERNATE_PROTOCOL_LOCATION_HTTP_STREAM_FACTORY_IMPL_JOB); @@ -986,14 +994,6 @@ int HttpStreamFactoryImpl::Job::DoInitConnectionComplete(int result) { return result; } - if (using_quic_) { - if (result < 0) - return result; - stream_ = quic_request_.ReleaseStream(); - next_state_ = STATE_NONE; - return OK; - } - if (result < 0 && !ssl_started) return ReconsiderProxyAfterError(result); establishing_tunnel_ = false; diff --git a/net/quic/quic_network_transaction_unittest.cc b/net/quic/quic_network_transaction_unittest.cc index 8a22aa5..5aa0368 100644 --- a/net/quic/quic_network_transaction_unittest.cc +++ b/net/quic/quic_network_transaction_unittest.cc @@ -281,6 +281,15 @@ class QuicNetworkTransactionTest EXPECT_EQ(ALTERNATE_PROTOCOL_BROKEN, alternate.protocol); } + void ExpectQuicAlternateProtocolMapping() { + ASSERT_TRUE(session_->http_server_properties()->HasAlternateProtocol( + HostPortPair::FromURL(request_.url))); + const PortAlternateProtocolPair alternate = + session_->http_server_properties()->GetAlternateProtocol( + HostPortPair::FromURL(request_.url)); + EXPECT_EQ(QUIC, alternate.protocol); + } + void AddHangingNonAlternateProtocolSocketData() { MockConnect hanging_connect(SYNCHRONOUS, ERR_IO_PENDING); hanging_data_.set_connect_data(hanging_connect); @@ -826,6 +835,33 @@ TEST_P(QuicNetworkTransactionTest, FailedZeroRttBrokenAlternateProtocol) { EXPECT_TRUE(quic_data.at_write_eof()); } +TEST_P(QuicNetworkTransactionTest, NoBrokenAlternateProtocolOnConnectFailure) { + HttpStreamFactory::EnableNpnSpdy3(); // Enables QUIC too. + + // Alternate-protocol job will fail before creating a QUIC session. + StaticSocketDataProvider quic_data(NULL, 0, NULL, 0); + quic_data.set_connect_data(MockConnect(SYNCHRONOUS, + ERR_INTERNET_DISCONNECTED)); + socket_factory_.AddSocketDataProvider(&quic_data); + + // Main job which will succeed even though the alternate job fails. + MockRead http_reads[] = { + MockRead("HTTP/1.1 200 OK\r\n\r\n"), + MockRead("hello from http"), + MockRead(SYNCHRONOUS, ERR_TEST_PEER_CLOSE_AFTER_NEXT_MOCK_READ), + MockRead(ASYNC, OK) + }; + + StaticSocketDataProvider http_data(http_reads, arraysize(http_reads), + NULL, 0); + socket_factory_.AddSocketDataProvider(&http_data); + + CreateSession(); + AddQuicAlternateProtocolMapping(MockCryptoClientStream::COLD_START); + SendRequestAndExpectHttpResponse("hello from http"); + ExpectQuicAlternateProtocolMapping(); +} + TEST_P(QuicNetworkTransactionTest, ConnectionCloseDuringConnect) { HttpStreamFactory::EnableNpnSpdy3(); // Enables QUIC too. diff --git a/net/quic/quic_stream_factory.cc b/net/quic/quic_stream_factory.cc index 5008ee8..642914d 100644 --- a/net/quic/quic_stream_factory.cc +++ b/net/quic/quic_stream_factory.cc @@ -430,7 +430,10 @@ QuicStreamFactory::QuicStreamFactory( QuicStreamFactory::~QuicStreamFactory() { CloseAllSessions(ERR_ABORTED); - STLDeleteElements(&all_sessions_); + while (!all_sessions_.empty()) { + delete all_sessions_.begin()->first; + all_sessions_.erase(all_sessions_.begin()); + } STLDeleteValues(&active_jobs_); } @@ -556,7 +559,6 @@ void QuicStreamFactory::OnIdleSession(QuicClientSession* session) { } void QuicStreamFactory::OnSessionGoingAway(QuicClientSession* session) { - const QuicConnectionStats& stats = session->connection()->GetStats(); const AliasSet& aliases = session_aliases_[session]; for (AliasSet::const_iterator it = aliases.begin(); it != aliases.end(); ++it) { @@ -569,28 +571,9 @@ void QuicStreamFactory::OnSessionGoingAway(QuicClientSession* session) { } active_sessions_.erase(*it); - if (!http_server_properties_) - continue; - - if (!session->IsCryptoHandshakeConfirmed()) { - // TODO(rch): In the special case where the session has received no - // packets from the peer, we should consider blacklisting this - // differently so that we still race TCP but we don't consider the - // session connected until the handshake has been confirmed. - HistogramBrokenAlternateProtocolLocation( - BROKEN_ALTERNATE_PROTOCOL_LOCATION_QUIC_STREAM_FACTORY); - http_server_properties_->SetBrokenAlternateProtocol(it->host_port_pair()); - UMA_HISTOGRAM_COUNTS("Net.QuicHandshakeNotConfirmedNumPacketsReceived", - stats.packets_received); - continue; - } - - HttpServerProperties::NetworkStats network_stats; - network_stats.srtt = base::TimeDelta::FromMicroseconds(stats.srtt_us); - network_stats.bandwidth_estimate = stats.estimated_bandwidth; - http_server_properties_->SetServerNetworkStats(it->host_port_pair(), - network_stats); + ProcessGoingAwaySession(session, *it); } + ProcessGoingAwaySession(session, all_sessions_[session]); if (!aliases.empty()) { const IpAliasKey ip_alias_key(session->connection()->peer_address(), aliases.begin()->is_https()); @@ -605,8 +588,8 @@ void QuicStreamFactory::OnSessionGoingAway(QuicClientSession* session) { void QuicStreamFactory::OnSessionClosed(QuicClientSession* session) { DCHECK_EQ(0u, session->GetNumOpenStreams()); OnSessionGoingAway(session); - all_sessions_.erase(session); delete session; + all_sessions_.erase(session); } void QuicStreamFactory::CancelRequest(QuicStreamRequest* request) { @@ -624,7 +607,7 @@ void QuicStreamFactory::CloseAllSessions(int error) { } while (!all_sessions_.empty()) { size_t initial_size = all_sessions_.size(); - (*all_sessions_.begin())->CloseSessionOnError(error); + all_sessions_.begin()->first->CloseSessionOnError(error); DCHECK_NE(initial_size, all_sessions_.size()); } DCHECK(all_sessions_.empty()); @@ -773,7 +756,7 @@ int QuicStreamFactory::CreateSession( connection, socket.Pass(), writer.Pass(), this, quic_crypto_client_stream_factory_, server_info.Pass(), server_id, config, &crypto_config_, net_log.net_log()); - all_sessions_.insert(*session); // owning pointer + all_sessions_[*session] = server_id; // owning pointer return OK; } @@ -817,4 +800,32 @@ void QuicStreamFactory::InitializeCachedStateInCryptoConfig( } } +void QuicStreamFactory::ProcessGoingAwaySession( + QuicClientSession* session, + const QuicServerId& server_id) { + if (!http_server_properties_) + return; + + const QuicConnectionStats& stats = session->connection()->GetStats(); + if (!session->IsCryptoHandshakeConfirmed()) { + // TODO(rch): In the special case where the session has received no + // packets from the peer, we should consider blacklisting this + // differently so that we still race TCP but we don't consider the + // session connected until the handshake has been confirmed. + HistogramBrokenAlternateProtocolLocation( + BROKEN_ALTERNATE_PROTOCOL_LOCATION_QUIC_STREAM_FACTORY); + http_server_properties_->SetBrokenAlternateProtocol( + server_id.host_port_pair()); + UMA_HISTOGRAM_COUNTS("Net.QuicHandshakeNotConfirmedNumPacketsReceived", + stats.packets_received); + return; + } + + HttpServerProperties::NetworkStats network_stats; + network_stats.srtt = base::TimeDelta::FromMicroseconds(stats.srtt_us); + network_stats.bandwidth_estimate = stats.estimated_bandwidth; + http_server_properties_->SetServerNetworkStats(server_id.host_port_pair(), + network_stats); +} + } // namespace net diff --git a/net/quic/quic_stream_factory.h b/net/quic/quic_stream_factory.h index 079b652..b7936c3 100644 --- a/net/quic/quic_stream_factory.h +++ b/net/quic/quic_stream_factory.h @@ -186,6 +186,7 @@ class NET_EXPORT_PRIVATE QuicStreamFactory }; typedef std::map<QuicServerId, QuicClientSession*> SessionMap; + typedef std::map<QuicClientSession*, QuicServerId> SessionIdMap; typedef std::set<QuicServerId> AliasSet; typedef std::map<QuicClientSession*, AliasSet> SessionAliasMap; typedef std::set<QuicClientSession*> SessionSet; @@ -220,6 +221,9 @@ class NET_EXPORT_PRIVATE QuicStreamFactory const QuicServerId& server_id, const scoped_ptr<QuicServerInfo>& server_info); + void ProcessGoingAwaySession(QuicClientSession* session, + const QuicServerId& server_id); + bool require_confirmation_; HostResolver* host_resolver_; ClientSocketFactory* client_socket_factory_; @@ -235,7 +239,7 @@ class NET_EXPORT_PRIVATE QuicStreamFactory scoped_ptr<QuicConnectionHelper> helper_; // Contains owning pointers to all sessions that currently exist. - SessionSet all_sessions_; + SessionIdMap all_sessions_; // Contains non-owning pointers to currently active session // (not going away session, once they're implemented). SessionMap active_sessions_; diff --git a/net/quic/quic_stream_factory_test.cc b/net/quic/quic_stream_factory_test.cc index e5b3a3e..27903e9 100644 --- a/net/quic/quic_stream_factory_test.cc +++ b/net/quic/quic_stream_factory_test.cc @@ -72,10 +72,10 @@ class QuicStreamFactoryPeer { static bool IsLiveSession(QuicStreamFactory* factory, QuicClientSession* session) { - for (QuicStreamFactory::SessionSet::iterator it = + for (QuicStreamFactory::SessionIdMap::iterator it = factory->all_sessions_.begin(); it != factory->all_sessions_.end(); ++it) { - if (*it == session) + if (it->first == session) return true; } return false; diff --git a/net/socket/socket_test_util.cc b/net/socket/socket_test_util.cc index 3e7554e..be81e9a 100644 --- a/net/socket/socket_test_util.cc +++ b/net/socket/socket_test_util.cc @@ -1540,7 +1540,7 @@ const BoundNetLog& MockUDPClientSocket::NetLog() const { int MockUDPClientSocket::Connect(const IPEndPoint& address) { connected_ = true; peer_addr_ = address; - return OK; + return data_->connect_data().result; } void MockUDPClientSocket::OnReadComplete(const MockRead& data) { |