Add Net.ProxyScriptFetchSuccessDuration UMA to record how long PAC script fetches take.

net/proxy/proxy_script_fetcher_impl.cc

Index: net/proxy/proxy_script_fetcher_impl.cc
diff --git a/net/proxy/proxy_script_fetcher_impl.cc b/net/proxy/proxy_script_fetcher_impl.cc
index 290363be6e49cf4de509bbd7f0d075e062cb3c6d..6534150a471f14f03a32efc9ea21f399796d0ae0 100644
--- a/net/proxy/proxy_script_fetcher_impl.cc
+++ b/net/proxy/proxy_script_fetcher_impl.cc
@@ -7,6 +7,7 @@
 #include "base/compiler_specific.h"
 #include "base/location.h"
 #include "base/logging.h"
+#include "base/metrics/histogram.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/string_util.h"
 #include "base/thread_task_runner_handle.h"
@@ -133,6 +134,9 @@ int ProxyScriptFetcherImpl::Fetch(
     return OK;
   }
 
+  DCHECK(fetch_start_time_.is_null());
+  fetch_start_time_ = base::Time::Now();

[eroman, 2015/07/13 21:39:04]

I suggest using base::TimeTicks over base::Time for measuring elapsed times.

base::Time is influenced from changing the system clock, so it is more likely to
measure walltime deltas incorrectly.

While base::TimeTicks is better in this regard, it has inconsistencies with how
time lapses measured across system sleep/suspend work, because unfortunately
last I looked we didn't use a consistent monotonic clock across platforms (ticks
will stand still on some platforms, but increase on others while sleeping).

... Even so, that is not likely to impact this histogram since fetches
interrupted by a suspend/sleep will almost surely fail (either aborted or
network error).

+
   cur_request_ =
       url_request_context_->CreateRequest(url, DEFAULT_PRIORITY, this);
   cur_request_->set_method("GET");
@@ -282,6 +286,11 @@ bool ProxyScriptFetcherImpl::ConsumeBytesRead(URLRequest* request,
 
 void ProxyScriptFetcherImpl::FetchCompleted() {
   if (result_code_ == OK) {
+    // Calculate duration of time for proxy script fetch to complete.
+    DCHECK(!fetch_start_time_.is_null());
+    UMA_HISTOGRAM_TIMES("Net.ProxyScriptFetcher.SuccessDuration",

[eroman, 2015/07/13 21:39:04]

I see you considered putting it in ProxyScriptDecider. This should be fine to
start getting some data.

Note that an advantage of ProxyScriptDecider is we might be rejecting slow
successful fetches in ProxyScriptDecider::DoVerifyPacScript().

For instance a hypothetical situation would be a captive portal that makes the
proxy script fetch really slow, however it still completes with an HTTP 200
code. The page served up however is an HTML page rather than a PAC script, so
DoVerifyPacScript() subsequently fails and we fallback to not using the script.

+                        base::Time::Now() - fetch_start_time_);
+
     // The caller expects the response to be encoded as UTF16.
     std::string charset;
     cur_request_->GetCharset(&charset);
@@ -305,6 +314,7 @@ void ProxyScriptFetcherImpl::ResetCurRequestState() {
   callback_.Reset();
   result_code_ = OK;
   result_text_ = NULL;
+  fetch_start_time_ = base::Time();
 }
 
 void ProxyScriptFetcherImpl::OnTimeout(int id) {