Persist the server config that is received via kSCUP.

net/quic/quic_crypto_client_stream.cc

Index: net/quic/quic_crypto_client_stream.cc
diff --git a/net/quic/quic_crypto_client_stream.cc b/net/quic/quic_crypto_client_stream.cc
index 40e4d45901c531d6fd711e3d1067a3b4cda7d953..94a7c14402ad8c9e98d08a1f4f9925edec9a6fc6 100644
--- a/net/quic/quic_crypto_client_stream.cc
+++ b/net/quic/quic_crypto_client_stream.cc
@@ -156,6 +156,13 @@ void QuicCryptoClientStream::HandleServerConfigUpdateMessage(
         error, "Server config update invalid: " + error_details);
     return;
   }
+
+  DCHECK(handshake_confirmed());
+  if (proof_verify_callback_) {
+    proof_verify_callback_->Cancel();
+  }
+  next_state_ = STATE_INITIALIZE_SCUP;
+  DoProofVeifyLoop();
 }
 
 // kMaxClientHellos is the maximum number of times that we'll send a client
@@ -308,57 +315,17 @@ void QuicCryptoClientStream::DoHandshakeLoop(
         next_state_ = STATE_GET_CHANNEL_ID;
         break;
       case STATE_VERIFY_PROOF: {
-        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";
-            return;
-          case QUIC_FAILURE:
-            delete proof_verify_callback;
-            break;
-          case QUIC_SUCCESS:
-            delete proof_verify_callback;
-            verify_ok_ = true;
-            break;
+        DCHECK(!handshake_confirmed());
+        if (QUIC_PENDING == DoVerifyProof(cached)) {
+          return;
         }
         break;
       }
       case STATE_VERIFY_PROOF_COMPLETE:
-        if (!verify_ok_) {
-          client_session()->OnProofVerifyDetailsAvailable(*verify_details_);
-          CloseConnectionWithDetails(
-              QUIC_PROOF_INVALID, "Proof invalid: " + verify_error_details_);
+        DCHECK(!handshake_confirmed());
+        if (QUIC_NO_ERROR != DoVerifyProofComplete(cached)) {
           return;
         }
-        // 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());
-          next_state_ = STATE_GET_CHANNEL_ID;
-        }
         break;
       case STATE_GET_CHANNEL_ID: {
         next_state_ = STATE_GET_CHANNEL_ID_COMPLETE;
@@ -471,8 +438,129 @@ void QuicCryptoClientStream::DoHandshakeLoop(
         // 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:
+      case STATE_VERIFY_PROOF_SCUP:
+      case STATE_VERIFY_PROOF_COMPLETE_SCUP:
+        NOTREACHED();
+        return;
+    }
+  }
+}
+
+void QuicCryptoClientStream::DoProofVeifyLoop() {
+  QuicCryptoClientConfig::CachedState* cached =
+      crypto_config_->LookupOrCreate(server_id_);
+  for (;;) {
+    const State state = next_state_;
+    next_state_ = STATE_IDLE;
+    switch (state) {
+      case STATE_INITIALIZE_SCUP: {
+        if (!server_id_.is_https()) {
+          // We don't check the certificates for insecure QUIC connections.
+          SetCachedProofValid(cached);
+        } else {
+          if (!cached->IsEmpty() && !cached->signature().empty()) {
+            // Note that we verify the proof even if the cached proof is valid.
+            // This allows us to respond to CA trust changes or certificate
+            // expiration because it may have been a while since we last
+            // verified the proof.
+            DCHECK(crypto_config_->proof_verifier());
+            // If the cached state needs to be verified, do it now.
+            next_state_ = STATE_VERIFY_PROOF_SCUP;
+          }
+        }
+        break;
+      }
+      case STATE_VERIFY_PROOF_SCUP: {
+        if (QUIC_PENDING == DoVerifyProof(cached)) {
+          return;
+        }
+        break;
+      }
+      case STATE_VERIFY_PROOF_COMPLETE_SCUP: {
+        if (QUIC_NO_ERROR != DoVerifyProofComplete(cached)) {
+          return;
+        }
+        break;
+      }
+      case STATE_IDLE:
+        return;  // We are done.
+      default:
+        NOTREACHED();
+        return;
+
+    }
+  }
+}
+
+QuicAsyncStatus QuicCryptoClientStream::DoVerifyProof(
+    QuicCryptoClientConfig::CachedState* cached) {
+  ProofVerifier* verifier = crypto_config_->proof_verifier();
+  DCHECK(verifier);
+  if (handshake_confirmed()) {
+    next_state_ = STATE_VERIFY_PROOF_COMPLETE_SCUP;
+  } else {
+    next_state_ = STATE_VERIFY_PROOF_COMPLETE;

[Ryan Hamilton, 2014/09/15 22:37:56]

Since we have to conditionally set next_state_ in these methods, I wonder if it
wouldn't be easier to just have a single DoLoop (or DoHandshakeLoop) method
which is capable of handling any of the state transitions, instead of a
HandshakeLoop and a ProofLoop?

[ramant (doing other things), 2014/09/16 21:06:32]

Done.

+  }
+  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_);
+    CloseConnectionWithDetails(
+        QUIC_PROOF_INVALID, "Proof invalid: " + verify_error_details_);
+    return QUIC_PROOF_INVALID;

[Ryan Hamilton, 2014/09/15 22:37:56]

possibly add a histogram here when the handshake has been confirmed to see how
often (never?) this happens.

[ramant (doing other things), 2014/09/16 21:06:32]

Done.

+  }
+
+  // Check if generation_counter has changed between STATE_VERIFY_PROOF (or
+  // STATE_VERIFY_PROOF_SCUP) and STATE_VERIFY_PROOF_COMPLETE (or
+  // STATE_VERIFY_PROOF_COMPLETE_SCUP) state changes.
+  if (generation_counter_ != cached->generation_counter()) {
+    if (handshake_confirmed()) {
+      next_state_ = STATE_VERIFY_PROOF_SCUP;
+    } else {
+      next_state_ = STATE_VERIFY_PROOF;
+    }
+  } else {
+    SetCachedProofValid(cached);
+    cached->SetProofVerifyDetails(verify_details_.release());
+    if (!handshake_confirmed()) {
+      next_state_ = STATE_GET_CHANNEL_ID;
     }
   }
+  return QUIC_NO_ERROR;
 }
 
 void QuicCryptoClientStream::SetCachedProofValid(