Add a dedicated SSL protocol version metric.

net/socket/ssl_client_socket_pool.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/socket/ssl_client_socket_pool.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
@@ skipping 483 matching lines @@
 
     UMA_HISTOGRAM_CUSTOM_TIMES("Net.SSL_Connection_Latency_2",
                                connect_duration,
                                base::TimeDelta::FromMilliseconds(1),
                                base::TimeDelta::FromMinutes(1),
                                100);
 
     SSLInfo ssl_info;
     ssl_socket_->GetSSLInfo(&ssl_info);
 
+    UMA_HISTOGRAM_ENUMERATION(

[Ryan Sleevi, 2015/02/19 19:24:02]

BUG: You're now not histograming for HPKP mismatches.

[davidben, 2015/02/19 19:32:20]

Is that a problem? Seems like a bug in the old code. We're also not measuring
Net.SSL_CipherSuite for such connections either. If the connection wasn't going
to be  allowed anyway, we probably don't want to count it was succeeding under
some version. The original metrics predate key pinning.

(If the user later clicks through---HPKP not allowing click-through
aside---there'll be a new connection and we'll count it there.)

+        "Net.SSL_Version",
+        SSLConnectionStatusToVersion(ssl_info.connection_status),
+        SSL_CONNECTION_VERSION_MAX);

[Ilya Sherman, 2015/02/19 20:18:35]

It looks like you're re-using an enum that's currently used outside of UMA. 
Please either annotate that enum documenting that it's used to back an UMA
histogram, and hence should be treated as append-only; or create a new enum
specifically to back the UMA histogram.

[davidben, 2015/02/19 21:22:34]

Done.

Ryan: I figured it would be okay to stabilize these flags since they're already
persisted to disk. But conceivably we could version them on disk. Do you agree
that this is fine?

[Ryan Sleevi, 2015/02/19 22:18:51]

Yeah.

+
     UMA_HISTOGRAM_SPARSE_SLOWLY("Net.SSL_CipherSuite",
                                 SSLConnectionStatusToCipherSuite(
                                     ssl_info.connection_status));
 
     UMA_HISTOGRAM_BOOLEAN(
         "Net.RenegotiationExtensionSupported",
         (ssl_info.connection_status &
          SSL_CONNECTION_NO_RENEGOTIATION_EXTENSION) == 0);
 
     if (ssl_info.handshake_type == SSLInfo::HANDSHAKE_RESUME) {
@@ skipping 334 matching lines @@
   CHECK(it != messenger_map_.end());
   delete it->second;
   messenger_map_.erase(it);
 }
 
 void SSLClientSocketPool::OnSSLConfigChanged() {
   FlushWithError(ERR_NETWORK_CHANGED);
 }
 
 }  // namespace net