Add revocation checking field trial.

net/socket/ssl_client_socket_pool.cc

 // Copyright (c) 2011 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/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
 #include "base/values.h"
 #include "net/base/net_errors.h"
@@ skipping 311 matching lines @@
       ssl_socket_->IgnoreCertError(result, params_->load_flags())) {
     DCHECK(ssl_connect_start_time_ != base::TimeTicks());
     base::TimeDelta connect_duration =
         base::TimeTicks::Now() - ssl_connect_start_time_;
     if (using_spdy) {
       UMA_HISTOGRAM_CUSTOM_TIMES("Net.SpdyConnectionLatency",
                                  connect_duration,
                                  base::TimeDelta::FromMilliseconds(1),
                                  base::TimeDelta::FromMinutes(10),
                                  100);
-    } else {
-      UMA_HISTOGRAM_CUSTOM_TIMES("Net.SSL_Connection_Latency",
+    }
+
+    UMA_HISTOGRAM_CUSTOM_TIMES("Net.SSL_Connection_Latency",

[Mike Belshe, 2011/06/08 04:50:38]

note:  you're changing the definition of these histograms... Before they were
"!spdy", but now they are everything - spdy or not.  I think this is ok....

[agl, 2011/06/08 18:57:12]

Yes, that's was intentional.

+                               connect_duration,
+                               base::TimeDelta::FromMilliseconds(1),
+                               base::TimeDelta::FromMinutes(10),
+                               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_Google",
                                  connect_duration,
                                  base::TimeDelta::FromMilliseconds(1),
                                  base::TimeDelta::FromMinutes(10),
                                  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_Google",
-                                   connect_duration,
-                                   base::TimeDelta::FromMilliseconds(1),
-                                   base::TimeDelta::FromMinutes(10),
-                                   100);
-      }
+    static const bool rev_checking_trial =
+        base::FieldTrialList::TrialExists("RevCheckingImpact");
+    if (rev_checking_trial) {

[Mike Belshe, 2011/06/08 04:50:38]

I think this is a problem.

You will compare:
a) sites which could have been pinned but which we overrode.
to 
b) all other sites that could have been pinned or not

[agl, 2011/06/08 18:57:12]

Yes, I meant to put the histogram in the |is_google| condition. However, another
case occurred: a user could be outside the trail but have manually disabled
revocation checking. Please see the code now that I've addressed that and check
that my assumptions about field trials are correct.

+      UMA_HISTOGRAM_CUSTOM_TIMES(base::FieldTrial::MakeName(
+                                     "Net.SSL_Connection_Latency_Google",
+                                     "RevCheckingImpact"),
+                                 connect_duration,
+                                 base::TimeDelta::FromMilliseconds(1),
+                                 base::TimeDelta::FromMinutes(10),
+                                 100);
     }
   }
 
   if (result == OK || IsCertificateError(result)) {
     set_socket(ssl_socket_.release());
   } else if (result == ERR_SSL_CLIENT_AUTH_CERT_NEEDED) {
     error_response_info_.cert_request_info = new SSLCertRequestInfo;
     ssl_socket_->GetSSLCertRequestInfo(error_response_info_.cert_request_info);
   }
 
@@ skipping 201 matching lines @@
 
 ClientSocketPoolHistograms* SSLClientSocketPool::histograms() const {
   return base_.histograms();
 }
 
 void SSLClientSocketPool::OnSSLConfigChanged() {
   Flush();
 }
 
 }  // namespace net