QUIC - minor reorg of QuicCryptoClientStream::DoHandshakeLoop.

Added Do... methods for each state in DoHandshakeLoop (similar to the
conventions that we typically use in our state machines).

DoVerifyProof and DoGetChannelID which could do async operation
return  QuicAsyncStatus.

In DoHandshakeLoop changed "for (;;) {" to be similar to other
do { ...} while loops of other state machines in chromium.

R=rch@chromium.org,agl@chromium.org

Review URL: https://codereview.chromium.org/588443002

Cr-Commit-Position: refs/heads/master@{#297316}

net/quic/quic_crypto_client_stream.cc

@@ -175,17 +175,56 @@ static const int kMaxClientHellos = 3;
 
 void QuicCryptoClientStream::DoHandshakeLoop(
     const CryptoHandshakeMessage* in) {
-  CryptoHandshakeMessage out;
-  QuicErrorCode error;
-  string error_details;
   QuicCryptoClientConfig::CachedState* cached =
       crypto_config_->LookupOrCreate(server_id_);
 
-  for (;;) {
+  QuicAsyncStatus rv = QUIC_SUCCESS;
+  do {
+    CHECK_NE(STATE_NONE, next_state_);
     const State state = next_state_;
     next_state_ = STATE_IDLE;
+    rv = QUIC_SUCCESS;
     switch (state) {
-      case STATE_INITIALIZE: {
+      case STATE_INITIALIZE:
+        DoInitialize(cached);
+        break;
+      case STATE_SEND_CHLO:
+        DoSendCHLO(in, cached);
+        return;
+      case STATE_RECV_REJ:
+        DoReceiveREJ(in, cached);
+        break;
+      case STATE_VERIFY_PROOF:
+        rv = DoVerifyProof(cached);
+        break;
+      case STATE_VERIFY_PROOF_COMPLETE:
+        DoVerifyProofComplete(cached);
+        break;
+      case STATE_GET_CHANNEL_ID:
+        rv = DoGetChannelID(cached);
+        break;
+      case STATE_GET_CHANNEL_ID_COMPLETE:
+        DoGetChannelIDComplete();
+        break;
+      case STATE_RECV_SHLO:
+        DoReceiveSHLO(in, cached);
+        break;
+      case STATE_IDLE:
+        // This means that the peer sent us a message that we weren't expecting.
+        CloseConnection(QUIC_INVALID_CRYPTO_MESSAGE_TYPE);
+        return;
+      case STATE_INITIALIZE_SCUP:
+        DoInitializeServerConfigUpdate(cached);
+        break;
+      case STATE_NONE:
+        NOTREACHED();
+        return;  // We are done.
+    }
+  } while (rv != QUIC_PENDING && next_state_ != STATE_NONE);
+}
+
+void QuicCryptoClientStream::DoInitialize(
+    QuicCryptoClientConfig::CachedState* cached) {
   if (!cached->IsEmpty() && !cached->signature().empty() &&
       server_id_.is_https()) {
     // Note that we verify the proof even if the cached proof is valid.
@@ -198,9 +237,11 @@ void QuicCryptoClientStream::DoHandshakeLoop(
   } else {
     next_state_ = STATE_GET_CHANNEL_ID;
   }
-        break;
-      }
-      case STATE_SEND_CHLO: {
+}
+
+void QuicCryptoClientStream::DoSendCHLO(
+    const CryptoHandshakeMessage* in,
+    QuicCryptoClientConfig::CachedState* cached) {
   // Send the client hello in plaintext.
   session()->connection()->SetDefaultEncryptionLevel(ENCRYPTION_NONE);
   if (num_client_hellos_ > kMaxClientHellos) {
@@ -209,6 +250,7 @@ void QuicCryptoClientStream::DoHandshakeLoop(
   }
   num_client_hellos_++;
 
+  CryptoHandshakeMessage out;
   if (!cached->IsComplete(session()->connection()->clock()->WallNow())) {
     crypto_config_->FillInchoateClientHello(
         server_id_,
@@ -234,8 +276,10 @@ void QuicCryptoClientStream::DoHandshakeLoop(
     SendHandshakeMessage(out);
     return;
   }
+
   session()->config()->ToHandshakeMessage(&out);
-        error = crypto_config_->FillClientHello(
+  string error_details;
+  QuicErrorCode error = crypto_config_->FillClientHello(
       server_id_,
       session()->connection()->connection_id(),
       session()->connection()->supported_versions().front(),
@@ -280,22 +324,27 @@ void QuicCryptoClientStream::DoHandshakeLoop(
     session()->OnCryptoHandshakeEvent(
         QuicSession::ENCRYPTION_REESTABLISHED);
   }
-        return;
-      }
-      case STATE_RECV_REJ:
+}
+
+void QuicCryptoClientStream::DoReceiveREJ(
+    const CryptoHandshakeMessage* in,
+    QuicCryptoClientConfig::CachedState* cached) {
   // We sent a dummy CHLO because we didn't have enough information to
   // perform a handshake, or we sent a full hello that the server
   // rejected. Here we hope to have a REJ that contains the information
   // that we need.
   if (in->tag() != kREJ) {
+    next_state_ = STATE_NONE;
     CloseConnectionWithDetails(QUIC_INVALID_CRYPTO_MESSAGE_TYPE,
                                "Expected REJ");
     return;
   }
-        error = crypto_config_->ProcessRejection(
+  string error_details;
+  QuicErrorCode error = crypto_config_->ProcessRejection(
       *in, session()->connection()->clock()->WallNow(), cached,
       server_id_.is_https(), &crypto_negotiated_params_, &error_details);
   if (error != QUIC_NO_ERROR) {
+    next_state_ = STATE_NONE;
     CloseConnectionWithDetails(error, error_details);
     return;
   }
@@ -310,28 +359,84 @@ void QuicCryptoClientStream::DoHandshakeLoop(
       // so we can assume no CA trust changes or certificate expiration
       // has happened since then.
       next_state_ = STATE_VERIFY_PROOF;
-            break;
+      return;
     }
   }
   next_state_ = STATE_GET_CHANNEL_ID;
+}
+
+QuicAsyncStatus QuicCryptoClientStream::DoVerifyProof(
+    QuicCryptoClientConfig::CachedState* cached) {
+  ProofVerifier* verifier = crypto_config_->proof_verifier();
+  DCHECK(verifier);
+  next_state_ = STATE_VERIFY_PROOF_COMPLETE;
+  generation_counter_ = cached->generation_counter();
+
+  ProofVerifierCallbackImpl* proof_verify_callback =
+      new ProofVerifierCallbackImpl(this);
+
+  verify_ok_ = false;
+
+  QuicAsyncStatus status = verifier->VerifyProof(
+      server_id_.host(),
+      cached->server_config(),
+      cached->certs(),
+      cached->signature(),
+      verify_context_.get(),
+      &verify_error_details_,
+      &verify_details_,
+      proof_verify_callback);
+
+  switch (status) {
+    case QUIC_PENDING:
+      proof_verify_callback_ = proof_verify_callback;
+      DVLOG(1) << "Doing VerifyProof";
       break;
-      case STATE_VERIFY_PROOF: {
-        if (QUIC_PENDING == DoVerifyProof(cached)) {
-          return;
-        }
+    case QUIC_FAILURE:
+      delete proof_verify_callback;
+      break;
+    case QUIC_SUCCESS:
+      delete proof_verify_callback;
+      verify_ok_ = true;
       break;
   }
-      case STATE_VERIFY_PROOF_COMPLETE:
-        if (QUIC_PROOF_INVALID == DoVerifyProofComplete(cached)) {
+  return status;
+}
+
+void QuicCryptoClientStream::DoVerifyProofComplete(
+    QuicCryptoClientConfig::CachedState* cached) {
+  if (!verify_ok_) {
+    next_state_ = STATE_NONE;
+    client_session()->OnProofVerifyDetailsAvailable(*verify_details_);
+    UMA_HISTOGRAM_BOOLEAN("Net.QuicVerifyProofFailed.HandshakeConfirmed",
+                          handshake_confirmed());
+    CloseConnectionWithDetails(
+        QUIC_PROOF_INVALID, "Proof invalid: " + verify_error_details_);
     return;
   }
-        break;
-      case STATE_GET_CHANNEL_ID: {
+
+  // Check if generation_counter has changed between STATE_VERIFY_PROOF and
+  // STATE_VERIFY_PROOF_COMPLETE state changes.
+  if (generation_counter_ != cached->generation_counter()) {
+    next_state_ = STATE_VERIFY_PROOF;
+  } else {
+    SetCachedProofValid(cached);
+    cached->SetProofVerifyDetails(verify_details_.release());
+    if (!handshake_confirmed()) {
+      next_state_ = STATE_GET_CHANNEL_ID;
+    } else {
+      next_state_ = STATE_NONE;
+    }
+  }
+}
+
+QuicAsyncStatus QuicCryptoClientStream::DoGetChannelID(
+    QuicCryptoClientConfig::CachedState* cached) {
   next_state_ = STATE_GET_CHANNEL_ID_COMPLETE;
   channel_id_key_.reset();
   if (!RequiresChannelID(cached)) {
     next_state_ = STATE_SEND_CHLO;
-          break;
+    return QUIC_SUCCESS;
   }
 
   ChannelIDSourceCallbackImpl* channel_id_source_callback =
@@ -345,27 +450,34 @@ void QuicCryptoClientStream::DoHandshakeLoop(
     case QUIC_PENDING:
       channel_id_source_callback_ = channel_id_source_callback;
       DVLOG(1) << "Looking up channel ID";
-            return;
+      break;
     case QUIC_FAILURE:
+      next_state_ = STATE_NONE;
       delete channel_id_source_callback;
       CloseConnectionWithDetails(QUIC_INVALID_CHANNEL_ID_SIGNATURE,
                                  "Channel ID lookup failed");
-            return;
+      break;
     case QUIC_SUCCESS:
       delete channel_id_source_callback;
       break;
   }
-        break;
-      }
-      case STATE_GET_CHANNEL_ID_COMPLETE:
+  return status;
+}
+
+void QuicCryptoClientStream::DoGetChannelIDComplete() {
   if (!channel_id_key_.get()) {
+    next_state_ = STATE_NONE;
     CloseConnectionWithDetails(QUIC_INVALID_CHANNEL_ID_SIGNATURE,
                                "Channel ID lookup failed");
     return;
   }
   next_state_ = STATE_SEND_CHLO;
-        break;
-      case STATE_RECV_SHLO: {
+}
+
+void QuicCryptoClientStream::DoReceiveSHLO(
+    const CryptoHandshakeMessage* in,
+    QuicCryptoClientConfig::CachedState* cached) {
+  next_state_ = STATE_NONE;
   // We sent a CHLO that we expected to be accepted and now we're hoping
   // for a SHLO from the server to confirm that.
   if (in->tag() == kREJ) {
@@ -379,13 +491,15 @@ void QuicCryptoClientStream::DoHandshakeLoop(
       return;
     }
     next_state_ = STATE_RECV_REJ;
-          break;
+    return;
   }
+
   if (in->tag() != kSHLO) {
     CloseConnectionWithDetails(QUIC_INVALID_CRYPTO_MESSAGE_TYPE,
                                "Expected SHLO or REJ");
     return;
   }
+
   // alternative_decrypter will be NULL if the original alternative
   // decrypter latched and became the primary decrypter. That happens
   // if we received a message encrypted with the INITIAL key.
@@ -395,27 +509,25 @@ void QuicCryptoClientStream::DoHandshakeLoop(
                                "unencrypted SHLO message");
     return;
   }
-        error = crypto_config_->ProcessServerHello(
+
+  string error_details;
+  QuicErrorCode error = crypto_config_->ProcessServerHello(
       *in, session()->connection()->connection_id(),
       session()->connection()->server_supported_versions(),
       cached, &crypto_negotiated_params_, &error_details);
 
   if (error != QUIC_NO_ERROR) {
-          CloseConnectionWithDetails(
-              error, "Server hello invalid: " + error_details);
+    CloseConnectionWithDetails(error, "Server hello invalid: " + error_details);
     return;
   }
-        error =
-            session()->config()->ProcessPeerHello(*in, SERVER, &error_details);
+  error = session()->config()->ProcessPeerHello(*in, SERVER, &error_details);
   if (error != QUIC_NO_ERROR) {
-          CloseConnectionWithDetails(
-              error, "Server hello invalid: " + error_details);
+    CloseConnectionWithDetails(error, "Server hello invalid: " + error_details);
     return;
   }
   session()->OnConfigNegotiated();
 
-        CrypterPair* crypters =
-            &crypto_negotiated_params_.forward_secure_crypters;
+  CrypterPair* crypters = &crypto_negotiated_params_.forward_secure_crypters;
   // TODO(agl): we don't currently latch this decrypter because the idea
   // has been floated that the server shouldn't send packets encrypted
   // with the FORWARD_SECURE key until it receives a FORWARD_SECURE
@@ -431,19 +543,6 @@ void QuicCryptoClientStream::DoHandshakeLoop(
   handshake_confirmed_ = true;
   session()->OnCryptoHandshakeEvent(QuicSession::HANDSHAKE_CONFIRMED);
   session()->connection()->OnHandshakeComplete();
-        return;
-      }
-      case STATE_IDLE:
-        // This means that the peer sent us a message that we weren't expecting.
-        CloseConnection(QUIC_INVALID_CRYPTO_MESSAGE_TYPE);
-        return;
-      case STATE_INITIALIZE_SCUP:
-        DoInitializeServerConfigUpdate(cached);
-        break;
-      case STATE_VERIFY_PROOF_DONE:
-        return;  // We are done.
-    }
-  }
 }
 
 void QuicCryptoClientStream::DoInitializeServerConfigUpdate(
@@ -452,84 +551,19 @@ void QuicCryptoClientStream::DoInitializeServerConfigUpdate(
   if (!server_id_.is_https()) {
     // We don't check the certificates for insecure QUIC connections.
     SetCachedProofValid(cached);
-    next_state_ = STATE_VERIFY_PROOF_DONE;
+    next_state_ = STATE_NONE;
   } else if (!cached->IsEmpty() && !cached->signature().empty()) {
     // Note that we verify the proof even if the cached proof is valid.
     DCHECK(crypto_config_->proof_verifier());
     next_state_ = STATE_VERIFY_PROOF;
   } else {
     update_ignored = true;
-    next_state_ = STATE_VERIFY_PROOF_DONE;
+    next_state_ = STATE_NONE;
   }
-  UMA_HISTOGRAM_BOOLEAN("Net.QuicNumServerConfig.UpdateMessagesIgnored",
+  UMA_HISTOGRAM_COUNTS("Net.QuicNumServerConfig.UpdateMessagesIgnored",
                        update_ignored);
 }
 
-QuicAsyncStatus QuicCryptoClientStream::DoVerifyProof(
-    QuicCryptoClientConfig::CachedState* cached) {
-  ProofVerifier* verifier = crypto_config_->proof_verifier();
-  DCHECK(verifier);
-  next_state_ = STATE_VERIFY_PROOF_COMPLETE;
-  generation_counter_ = cached->generation_counter();
-
-  ProofVerifierCallbackImpl* proof_verify_callback =
-      new ProofVerifierCallbackImpl(this);
-
-  verify_ok_ = false;
-
-  QuicAsyncStatus status = verifier->VerifyProof(
-      server_id_.host(),
-      cached->server_config(),
-      cached->certs(),
-      cached->signature(),
-      verify_context_.get(),
-      &verify_error_details_,
-      &verify_details_,
-      proof_verify_callback);
-
-  switch (status) {
-    case QUIC_PENDING:
-      proof_verify_callback_ = proof_verify_callback;
-      DVLOG(1) << "Doing VerifyProof";
-      break;
-    case QUIC_FAILURE:
-      delete proof_verify_callback;
-      break;
-    case QUIC_SUCCESS:
-      delete proof_verify_callback;
-      verify_ok_ = true;
-      break;
-  }
-  return status;
-}
-
-QuicErrorCode QuicCryptoClientStream::DoVerifyProofComplete(
-    QuicCryptoClientConfig::CachedState* cached) {
-  if (!verify_ok_) {
-    client_session()->OnProofVerifyDetailsAvailable(*verify_details_);
-    UMA_HISTOGRAM_BOOLEAN("Net.QuicVerifyProofFailed.HandshakeConfirmed",
-                          handshake_confirmed());
-    CloseConnectionWithDetails(
-        QUIC_PROOF_INVALID, "Proof invalid: " + verify_error_details_);
-    return QUIC_PROOF_INVALID;
-  }
-
-  // Check if generation_counter has changed between STATE_VERIFY_PROOF and
-  // STATE_VERIFY_PROOF_COMPLETE state changes.
-  if (generation_counter_ != cached->generation_counter()) {
-    next_state_ = STATE_VERIFY_PROOF;
-  } else {
-    SetCachedProofValid(cached);
-    cached->SetProofVerifyDetails(verify_details_.release());
-    if (!handshake_confirmed()) {
-      next_state_ = STATE_GET_CHANNEL_ID;
-    } else {
-      next_state_ = STATE_VERIFY_PROOF_DONE;
-    }
-  }
-  return QUIC_NO_ERROR;
-}
-
 void QuicCryptoClientStream::SetCachedProofValid(
     QuicCryptoClientConfig::CachedState* cached) {
   cached->SetProofValid();

net/quic/quic_crypto_client_stream.h

@@ -107,7 +107,7 @@ class NET_EXPORT_PRIVATE QuicCryptoClientStream : public QuicCryptoStream {
     STATE_GET_CHANNEL_ID_COMPLETE,
     STATE_RECV_SHLO,
     STATE_INITIALIZE_SCUP,
-    STATE_VERIFY_PROOF_DONE,
+    STATE_NONE,
   };
 
   // Handles new server config and optional source-address token provided by the
@@ -119,19 +119,41 @@ class NET_EXPORT_PRIVATE QuicCryptoClientStream : public QuicCryptoStream {
   // |in| may be NULL if the call did not result from a received message.
   void DoHandshakeLoop(const CryptoHandshakeMessage* in);
 
-  // Start the proof verification if |server_id_| is https and |cached| has
-  // signature.
-  void DoInitializeServerConfigUpdate(
+  // Start the handshake process.
+  void DoInitialize(QuicCryptoClientConfig::CachedState* cached);
+
+  // Send either InchoateClientHello or ClientHello message to the server.
+  void DoSendCHLO(const CryptoHandshakeMessage* in,
                   QuicCryptoClientConfig::CachedState* cached);
 
-  // Starts the proof verification. Returns the QuicAsyncStatus returned by the
-  // ProofVerifier's VerifyProof.
+  // Process REJ message from the server.
+  void DoReceiveREJ(const CryptoHandshakeMessage* in,
+                    QuicCryptoClientConfig::CachedState* cached);
+
+  // Start the proof verification process. Returns the QuicAsyncStatus returned
+  // by the ProofVerifier's VerifyProof.
   QuicAsyncStatus DoVerifyProof(QuicCryptoClientConfig::CachedState* cached);
 
   // If proof is valid then it sets the proof as valid (which persists the
-  // server config) and returns QUIC_NO_ERROR. If not, it closes the connection
-  // and returns QUIC_PROOF_INVALID.
-  QuicErrorCode DoVerifyProofComplete(
+  // server config). If not, it closes the connection.
+  void DoVerifyProofComplete(QuicCryptoClientConfig::CachedState* cached);
+
+  // Start the look up of Channel ID process. Returns either QUIC_SUCCESS if
+  // RequiresChannelID returns false or QuicAsyncStatus returned by
+  // GetChannelIDKey.
+  QuicAsyncStatus DoGetChannelID(QuicCryptoClientConfig::CachedState* cached);
+
+  // If there is no channel ID, then close the connection otherwise transtion to
+  // STATE_SEND_CHLO state.
+  void DoGetChannelIDComplete();
+
+  // Process SHLO message from the server.
+  void DoReceiveSHLO(const CryptoHandshakeMessage* in,
+                     QuicCryptoClientConfig::CachedState* cached);
+
+  // Start the proof verification if |server_id_| is https and |cached| has
+  // signature.
+  void DoInitializeServerConfigUpdate(
       QuicCryptoClientConfig::CachedState* cached);
 
   // Called to set the proof of |cached| valid.  Also invokes the session's