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);