Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(331)

Unified Diff: net/quic/chromium/quic_http_stream.cc

Issue 2789093003: Mark QUIC broken when the network blackholes after the handshake (Closed)
Patch Set: Created 3 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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 3b031b93f7ce6d1d9a74d68f82965c9f39d87734..ac1fae17037aa7ad4d649c235de8e12915ac0b22 100644
--- a/net/quic/chromium/quic_http_stream.cc
+++ b/net/quic/chromium/quic_http_stream.cc
@@ -46,11 +46,15 @@ std::unique_ptr<base::Value> NetLogQuicPushStreamCallback(
QuicHttpStream::QuicHttpStream(
const base::WeakPtr<QuicChromiumClientSession>& session,
- HttpServerProperties* http_server_properties)
+ HttpServerProperties* http_server_properties,
+ bool mark_quic_broken_when_network_suspected)
: MultiplexedHttpStream(MultiplexedSessionHandle(session)),
next_state_(STATE_NONE),
session_(session),
+ server_id_(session->server_id()),
http_server_properties_(http_server_properties),
+ mark_quic_broken_when_network_suspected_(
+ mark_quic_broken_when_network_suspected),
quic_version_(session->GetQuicVersion()),
session_error_(ERR_UNEXPECTED),
was_handshake_confirmed_(session->IsCryptoHandshakeConfirmed()),
@@ -320,7 +324,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;
@@ -362,6 +365,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_) {
@@ -857,8 +861,8 @@ void QuicHttpStream::SetResponseStatus(int response_status) {
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;
@@ -872,6 +876,31 @@ 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 (!mark_quic_broken_when_network_suspected_)
+ return ERR_QUIC_PROTOCOL_ERROR;
+
+ // For error codes which could have been generated by "the network", mark
+ // QUIC as broken and return ERR_QUIC_BROKEN_ERROR to permit
+ // HttpNetworkTransaction to retry the request over TCP.
+ if (quic_connection_error_ == QUIC_NETWORK_IDLE_TIMEOUT ||
+ quic_connection_error_ == QUIC_TIMEOUTS_WITH_OPEN_STREAMS ||
+ quic_connection_error_ == QUIC_TOO_MANY_RTOS) {
+ HistogramBrokenAlternateProtocolLocation(
+ BROKEN_ALTERNATE_PROTOCOL_LOCATION_QUIC_HTTP_STREAM);
+ http_server_properties_->MarkAlternativeServiceBroken(
+ AlternativeService(kProtoQUIC, server_id_.host(), server_id_.port()));
+ return ERR_QUIC_BROKEN_ERROR;
+ }
+
+ // All other errors are fatal.
return ERR_QUIC_PROTOCOL_ERROR;
}

Powered by Google App Engine
This is Rietveld 408576698