Commit fab11cdb authored by jdoerrie's avatar jdoerrie Committed by Commit bot

Revert of When HttpNetworkTransaction encounters QUIC errors, retry the...

Revert of When HttpNetworkTransaction encounters QUIC errors, retry the request (patchset #3 id:40001 of https://codereview.chromium.org/2818623002/ )

Reason for revert:
Likely cause of compilation error in Win x64.

BUG=711316

Original issue's description:
> When HttpNetworkTransaction encounters QUIC errors, retry the request
> with alt-svc disabled, and if that succeeds then mark QUIC as broken.
>
> Protected by the retry_without_alt_svc_on_quic_errors finch param.
>
> BUG=705033
>
> Review-Url: https://codereview.chromium.org/2818623002
> Cr-Commit-Position: refs/heads/master@{#464391}
> Committed: https://chromium.googlesource.com/chromium/src/+/d0dbccf1f8d23dfa32defdd02c0a3df990729358

TBR=jri@chromium.org,ianswett@chromium.org,rch@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=705033

Review-Url: https://codereview.chromium.org/2811993005
Cr-Commit-Position: refs/heads/master@{#464403}
parent 7bd88f51
......@@ -115,14 +115,6 @@ bool ShouldMarkQuicBrokenWhenNetworkBlackholes(
"true");
}
bool ShouldRetryWithoutAltSvcOnQuicErrors(
const VariationParameters& quic_trial_params) {
return base::LowerCaseEqualsASCII(
GetVariationParam(quic_trial_params,
"retry_without_alt_svc_on_quic_errors"),
"true");
}
bool ShouldQuicDisableConnectionPooling(
const VariationParameters& quic_trial_params) {
return base::LowerCaseEqualsASCII(
......@@ -343,8 +335,7 @@ void ConfigureQuicParams(base::StringPiece quic_trial_group,
is_quic_force_enabled);
params->mark_quic_broken_when_network_blackholes =
ShouldMarkQuicBrokenWhenNetworkBlackholes(quic_trial_params);
params->retry_without_alt_svc_on_quic_errors =
ShouldRetryWithoutAltSvcOnQuicErrors(quic_trial_params);
params->enable_quic_alternative_service_with_different_host =
ShouldQuicEnableAlternativeServicesForDifferentHost(quic_trial_params);
......
......@@ -68,7 +68,6 @@ TEST_F(NetworkSessionConfiguratorTest, EnableQuicFromFieldTrialGroup) {
EXPECT_TRUE(params_.enable_quic);
EXPECT_FALSE(params_.mark_quic_broken_when_network_blackholes);
EXPECT_FALSE(params_.retry_without_alt_svc_on_quic_errors);
EXPECT_EQ(1350u, params_.quic_max_packet_length);
EXPECT_EQ(net::QuicTagVector(), params_.quic_connection_options);
EXPECT_FALSE(params_.quic_always_require_handshake_confirmation);
......@@ -134,17 +133,6 @@ TEST_F(NetworkSessionConfiguratorTest,
EXPECT_TRUE(params_.mark_quic_broken_when_network_blackholes);
}
TEST_F(NetworkSessionConfiguratorTest, RetryWithoutAltSvcOnQuicErrors) {
std::map<std::string, std::string> field_trial_params;
field_trial_params["retry_without_alt_svc_on_quic_errors"] = "true";
variations::AssociateVariationParams("QUIC", "Enabled", field_trial_params);
base::FieldTrialList::CreateFieldTrial("QUIC", "Enabled");
ParseFieldTrials();
EXPECT_TRUE(params_.retry_without_alt_svc_on_quic_errors);
}
TEST_F(NetworkSessionConfiguratorTest,
QuicCloseSessionsOnIpChangeFromFieldTrialParams) {
std::map<std::string, std::string> field_trial_params;
......
......@@ -127,7 +127,6 @@ HttpNetworkSession::Params::Params()
enable_quic_alternative_service_with_different_host(true),
enable_quic(false),
mark_quic_broken_when_network_blackholes(false),
retry_without_alt_svc_on_quic_errors(false),
quic_always_require_handshake_confirmation(false),
quic_disable_connection_pooling(false),
quic_load_server_info_timeout_srtt_multiplier(0.25f),
......
......@@ -122,9 +122,6 @@ class NET_EXPORT HttpNetworkSession
// Marks a QUIC server broken when a connection blackholes after the
// handshake is confirmed.
bool mark_quic_broken_when_network_blackholes;
// Retry requests which fail with QUIC_PROTOCOL_ERROR, and mark QUIC
// broken if the retry succeeds.
bool retry_without_alt_svc_on_quic_errors;
// Disables QUIC's 0-RTT behavior.
bool quic_always_require_handshake_confirmation;
// Disables QUIC connection pooling.
......
......@@ -851,11 +851,9 @@ int HttpNetworkTransaction::DoCreateStream() {
response_.network_accessed = true;
next_state_ = STATE_CREATE_STREAM_COMPLETE;
// 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_);
// 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_);
if (ForWebSocketHandshake()) {
stream_request_.reset(
session_->http_stream_factory_for_websocket()
......@@ -1358,14 +1356,6 @@ 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) {
session_->http_server_properties()->MarkAlternativeServiceBroken(
retried_alternative_service_);
}
}
// Clear these to avoid leaving around old state.
......@@ -1566,25 +1556,19 @@ int HttpNetworkTransaction::HandleIOError(int error) {
ResetConnectionAndRequestForResend();
error = OK;
break;
case ERR_QUIC_PROTOCOL_ERROR:
if (GetResponseHeaders() != nullptr ||
!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;
case ERR_QUIC_PROTOCOL_ERROR: {
AlternativeService alternative_service;
if (GetResponseHeaders() == nullptr &&
stream_->GetAlternativeService(&alternative_service) &&
session_->http_server_properties()->IsAlternativeServiceBroken(
alternative_service)) {
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_)
......
......@@ -380,10 +380,6 @@ class NET_EXPORT_PRIVATE HttpNetworkTransaction
// Enable using alternative services for the request.
bool enable_alternative_services_;
// When a request is retried because of errors with the alternative service,
// this will store the alternative service used.
AlternativeService retried_alternative_service_;
// The helper object to use to create WebSocketHandshakeStreamBase
// objects. Only relevant when establishing a WebSocket connection.
WebSocketHandshakeStreamBase::CreateHelper*
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment