 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/http/http_stream_factory_impl.cc | 
| diff --git a/net/http/http_stream_factory_impl.cc b/net/http/http_stream_factory_impl.cc | 
| index 93b85df394c223f1ec0b7644111c55d1aee545c8..9430aa0c5cebc0a108df434b4d39e4717518f72d 100644 | 
| --- a/net/http/http_stream_factory_impl.cc | 
| +++ b/net/http/http_stream_factory_impl.cc | 
| @@ -90,23 +90,26 @@ HttpStreamRequest* HttpStreamFactoryImpl::RequestStreamInternal( | 
| delegate, | 
| websocket_handshake_stream_create_helper, | 
| net_log); | 
| + HostPortPair server = HostPortPair::FromURL(request_info.url); | 
| + GURL original_url = ApplyHostMappingRules(request_info.url, &server); | 
| 
Ryan Hamilton
2015/12/18 22:56:29
nit: origin_url not original_url
 
Zhongyi Shi
2015/12/19 01:23:02
Done.
 | 
| + | 
| Job* job = new Job(this, session_, request_info, priority, server_ssl_config, | 
| - proxy_ssl_config, net_log.net_log()); | 
| + proxy_ssl_config, server, original_url, net_log.net_log()); | 
| request->AttachJob(job); | 
| - const AlternativeServiceVector alternative_service_vector = | 
| - GetAlternativeServicesFor(request_info.url, delegate); | 
| + const AlternativeService alternative_service = | 
| + GetAlternativeServiceFor(request_info, delegate); | 
| - if (!alternative_service_vector.empty()) { | 
| - // TODO(bnc): Pass on multiple alternative services to Job. | 
| - const AlternativeService& alternative_service = | 
| - alternative_service_vector[0]; | 
| + if (alternative_service.protocol != UNINITIALIZED_ALTERNATE_PROTOCOL) { | 
| // Never share connection with other jobs for FTP requests. | 
| DCHECK(!request_info.url.SchemeIs("ftp")); | 
| + HostPortPair server = alternative_service.host_port_pair(); | 
| + GURL original_url = ApplyHostMappingRules(request_info.url, &server); | 
| Job* alternative_job = | 
| new Job(this, session_, request_info, priority, server_ssl_config, | 
| - proxy_ssl_config, alternative_service, net_log.net_log()); | 
| + proxy_ssl_config, server, original_url, alternative_service, | 
| + net_log.net_log()); | 
| request->AttachJob(alternative_job); | 
| job->WaitFor(alternative_job); | 
| @@ -129,12 +132,9 @@ void HttpStreamFactoryImpl::PreconnectStreams( | 
| const SSLConfig& server_ssl_config, | 
| const SSLConfig& proxy_ssl_config) { | 
| DCHECK(!for_websockets_); | 
| - AlternativeService alternative_service; | 
| - AlternativeServiceVector alternative_service_vector = | 
| - GetAlternativeServicesFor(request_info.url, nullptr); | 
| - if (!alternative_service_vector.empty()) { | 
| - // TODO(bnc): Pass on multiple alternative services to Job. | 
| - alternative_service = alternative_service_vector[0]; | 
| + AlternativeService alternative_service = | 
| + GetAlternativeServiceFor(request_info, nullptr); | 
| + if (alternative_service.protocol != UNINITIALIZED_ALTERNATE_PROTOCOL) { | 
| if (session_->params().quic_disable_preconnect_if_0rtt && | 
| alternative_service.protocol == QUIC && | 
| session_->quic_stream_factory()->ZeroRTTEnabledFor(QuicServerId( | 
| @@ -147,9 +147,11 @@ void HttpStreamFactoryImpl::PreconnectStreams( | 
| // priority currently makes sense for preconnects. The priority for | 
| // preconnects is currently ignored (see RequestSocketsForPool()), but could | 
| // be used at some point for proxy resolution or something. | 
| - Job* job = | 
| - new Job(this, session_, request_info, IDLE, server_ssl_config, | 
| - proxy_ssl_config, alternative_service, session_->net_log()); | 
| + HostPortPair server = alternative_service.host_port_pair(); | 
| + GURL original_url = ApplyHostMappingRules(request_info.url, &server); | 
| + Job* job = new Job(this, session_, request_info, IDLE, server_ssl_config, | 
| + proxy_ssl_config, server, original_url, | 
| + alternative_service, session_->net_log()); | 
| preconnect_job_set_.insert(job); | 
| job->Preconnect(num_streams); | 
| } | 
| @@ -158,11 +160,13 @@ const HostMappingRules* HttpStreamFactoryImpl::GetHostMappingRules() const { | 
| return session_->params().host_mapping_rules; | 
| } | 
| -AlternativeServiceVector HttpStreamFactoryImpl::GetAlternativeServicesFor( | 
| - const GURL& original_url, | 
| +AlternativeService HttpStreamFactoryImpl::GetAlternativeServiceFor( | 
| + const HttpRequestInfo& request_info, | 
| HttpStreamRequest::Delegate* delegate) { | 
| + GURL original_url = request_info.url; | 
| + | 
| if (original_url.SchemeIs("ftp")) | 
| - return AlternativeServiceVector(); | 
| + return AlternativeService(); | 
| HostPortPair origin = HostPortPair::FromURL(original_url); | 
| HttpServerProperties& http_server_properties = | 
| @@ -170,7 +174,7 @@ AlternativeServiceVector HttpStreamFactoryImpl::GetAlternativeServicesFor( | 
| const AlternativeServiceVector alternative_service_vector = | 
| http_server_properties.GetAlternativeServices(origin); | 
| if (alternative_service_vector.empty()) | 
| - return AlternativeServiceVector(); | 
| + return AlternativeService(); | 
| bool quic_advertised = false; | 
| bool quic_all_broken = true; | 
| @@ -178,7 +182,9 @@ AlternativeServiceVector HttpStreamFactoryImpl::GetAlternativeServicesFor( | 
| const bool enable_different_host = | 
| session_->params().use_alternative_services; | 
| - AlternativeServiceVector enabled_alternative_service_vector; | 
| + // First Alt-Svc that is not marked as broken. | 
| + AlternativeService candidate_alternative_service; | 
| + | 
| for (const AlternativeService& alternative_service : | 
| alternative_service_vector) { | 
| DCHECK(IsAlternateProtocolValid(alternative_service.protocol)); | 
| @@ -214,7 +220,10 @@ AlternativeServiceVector HttpStreamFactoryImpl::GetAlternativeServicesFor( | 
| if (session_->HasSpdyExclusion(origin)) | 
| continue; | 
| - enabled_alternative_service_vector.push_back(alternative_service); | 
| + // Cache the first entry. | 
| + if (candidate_alternative_service.protocol == | 
| + UNINITIALIZED_ALTERNATE_PROTOCOL) | 
| + candidate_alternative_service = alternative_service; | 
| continue; | 
| } | 
| @@ -229,11 +238,26 @@ AlternativeServiceVector HttpStreamFactoryImpl::GetAlternativeServicesFor( | 
| if (!original_url.SchemeIs("https")) | 
| continue; | 
| - enabled_alternative_service_vector.push_back(alternative_service); | 
| + // Check whether there's an active session to pool for this QUIC Alt-Svc. | 
| + HostPortPair destination = alternative_service.host_port_pair(); | 
| + StringPiece origin_host = | 
| + ApplyHostMappingRules(request_info.url, &destination).host(); | 
| + QuicServerId server_id(destination, request_info.privacy_mode); | 
| + if (session_->quic_stream_factory()->CanPool( | 
| + server_id, request_info.privacy_mode, origin_host)) | 
| + return alternative_service; | 
| + | 
| + // Cache this entry if the first non-broken Alt-Svc is not there yet. | 
| + if (candidate_alternative_service.protocol == | 
| + UNINITIALIZED_ALTERNATE_PROTOCOL) | 
| + candidate_alternative_service = alternative_service; | 
| } | 
| + | 
| + // Ask delegate to mark QUIC as broken for the origin. | 
| if (quic_advertised && quic_all_broken && delegate != nullptr) | 
| delegate->OnQuicBroken(); | 
| - return enabled_alternative_service_vector; | 
| + | 
| + return candidate_alternative_service; | 
| } | 
| void HttpStreamFactoryImpl::OrphanJob(Job* job, const Request* request) { |