Mark QUIC broken when the network blackholes after the handshake

net/quic/chromium/quic_http_stream.cc

Index: net/quic/chromium/quic_http_stream.cc
diff --git a/net/quic/chromium/quic_http_stream.cc b/net/quic/chromium/quic_http_stream.cc
index 4debbec04ae9181168f55064813d1167476bc7ee..cc08ee3b9dbac4d44ba486bb578aec42c808b952 100644
--- a/net/quic/chromium/quic_http_stream.cc
+++ b/net/quic/chromium/quic_http_stream.cc
@@ -50,6 +50,7 @@ QuicHttpStream::QuicHttpStream(
     : MultiplexedHttpStream(MultiplexedSessionHandle(session)),
       next_state_(STATE_NONE),
       session_(session),
+      server_id_(session->server_id()),
       http_server_properties_(http_server_properties),
       quic_version_(session->GetQuicVersion()),
       session_error_(ERR_UNEXPECTED),
@@ -322,7 +323,6 @@ int QuicHttpStream::ReadResponseHeaders(const CompletionCallback& callback) {
 
   if (stream_ == nullptr)
     return GetResponseStatus();
-
   // Check if we already have the response headers. If so, return synchronously.
   if (response_headers_received_)
     return OK;
@@ -364,6 +364,7 @@ int QuicHttpStream::ReadResponseBody(IOBuffer* buf,
 }
 
 void QuicHttpStream::Close(bool /*not_reusable*/) {
+  session_error_ = ERR_ABORTED;
   SaveResponseStatus();
   // Note: the not_reusable flag has no meaning for QUIC streams.
   if (stream_) {
@@ -856,11 +857,20 @@ void QuicHttpStream::SetResponseStatus(int response_status) {
   response_status_ = response_status;
 }
 
+// Returns true if |error| is possibly caused by the network
+bool IsPossiblyNetworkError(QuicErrorCode error) {

[Jana, 2017/04/07 21:59:29]

You don't need this function anymore, do you?

[Ryan Hamilton, 2017/04/07 22:37:20]

Right! Good point. I should move this comment to the session though.

Thanks. DOne.

+  // QUIC connections should not timeout while there are open streams,
+  // since PING frames are sent to prevent timeouts. If, however, the
+  // connection timed out while this stream was open then QUIC traffic
+  // has become blackholed.
+  return error == QUIC_NETWORK_IDLE_TIMEOUT || error == QUIC_TOO_MANY_RTOS;
+}
+
 int QuicHttpStream::ComputeResponseStatus() const {
   DCHECK(!has_response_status_);
 
-  // If the handshake has failed this will be handled by the
-  // QuicStreamFactory and HttpStreamFactory to mark QUIC as broken.
+  // If the handshake has failed this will be handled by the QuicStreamFactory
+  // and HttpStreamFactory to mark QUIC as broken if TCP is actually working.
   if (!was_handshake_confirmed_)
     return ERR_QUIC_HANDSHAKE_FAILED;
 
@@ -874,6 +884,23 @@ int QuicHttpStream::ComputeResponseStatus() const {
   if (!response_info_)
     return ERR_CONNECTION_CLOSED;
 
+  // Explicit stream error are always fatal.
+  if (quic_stream_error_ != QUIC_STREAM_NO_ERROR &&
+      quic_stream_error_ != QUIC_STREAM_CONNECTION_ERROR) {
+    return ERR_QUIC_PROTOCOL_ERROR;
+  }
+
+  DCHECK_NE(QUIC_HANDSHAKE_TIMEOUT, quic_connection_error_);
+
+  // If the headers have not been received and QUIC is now broken, return
+  // ERR_QUIC_BROKEN_ERROR to permit HttpNetworkTransaction to retry the request
+  // over TCP.
+  if (!response_headers_received_ &&
+      http_server_properties_->IsAlternativeServiceBroken(AlternativeService(
+          kProtoQUIC, server_id_.host(), server_id_.port()))) {
+    return ERR_QUIC_BROKEN_ERROR;
+  }
+
   return ERR_QUIC_PROTOCOL_ERROR;
 }