When HttpNetworkTransaction encounters QUIC errors, retry the request

net/http/http_network_transaction.cc

Index: net/http/http_network_transaction.cc
diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index 456a871d22c66ce2b868f7905866c19456ecc79f..0c6ae65d809f04031379141bbff7b574db9e3e0b 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -851,9 +851,11 @@ int HttpNetworkTransaction::DoCreateStream() {
   response_.network_accessed = true;
 
   next_state_ = STATE_CREATE_STREAM_COMPLETE;
-  // IP based pooling and Alternative Services are disabled under the same
-  // circumstances: on a retry after 421 Misdirected Request is received.
-  DCHECK(enable_ip_based_pooling_ == enable_alternative_services_);
+  // IP based pooling is only enabled on a retry after 421 Misdirected Request
+  // is received. Alternative Services are also disabled in this case (though
+  // they can also be disabled when retrying after a QUIC error).
+  if (!enable_ip_based_pooling_)
+    DCHECK(!enable_alternative_services_);
   if (ForWebSocketHandshake()) {
     stream_request_.reset(
         session_->http_stream_factory_for_websocket()
@@ -1356,6 +1358,14 @@ int HttpNetworkTransaction::DoReadBodyComplete(int result) {
     // again in ~HttpNetworkTransaction.  Clean that up.
 
     // The next Read call will return 0 (EOF).
+
+    // This transaction was successful. If it had been retried because of an
+    // error with an alternative service, mark that alternative service broken.
+    if (!enable_alternative_services_ &&
+        retried_alternative_service_.protocol != kProtoUnknown) {

[ianswett, 2017/04/13 14:02:45]

To clarify, does this break any alt-svc, not just QUIC?

[Ryan Hamilton, 2017/04/13 17:26:56]

Yes, this code here is not QUIC specific so it would mark any alt-svc as broken.

That said, the only code that sets retried_alternative_service_ does so in the
QUIC case. So for the time being, this is "QUIC specific".

+      session_->http_server_properties()->MarkAlternativeServiceBroken(
+          retried_alternative_service_);
+    }
   }
 
   // Clear these to avoid leaving around old state.
@@ -1556,19 +1566,25 @@ int HttpNetworkTransaction::HandleIOError(int error) {
       ResetConnectionAndRequestForResend();
       error = OK;
       break;
-    case ERR_QUIC_PROTOCOL_ERROR: {
-      AlternativeService alternative_service;
-      if (GetResponseHeaders() == nullptr &&
-          stream_->GetAlternativeService(&alternative_service) &&
-          session_->http_server_properties()->IsAlternativeServiceBroken(
-              alternative_service)) {
+    case ERR_QUIC_PROTOCOL_ERROR:
+      if (GetResponseHeaders() != nullptr ||

[ianswett, 2017/04/13 14:02:45]

Can you add some comments here?  I believe I understand this, but I'm not as
familiar with this code as I'd like.

[Ryan Hamilton, 2017/04/13 17:26:56]

Done.

+          !stream_->GetAlternativeService(&retried_alternative_service_)) {
+        break;
+      }
+      if (session_->http_server_properties()->IsAlternativeServiceBroken(
+              retried_alternative_service_)) {
+        net_log_.AddEventWithNetErrorCode(
+            NetLogEventType::HTTP_TRANSACTION_RESTART_AFTER_ERROR, error);
+        ResetConnectionAndRequestForResend();
+        error = OK;
+      } else if (session_->params().retry_without_alt_svc_on_quic_errors) {
+        enable_alternative_services_ = false;
         net_log_.AddEventWithNetErrorCode(
             NetLogEventType::HTTP_TRANSACTION_RESTART_AFTER_ERROR, error);
         ResetConnectionAndRequestForResend();
         error = OK;
       }
       break;
-    }
     case ERR_MISDIRECTED_REQUEST:
       // If this is the second try, just give up.
       if (!enable_ip_based_pooling_ && !enable_alternative_services_)