Document callbacks from QuicSessionPool::Job
This updates comments about some of the callbacks from
`QuicSessionPool::Job` and their eventual use to start the main job when
a QUIC alternative job will fail.
It includes a note about the delayed nature of the
`OnHostResolutionComplete` callback.
Change-Id: I2db1dcbb03e0709045946530bf2510ca719d8cab
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5228397
Reviewed-by: Liza Burakova <liza@chromium.org>
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Commit-Queue: Dustin Mitchell <djmitche@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1252642}
diff --git a/net/http/http_stream_factory_job.cc b/net/http/http_stream_factory_job.cc
index 83431788..1142499 100644
--- a/net/http/http_stream_factory_job.cc
+++ b/net/http/http_stream_factory_job.cc
@@ -967,6 +967,9 @@
// delay the main job.
delegate_->MaybeSetWaitTimeForMainJob(
quic_request_.GetTimeDelayForWaitingJob());
+ // Set up to get notified of either host resolution completion or session
+ // creation, in order to call the delegate's `OnConnectionInitialized`
+ // callback.
expect_on_quic_host_resolution_ = quic_request_.WaitForHostResolution(
base::BindOnce(&Job::OnQuicHostResolution, base::Unretained(this)));
expect_on_quic_session_created_ = quic_request_.WaitForQuicSessionCreation(
@@ -975,20 +978,25 @@
return rv;
}
+void HttpStreamFactory::Job::OnQuicHostResolution(int result) {
+ DCHECK(expect_on_quic_host_resolution_);
+ expect_on_quic_host_resolution_ = false;
+ // If no `OnQuicSessionCreated` call is expected, then consider the
+ // connection "initialized" and inform the delegate. Note that
+ // `OnQuicHostResolution` is actually called somewhat _after_ host resolution
+ // is complete -- the `Job` has already run to the point where it can make no
+ // further progress.
+ if (!expect_on_quic_session_created_) {
+ delegate_->OnConnectionInitialized(this, result);
+ }
+}
+
void HttpStreamFactory::Job::OnQuicSessionCreated(int result) {
DCHECK(expect_on_quic_session_created_);
expect_on_quic_session_created_ = false;
delegate_->OnConnectionInitialized(this, result);
}
-void HttpStreamFactory::Job::OnQuicHostResolution(int result) {
- DCHECK(expect_on_quic_host_resolution_);
- expect_on_quic_host_resolution_ = false;
- if (!expect_on_quic_session_created_) {
- delegate_->OnConnectionInitialized(this, result);
- }
-}
-
void HttpStreamFactory::Job::OnFailedOnDefaultNetwork(int result) {
DCHECK(job_type_ == ALTERNATIVE || job_type_ == DNS_ALPN_H3);
DCHECK(using_quic_);
diff --git a/net/http/http_stream_factory_job.h b/net/http/http_stream_factory_job.h
index 94295a4..8312c72 100644
--- a/net/http/http_stream_factory_job.h
+++ b/net/http/http_stream_factory_job.h
@@ -114,7 +114,9 @@
Job* job,
const ConnectionAttempts& attempts) = 0;
- // Invoked when |job| finishes initiating a connection.
+ // Invoked when |job| finishes initiating a connection. This may occur
+ // before the handshake is complete, and provides the delegate an
+ // early chance to handle any errors.
virtual void OnConnectionInitialized(Job* job, int rv) = 0;
// Return false if |job| can advance to the next state. Otherwise, |job|
diff --git a/net/quic/quic_session_pool.h b/net/quic/quic_session_pool.h
index d8d58e11..ef7548f 100644
--- a/net/quic/quic_session_pool.h
+++ b/net/quic/quic_session_pool.h
@@ -168,14 +168,6 @@
// ERR_IO_PENDING.
bool WaitForHostResolution(CompletionOnceCallback callback);
- // Tells QuicSessionRequest it should expect OnHostResolutionComplete()
- // to be called in the future.
- void ExpectOnHostResolution();
-
- // Will be called by the associated QuicSessionPool::Job when host
- // resolution completes asynchronously after Request().
- void OnHostResolutionComplete(int rv);
-
// This function must be called after Request() returns ERR_IO_PENDING.
// Returns true if no QUIC session has been created yet. If true is returned,
// `callback` will be run when the QUIC session has been created and will be
@@ -184,12 +176,31 @@
// `callback` will be run with ERR_IO_PENDING.
bool WaitForQuicSessionCreation(CompletionOnceCallback callback);
- // Tells QuicSessionRequest it should expect OnQuicSessionCreationComplete()
- // to be called in the future.
+ // QuicSessionPool::Jobs may notify associated requests at two points in the
+ // connection process before completion: host resolution and session creation.
+ // The `Expect` methods below inform the request whether it should expect
+ // these notifications.
+
+ // Tells QuicSessionRequest that `QuicSessionPool::Job` will call
+ // `OnHostResolutionComplete()` in the future. Must be called before
+ // `WaitForHostResolution()`
+ void ExpectOnHostResolution();
+
+ // Will be called by the associated `QuicSessionPool::Job` when host
+ // resolution completes asynchronously after Request(), if
+ // `ExpectOnHostResolution()` was called. This is called after the Job can
+ // make no further progress, and includes the result of that progress, perhaps
+ // `ERR_IO_PENDING`.
+ void OnHostResolutionComplete(int rv);
+
+ // Tells QuicSessionRequest that `QuicSessionPool::Job` will call
+ // `OnQuicSessionCreationComplete()` in the future. Must be called before
+ // `WaitForQuicSessionCreation()`.
void ExpectQuicSessionCreation();
- // Will be called by the associated QuicSessionPool::Job when session
- // creation completes asynchronously after Request().
+ // Will be called by the associated `QuicSessionPool::Job` when session
+ // creation completes asynchronously after Request(), if
+ // `ExpectQuicSessionCreation` was called.
void OnQuicSessionCreationComplete(int rv);
void OnRequestComplete(int rv);