 Chromium Code Reviews
 Chromium Code Reviews Issue 1540463003:
  Change the interface of GetAlternativeServicesFor, always return the best Alt-Svc entry.  (Closed) 
  Base URL: https://chromium.googlesource.com/chromium/src.git@master
    
  
    Issue 1540463003:
  Change the interface of GetAlternativeServicesFor, always return the best Alt-Svc entry.  (Closed) 
  Base URL: https://chromium.googlesource.com/chromium/src.git@master| Index: net/quic/quic_network_transaction_unittest.cc | 
| diff --git a/net/quic/quic_network_transaction_unittest.cc b/net/quic/quic_network_transaction_unittest.cc | 
| index fa5503a031989af238f80aea12e56f482e5ec4d2..af447801219dd4eb988df8b3c7c664d642c8f255 100644 | 
| --- a/net/quic/quic_network_transaction_unittest.cc | 
| +++ b/net/quic/quic_network_transaction_unittest.cc | 
| @@ -234,7 +234,17 @@ class QuicNetworkTransactionTest | 
| scoped_ptr<QuicEncryptedPacket> ConstructAckPacket( | 
| QuicPacketNumber largest_received, | 
| QuicPacketNumber least_unacked) { | 
| - return maker_.MakeAckPacket(2, largest_received, least_unacked, true); | 
| + return maker_.MakeAckPacket(2, largest_received, least_unacked, | 
| + least_unacked, true); | 
| + } | 
| + | 
| + scoped_ptr<QuicEncryptedPacket> ConstructAckAndConnectionClosePacket( | 
| + QuicPacketNumber packet_number, | 
| + QuicPacketNumber largest_received, | 
| + QuicPacketNumber ack_least_unacked, | 
| + QuicPacketNumber stop_least_unacked) { | 
| + return maker_.MakeAckPacket(packet_number, largest_received, | 
| + ack_least_unacked, stop_least_unacked, true); | 
| } | 
| scoped_ptr<QuicEncryptedPacket> ConstructAckAndConnectionClosePacket( | 
| @@ -283,12 +293,23 @@ class QuicNetworkTransactionTest | 
| QuicStreamId stream_id, | 
| bool should_include_version, | 
| bool fin, | 
| - const SpdyHeaderBlock& headers) { | 
| + const SpdyHeaderBlock& headers, | 
| + QuicStreamOffset offset) { | 
| SpdyPriority priority = | 
| ConvertRequestPriorityToQuicPriority(DEFAULT_PRIORITY); | 
| return maker_.MakeRequestHeadersPacket(packet_number, stream_id, | 
| should_include_version, fin, | 
| - priority, headers); | 
| + priority, headers, offset); | 
| + } | 
| + | 
| + scoped_ptr<QuicEncryptedPacket> ConstructRequestHeadersPacket( | 
| + QuicPacketNumber packet_number, | 
| + QuicStreamId stream_id, | 
| + bool should_include_version, | 
| + bool fin, | 
| + const SpdyHeaderBlock& headers) { | 
| + return ConstructRequestHeadersPacket( | 
| + packet_number, stream_id, should_include_version, fin, headers, 0); | 
| } | 
| scoped_ptr<QuicEncryptedPacket> ConstructResponseHeadersPacket( | 
| @@ -297,8 +318,19 @@ class QuicNetworkTransactionTest | 
| bool should_include_version, | 
| bool fin, | 
| const SpdyHeaderBlock& headers) { | 
| + return ConstructResponseHeadersPacket( | 
| + packet_number, stream_id, should_include_version, fin, headers, 0); | 
| + } | 
| + | 
| + scoped_ptr<QuicEncryptedPacket> ConstructResponseHeadersPacket( | 
| + QuicPacketNumber packet_number, | 
| + QuicStreamId stream_id, | 
| + bool should_include_version, | 
| + bool fin, | 
| + const SpdyHeaderBlock& headers, | 
| + QuicStreamOffset offset) { | 
| return maker_.MakeResponseHeadersPacket( | 
| - packet_number, stream_id, should_include_version, fin, headers); | 
| + packet_number, stream_id, should_include_version, fin, headers, offset); | 
| } | 
| void CreateSession() { | 
| @@ -771,14 +803,13 @@ TEST_P(QuicNetworkTransactionTest, | 
| } | 
| // When multiple alternative services are advertised, | 
| -// HttpStreamFactoryImpl::RequestStreamInternal() only passes the first one to | 
| -// Job. This is what the following test verifies. | 
| -// TODO(bnc): Update this test when multiple alternative services are handled | 
| -// properly. | 
| -TEST_P(QuicNetworkTransactionTest, UseFirstAlternativeServiceForQuic) { | 
| +// HttpStreamFactoryImpl::RequestStreamInternal() should select the alternative | 
| +// service which uses existing QUIC session if available. If no existing QUIC | 
| +// session can be used, use the first alternative service from the list. | 
| +TEST_P(QuicNetworkTransactionTest, UseBestAlternativeServiceForQuic) { | 
| MockRead http_reads[] = { | 
| MockRead("HTTP/1.1 200 OK\r\n"), | 
| - MockRead("Alt-Svc: quic=\":443\", quic=\":1234\"\r\n\r\n"), | 
| + MockRead("Alt-Svc: quic=\"foo.example.com:443\", quic=\":1234\"\r\n\r\n"), | 
| MockRead("hello world"), | 
| MockRead(SYNCHRONOUS, ERR_TEST_PEER_CLOSE_AFTER_NEXT_MOCK_READ), | 
| MockRead(ASYNC, OK)}; | 
| @@ -788,6 +819,9 @@ TEST_P(QuicNetworkTransactionTest, UseFirstAlternativeServiceForQuic) { | 
| socket_factory_.AddSocketDataProvider(&http_data); | 
| socket_factory_.AddSSLSocketDataProvider(&ssl_data_); | 
| + // First QUIC request data. | 
| + // Open a session to mail.example.org:443 using the first entry of the | 
| 
Ryan Hamilton
2015/12/30 21:51:41
The Alt-Svc: header seems to be foo.example.com no
 | 
| + // alternative service list. | 
| MockQuicData mock_quic_data; | 
| mock_quic_data.AddWrite( | 
| ConstructRequestHeadersPacket(1, kClientDataStreamId1, true, true, | 
| @@ -797,6 +831,18 @@ TEST_P(QuicNetworkTransactionTest, UseFirstAlternativeServiceForQuic) { | 
| mock_quic_data.AddRead( | 
| ConstructDataPacket(2, kClientDataStreamId1, false, true, 0, "hello!")); | 
| mock_quic_data.AddWrite(ConstructAckPacket(2, 1)); | 
| + | 
| + // Second QUIC request data. | 
| + // Connection pooling, using existing session, no need to include version | 
| + // as version negotiation has been completed. | 
| + mock_quic_data.AddWrite(ConstructRequestHeadersPacket( | 
| + 3, kClientDataStreamId2, false, true, | 
| + GetRequestHeaders("GET", "https", "/"), 31)); | 
| + mock_quic_data.AddRead(ConstructResponseHeadersPacket( | 
| + 3, kClientDataStreamId2, false, false, GetResponseHeaders("200 OK"), 31)); | 
| + mock_quic_data.AddRead( | 
| + ConstructDataPacket(4, kClientDataStreamId2, false, true, 0, "hello!")); | 
| + mock_quic_data.AddWrite(ConstructAckAndConnectionClosePacket(4, 4, 3, 1)); | 
| mock_quic_data.AddRead(ASYNC, ERR_IO_PENDING); // No more data to read | 
| mock_quic_data.AddRead(ASYNC, 0); // EOF | 
| @@ -806,6 +852,35 @@ TEST_P(QuicNetworkTransactionTest, UseFirstAlternativeServiceForQuic) { | 
| CreateSessionWithNextProtos(); | 
| SendRequestAndExpectHttpResponse("hello world"); | 
| + | 
| + SendRequestAndExpectQuicResponseOnPort("hello!", 443); | 
| + | 
| + session_->http_server_properties()->ClearAlternativeServices( | 
| + HostPortPair::FromURL(request_.url)); | 
| + | 
| + // Manually reset the alternative service map where an existing QUIC session | 
| + // is in the second entry of the list. | 
| + HostPortPair alternative1("mail.example.com", 1234); | 
| + HostPortPair alternative2("foo.example.com", 443); | 
| + HostPortPair alternative3("bar.example.com", 443); | 
| + | 
| + AlternativeServiceInfoVector alternative_service_info_vector; | 
| + base::Time expiration = base::Time::Now() + base::TimeDelta::FromDays(1); | 
| + AlternativeService alternative_service1(QUIC, alternative1); | 
| + AlternativeServiceInfo alternative_service_info1(alternative_service1, 1.0, | 
| + expiration); | 
| + alternative_service_info_vector.push_back(alternative_service_info1); | 
| + AlternativeService alternative_service2(QUIC, alternative2); | 
| + AlternativeServiceInfo alternative_service_info2(alternative_service2, 1.0, | 
| + expiration); | 
| + alternative_service_info_vector.push_back(alternative_service_info2); | 
| + AlternativeService alternative_service3(QUIC, alternative3); | 
| + AlternativeServiceInfo alternative_service_info3(alternative_service3, 1.0, | 
| + expiration); | 
| + alternative_service_info_vector.push_back(alternative_service_info3); | 
| + session_->http_server_properties()->SetAlternativeServices( | 
| + HostPortPair::FromURL(request_.url), alternative_service_info_vector); | 
| + | 
| SendRequestAndExpectQuicResponseOnPort("hello!", 443); | 
| } |