Create histograms to track time-to-error values for different errors.

content/browser/loader/resource_dispatcher_host_impl.cc

Index: content/browser/loader/resource_dispatcher_host_impl.cc
diff --git a/content/browser/loader/resource_dispatcher_host_impl.cc b/content/browser/loader/resource_dispatcher_host_impl.cc
index d758f687987af444c94f1ed25d29cfbd25fc31d5..e42f8c6ead4b1b71a11b8df0746e24a9bded7b22 100644
--- a/content/browser/loader/resource_dispatcher_host_impl.cc
+++ b/content/browser/loader/resource_dispatcher_host_impl.cc
@@ -24,6 +24,7 @@
 #include "base/profiler/scoped_tracker.h"
 #include "base/stl_util.h"
 #include "base/third_party/dynamic_annotations/dynamic_annotations.h"
+#include "base/time/time.h"
 #include "content/browser/appcache/appcache_interceptor.h"
 #include "content/browser/appcache/chrome_appcache_service.h"
 #include "content/browser/cert_store_impl.h"
@@ -862,6 +863,37 @@ void ResourceDispatcherHostImpl::DidFinishLoading(ResourceLoader* loader) {
         "Net.ErrorCodesForMainFrame3",
         -loader->request()->status().error());
 
+    // Record time to success and error for the most common errors, and for
+    // the aggregate remainder errors.

[mmenke, 2015/05/12 18:23:26]

High level question:

What's the motivation for adding timing hooks here instead of in
https://code.google.com/p/chromium/codesearch#chromium/src/net/url_request/ur...

Looks like there's also a cancel time there.  The times here do include
redirects, and those don't, but adding yet another incomparable set of time
histograms.

I suppose I do want to get rid of the LOAD_MAIN_FRAME flag, and we can't only
filter out the non-main frame resources there without using it.

[Randy Smith (Not in Mondays), 2015/05/12 20:51:39]

Discussed on main CL thread; IMO no code changes came from discussion.

+    base::TimeDelta request_loading_time(
+        base::Time::Now() - loader->request()->request_time());
+    switch (loader->request()->status().error()) {

[mmenke, 2015/05/12 18:23:26]

Looking at error without looking at loader->request()->status().status() makes
me nervous.

[Randy Smith (Not in Mondays), 2015/05/12 20:51:39]

Believe main thread discussion left this as ok to be consistent wiht the
ErrorCodesForMainFrame3 histogram above.

+      case net::OK:
+        UMA_HISTOGRAM_LONG_TIMES(
+            "Net.RequestTimeToSuccess", request_loading_time);

[mmenke, 2015/05/12 18:23:26]

optional:  May want to add another dot in here, to make them stand out more.

Net.RequestTime.Blah

[Randy Smith (Not in Mondays), 2015/05/12 20:51:39]

Good idea; done.

+        break;
+      case net::ERR_ABORTED:

[mmenke, 2015/05/12 18:23:26]

We should include loader->request()->status().status() == CANCELED in this
block, too.

[Randy Smith (Not in Mondays), 2015/05/12 20:51:39]

Left as is for compatibility with ErrorCodesForMainFrame3 above as per main
thread discussion.

+        UMA_HISTOGRAM_LONG_TIMES(
+            "Net.RequestTimeToErrAborted", request_loading_time);
+        break;
+      case net::ERR_TIMED_OUT:
+        UMA_HISTOGRAM_LONG_TIMES(
+            "Net.RequestTimeToErrTimedOut", request_loading_time);
+        break;
+      case net::ERR_CONNECTION_RESET:
+        UMA_HISTOGRAM_LONG_TIMES(
+            "Net.RequestTimeToErrConnectionReset", request_loading_time);
+        break;
+      case net::ERR_INTERNET_DISCONNECTED:
+        UMA_HISTOGRAM_LONG_TIMES(
+            "Net.RequestTimeToErrInternetDisconnected", request_loading_time);
+        break;

[jkarlin, 2015/05/12 18:26:06]

Perhaps ERR_CONNECTION_REFUSED, ERR_CONNECTION_TIMED_OUT, and
ERR_NAME_NOT_RESOLVED as well? Those all seem to show up a fair bit in
Net.ErrorCodesForMainFrame3 UMA. 

[Randy Smith (Not in Mondays), 2015/05/12 20:51:39]

ERR_NAME_NOT_RESOLVED was an oversight on my part; added.  CONNECTION_REFUSED
doesn't seem that high to me (close to a tenth of
INTERNET_DISCONNECTED/NAME_NOT_RESOLVED in an android only everything UMA view)
and I can't find CONNECTION_TIMED_OUT at all (as opposed to TIMED_OUT, which I'm
already including).

I've also sorted these by name, which might have been the confusion with the
timed out errors.

+      default:
+        UMA_HISTOGRAM_LONG_TIMES(
+            "Net.RequestTimeToMiscError", request_loading_time);
+        break;
+    }
+
     if (loader->request()->url().SchemeIsCryptographic()) {
       if (loader->request()->url().host() == "www.google.com") {
         UMA_HISTOGRAM_SPARSE_SLOWLY("Net.ErrorCodesForHTTPSGoogleMainFrame2",