-
Notifications
You must be signed in to change notification settings - Fork 189
net/: Use both QUIC and TCP for initial URL requests #9262
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1272,6 +1272,26 @@ HttpStreamFactory::JobController::GetAlternativeServiceInfoInternal( | |
| url::SchemeHostPort(original_url), | ||
| request_info.network_anonymization_key); | ||
| if (alternative_service_info_vector.empty()) { | ||
| #if BUILDFLAG(IS_COBALT) | ||
| // This block of code suggests QUIC connection for initial requests to a | ||
| // new host. This method is proven to provide performance benefit while still | ||
| // enabling Cobalt network module to fall back on TCP connection when QUIC | ||
| // fails or is too slow. | ||
| if (session_->IsQuicEnabled() && session_->UseQuicForUnknownOrigin()) { | ||
| url::SchemeHostPort origin(original_url); | ||
| #if defined(COBALT_BUILD_TYPE_GOLD) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When would port not be 443? I also think it's dangerous to have qa and gold differ on this behavior. Prod could break if there's some issue with the prod-specific code.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. #3514 I would assume for testing/debugging purposes -- @kaidokert do you remember the exact reasoning?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is for mirroring the enforcing of https for gold builds.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should at least enforce it to be a privileged port (<1024). |
||
| const int kDefaultQUICServerPort = 443; | ||
| if (origin.port() == kDefaultQUICServerPort) { | ||
| #endif | ||
| quic::ParsedQuicVersionVector versions = quic::AllSupportedVersions(); | ||
| return AlternativeServiceInfo::CreateQuicAlternativeServiceInfo( | ||
| AlternativeService(NextProto::kProtoQUIC, origin.host(), origin.port()), | ||
| base::Time::Max(), versions); | ||
| #if defined(COBALT_BUILD_TYPE_GOLD) | ||
| } | ||
| #endif | ||
ckim1346 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| #endif // BUILDFLAG(IS_COBALT) | ||
| return AlternativeServiceInfo(); | ||
| } | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.