When HttpNetworkTransaction encounters QUIC errors, retry the request

net/http/http_network_transaction.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/http/http_network_transaction.h"
 
 #include <memory>
 #include <set>
 #include <utility>
 #include <vector>
@@ skipping 833 matching lines @@
 
 int HttpNetworkTransaction::DoCreateStream() {
   // TODO(mmenke): Remove ScopedTracker below once crbug.com/424359 is fixed.
   tracked_objects::ScopedTracker tracking_profile(
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "424359 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()
             ->RequestWebSocketHandshakeStream(
                 *request_, priority_, server_ssl_config_, proxy_ssl_config_,
                 this, websocket_handshake_stream_base_create_helper_,
                 enable_ip_based_pooling_, enable_alternative_services_,
                 net_log_));
   } else {
     stream_request_.reset(session_->http_stream_factory()->RequestStream(
@@ skipping 482 matching lines @@
         stream_->IsResponseBodyComplete() && stream_->CanReuseConnection();
 
     stream_->Close(!keep_alive);
     // Note: we don't reset the stream here.  We've closed it, but we still
     // need it around so that callers can call methods such as
     // GetUploadProgress() and have them be meaningful.
     // TODO(mbelshe): This means we closed the stream here, and we close it
     // 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.
   read_buf_ = NULL;
   read_buf_len_ = 0;
 
   return result;
 }
 
 int HttpNetworkTransaction::DoDrainBodyForAuthRestart() {
@@ skipping 180 matching lines @@
       }
       break;
     case ERR_SPDY_PING_FAILED:
     case ERR_SPDY_SERVER_REFUSED_STREAM:
     case ERR_QUIC_HANDSHAKE_FAILED:
       net_log_.AddEventWithNetErrorCode(
           NetLogEventType::HTTP_TRANSACTION_RESTART_AFTER_ERROR, 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_)
         return OK;
       // Otherwise retry the request with both IP based pooling
       // and Alternative Services disabled.
       enable_ip_based_pooling_ = false;
       enable_alternative_services_ = false;
       net_log_.AddEventWithNetErrorCode(
           NetLogEventType::HTTP_TRANSACTION_RESTART_AFTER_ERROR, error);
@@ skipping 200 matching lines @@
       continue;
     if (allowed_encodings.find(encoding) == allowed_encodings.end()) {
       FilterSourceStream::ReportContentDecodingFailed(source_type);
       return false;
     }
   }
   return true;
 }
 
 }  // namespace net