Adding new SSL Connection Error/Latency metrics

net/socket/ssl_client_socket_pool.cc

Index: net/socket/ssl_client_socket_pool.cc
diff --git a/net/socket/ssl_client_socket_pool.cc b/net/socket/ssl_client_socket_pool.cc
index f422a7679474b326bd14741c448819ed5c3cd306..fba4bbeb61d761a190071ae405eee2130b911231 100644
--- a/net/socket/ssl_client_socket_pool.cc
+++ b/net/socket/ssl_client_socket_pool.cc
@@ -346,6 +346,17 @@ int SSLConnectJob::DoSSLConnectComplete(int result) {
     return ERR_ALPN_NEGOTIATION_FAILED;
   }
 
+  const std::string& host = params_->host_and_port().host();
+  bool is_google =
+      host == "google.com" ||
+      (host.size() > 11 && host.rfind(".google.com") == host.size() - 11);
+
+  // These are hosts that we expect to always offer TLS 1.3 Connections to

[davidben, 2017/03/07 19:51:13]

Period after TLS 1.3.

[svaldez, 2017/03/07 20:06:59]

Done.

+  // them, whether or not this browser is in the experiment group, form the
+  // basis of our comparisons.

[davidben, 2017/03/07 19:51:13]

Not sure if this comment is true yet. Perhaps:

These are hosts that we intend to use in the initial TLS 1.3 deployment. TLS
connections to them, whether or not this browser is in the experiment group,
form the basis of our comparisons.

[svaldez, 2017/03/07 20:06:59]

Done.

+  bool tls13_supported =
+      (host == "drive.google.com" || host == "mail.google.com");
+
   if (result == OK ||
       ssl_socket_->IgnoreCertError(result, params_->load_flags())) {
     DCHECK(!connect_timing_.ssl_start.is_null());
@@ -396,10 +407,6 @@ int SSLConnectJob::DoSSLConnectComplete(int result) {
                                  100);
     }
 
-    const std::string& host = params_->host_and_port().host();
-    bool is_google =
-        host == "google.com" ||
-        (host.size() > 11 && host.rfind(".google.com") == host.size() - 11);
     if (is_google) {
       UMA_HISTOGRAM_CUSTOM_TIMES("Net.SSL_Connection_Latency_Google2",
                                  connect_duration,
@@ -422,10 +429,27 @@ int SSLConnectJob::DoSSLConnectComplete(int result) {
                                    100);
       }
     }
+
+    if (tls13_supported) {
+      UMA_HISTOGRAM_CUSTOM_TIMES("Net.SSL_Connection_Latency_TLS13",

[davidben, 2017/03/07 19:51:13]

TLS13 is going to get confusing in the future since it reads like we're
measuring all TLS 1.3 servers. TLS13Experiment?

[svaldez, 2017/03/07 20:06:59]

Done.

+                                 connect_duration,
+                                 base::TimeDelta::FromMilliseconds(1),
+                                 base::TimeDelta::FromMinutes(1), 100);
+    }
   }
 
   UMA_HISTOGRAM_SPARSE_SLOWLY("Net.SSL_Connection_Error", std::abs(result));
 
+  if (is_google) {
+    UMA_HISTOGRAM_SPARSE_SLOWLY("Net.SSL_Connection_Error_Google",
+                                std::abs(result));
+  }
+
+  if (tls13_supported) {
+    UMA_HISTOGRAM_SPARSE_SLOWLY("Net.SSL_Connection_Error_TLS13",

[davidben, 2017/03/07 19:51:13]

Ditto.

[svaldez, 2017/03/07 20:06:59]

Done.

[svaldez, 2017/03/07 20:06:59]

Done.

+                                std::abs(result));
+  }
+
   if (result == OK || IsCertificateError(result)) {
     SetSocket(std::move(ssl_socket_));
   } else if (result == ERR_SSL_CLIENT_AUTH_CERT_NEEDED) {