net/: Use both QUIC and TCP for initial URL requests#9262
net/: Use both QUIC and TCP for initial URL requests#9262ckim1346 wants to merge 4 commits intoyoutube:mainfrom
Conversation
🤖 Gemini Suggested Commit Message💡 Pro Tips for a Better Commit Message:
|
| #if BUILDFLAG(IS_ANDROIDTV) | ||
| if (session_->IsQuicEnabled() && session_->UseQuicForUnknownOrigin()) { | ||
| url::SchemeHostPort origin(original_url); | ||
| #if defined(COBALT_BUILD_TYPE_GOLD) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
#3514 I would assume for testing/debugging purposes -- @kaidokert do you remember the exact reasoning?
There was a problem hiding this comment.
I think this is for mirroring the enforcing of https for gold builds.
There was a problem hiding this comment.
I think we should at least enforce it to be a privileged port (<1024).
There was a problem hiding this comment.
Code Review
This pull request reintroduces the use_quic_for_unknown_origins feature, which opportunistically attempts a QUIC connection for initial requests to improve performance, with a fallback to TCP. The implementation correctly restricts this behavior to port 443 for production builds. However, there is a critical preprocessor logic error in http_stream_factory_job_controller.cc that will cause a compilation failure. I've provided a comment with a suggested fix for this issue.
This PR reintroduces the
use_quic_for_unknown_originsflag to try both QUIC and TCP for initial/unknown URL requests. If QUIC returns first, it will cancel the TCP request and otherwise fall back on the TCP ensuring we try quick but also have a reliable fallback. Additionally, we allow non-production builds to use any port whereas production builds can only use port 443.Please refer to #2981 and #3514.
Bug: 467778593