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

Side by Side Diff: net/http/http_network_transaction.cc

Issue 1116063006: Only record fallback metrics on successful requests. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: sleevi comments Created 5 years, 7 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 unified diff | Download patch
« no previous file with comments | « net/http/http_network_transaction.h ('k') | net/http/http_stream_factory_impl_job.cc » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "net/http/http_network_transaction.h" 5 #include "net/http/http_network_transaction.h"
6 6
7 #include <set> 7 #include <set>
8 #include <vector> 8 #include <vector>
9 9
10 #include "base/bind.h" 10 #include "base/bind.h"
(...skipping 768 matching lines...) Expand 10 before | Expand all | Expand 10 after
779 "424359 HttpNetworkTransaction::DoCreateStreamComplete")); 779 "424359 HttpNetworkTransaction::DoCreateStreamComplete"));
780 780
781 // If |result| is ERR_HTTPS_PROXY_TUNNEL_RESPONSE, then 781 // If |result| is ERR_HTTPS_PROXY_TUNNEL_RESPONSE, then
782 // DoCreateStreamComplete is being called from OnHttpsProxyTunnelResponse, 782 // DoCreateStreamComplete is being called from OnHttpsProxyTunnelResponse,
783 // which resets the stream request first. Therefore, we have to grab the 783 // which resets the stream request first. Therefore, we have to grab the
784 // connection attempts in *that* function instead of here in that case. 784 // connection attempts in *that* function instead of here in that case.
785 if (result != ERR_HTTPS_PROXY_TUNNEL_RESPONSE) 785 if (result != ERR_HTTPS_PROXY_TUNNEL_RESPONSE)
786 CopyConnectionAttemptsFromStreamRequest(); 786 CopyConnectionAttemptsFromStreamRequest();
787 787
788 if (result == OK) { 788 if (result == OK) {
789 if (request_->url.SchemeIsCryptographic())
790 RecordSSLFallbackMetrics();
789 next_state_ = STATE_INIT_STREAM; 791 next_state_ = STATE_INIT_STREAM;
790 DCHECK(stream_.get()); 792 DCHECK(stream_.get());
791 } else if (result == ERR_SSL_CLIENT_AUTH_CERT_NEEDED) { 793 } else if (result == ERR_SSL_CLIENT_AUTH_CERT_NEEDED) {
792 result = HandleCertificateRequest(result); 794 result = HandleCertificateRequest(result);
793 } else if (result == ERR_HTTPS_PROXY_TUNNEL_RESPONSE) { 795 } else if (result == ERR_HTTPS_PROXY_TUNNEL_RESPONSE) {
794 // Return OK and let the caller read the proxy's error page 796 // Return OK and let the caller read the proxy's error page
795 next_state_ = STATE_NONE; 797 next_state_ = STATE_NONE;
796 return OK; 798 return OK;
797 } else if (result == ERR_HTTP_1_1_REQUIRED || 799 } else if (result == ERR_HTTP_1_1_REQUIRED ||
798 result == ERR_PROXY_HTTP_1_1_REQUIRED) { 800 result == ERR_PROXY_HTTP_1_1_REQUIRED) {
(...skipping 724 matching lines...) Expand 10 before | Expand all | Expand 10 after
1523 1525
1524 pending_auth_target_ = HttpAuth::AUTH_NONE; 1526 pending_auth_target_ = HttpAuth::AUTH_NONE;
1525 read_buf_ = NULL; 1527 read_buf_ = NULL;
1526 read_buf_len_ = 0; 1528 read_buf_len_ = 0;
1527 headers_valid_ = false; 1529 headers_valid_ = false;
1528 request_headers_.Clear(); 1530 request_headers_.Clear();
1529 response_ = HttpResponseInfo(); 1531 response_ = HttpResponseInfo();
1530 establishing_tunnel_ = false; 1532 establishing_tunnel_ = false;
1531 } 1533 }
1532 1534
1535 void HttpNetworkTransaction::RecordSSLFallbackMetrics() {
1536 // Note: these values are used in histograms, so new values must be appended.
1537 enum FallbackVersion {
1538 FALLBACK_NONE = 0, // SSL version fallback did not occur.
1539 FALLBACK_SSL3 = 1, // Fell back to SSL 3.0.
1540 FALLBACK_TLS1 = 2, // Fell back to TLS 1.0.
1541 FALLBACK_TLS1_1 = 3, // Fell back to TLS 1.1.
1542 FALLBACK_MAX,
1543 };
1544
1545 FallbackVersion fallback = FALLBACK_NONE;
1546 if (server_ssl_config_.version_fallback) {
1547 switch (server_ssl_config_.version_max) {
1548 case SSL_PROTOCOL_VERSION_SSL3:
1549 fallback = FALLBACK_SSL3;
1550 break;
1551 case SSL_PROTOCOL_VERSION_TLS1:
1552 fallback = FALLBACK_TLS1;
1553 break;
1554 case SSL_PROTOCOL_VERSION_TLS1_1:
1555 fallback = FALLBACK_TLS1_1;
1556 break;
1557 default:
1558 NOTREACHED();
1559 }
1560 }
1561 UMA_HISTOGRAM_ENUMERATION("Net.ConnectionUsedSSLVersionFallback2", fallback,
1562 FALLBACK_MAX);
1563
1564 // We also wish to measure the amount of fallback connections for a host that
1565 // we know implements TLS up to 1.2. Ideally there would be no fallback here
1566 // but high numbers of SSLv3 would suggest that SSLv3 fallback is being
1567 // caused by network middleware rather than buggy HTTPS servers.
Ryan Sleevi 2015/05/02 00:37:34 Update this comment to reflect your explanation th
davidben 2015/05/04 17:51:17 Done.
1568 const std::string& host = request_->url.host();
1569 if (EndsWith(host, "google.com", true) &&
1570 (host.size() == 10 || host[host.size() - 11] == '.')) {
1571 UMA_HISTOGRAM_ENUMERATION("Net.GoogleConnectionUsedSSLVersionFallback2",
1572 fallback, FALLBACK_MAX);
1573 }
1574
1575 UMA_HISTOGRAM_BOOLEAN("Net.ConnectionUsedSSLDeprecatedCipherFallback2",
1576 server_ssl_config_.enable_deprecated_cipher_suites);
1577 }
1578
1533 HttpResponseHeaders* HttpNetworkTransaction::GetResponseHeaders() const { 1579 HttpResponseHeaders* HttpNetworkTransaction::GetResponseHeaders() const {
1534 return response_.headers.get(); 1580 return response_.headers.get();
1535 } 1581 }
1536 1582
1537 bool HttpNetworkTransaction::ShouldResendRequest() const { 1583 bool HttpNetworkTransaction::ShouldResendRequest() const {
1538 bool connection_is_proven = stream_->IsConnectionReused(); 1584 bool connection_is_proven = stream_->IsConnectionReused();
1539 bool has_received_headers = GetResponseHeaders() != NULL; 1585 bool has_received_headers = GetResponseHeaders() != NULL;
1540 1586
1541 // NOTE: we resend a request only if we reused a keep-alive connection. 1587 // NOTE: we resend a request only if we reused a keep-alive connection.
1542 // This automatically prevents an infinite resend loop because we'll run 1588 // This automatically prevents an infinite resend loop because we'll run
(...skipping 134 matching lines...) Expand 10 before | Expand all | Expand 10 after
1677 DCHECK(stream_request_); 1723 DCHECK(stream_request_);
1678 1724
1679 // Since the transaction can restart with auth credentials, it may create a 1725 // Since the transaction can restart with auth credentials, it may create a
1680 // stream more than once. Accumulate all of the connection attempts across 1726 // stream more than once. Accumulate all of the connection attempts across
1681 // those streams by appending them to the vector: 1727 // those streams by appending them to the vector:
1682 for (const auto& attempt : stream_request_->connection_attempts()) 1728 for (const auto& attempt : stream_request_->connection_attempts())
1683 connection_attempts_.push_back(attempt); 1729 connection_attempts_.push_back(attempt);
1684 } 1730 }
1685 1731
1686 } // namespace net 1732 } // namespace net
OLDNEW
« no previous file with comments | « net/http/http_network_transaction.h ('k') | net/http/http_stream_factory_impl_job.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698