Add metrics for DHCP WPAD feature.

net/proxy/dhcp_proxy_script_fetcher_win.cc

Index: net/proxy/dhcp_proxy_script_fetcher_win.cc
diff --git a/net/proxy/dhcp_proxy_script_fetcher_win.cc b/net/proxy/dhcp_proxy_script_fetcher_win.cc
index 35109f608647e7a53974c190a4416aa705efdd94..d1e8dc2ac96488d77509fecd7f15b5ec5c3402d8 100644
--- a/net/proxy/dhcp_proxy_script_fetcher_win.cc
+++ b/net/proxy/dhcp_proxy_script_fetcher_win.cc
@@ -4,6 +4,8 @@
 
 #include "net/proxy/dhcp_proxy_script_fetcher_win.h"
 
+#include "base/metrics/histogram.h"
+#include "base/perftimer.h"
 #include "net/base/net_errors.h"
 #include "net/proxy/dhcp_proxy_script_adapter_fetcher_win.h"
 
@@ -18,6 +20,41 @@ namespace {
 // adapter finishes first.
 const int kMaxWaitAfterFirstResultMs = 400;
 
+const unsigned long kGetAdaptersAddressesErrors[] = {
+  ERROR_ADDRESS_NOT_ASSOCIATED,
+  ERROR_BUFFER_OVERFLOW,
+  ERROR_INVALID_PARAMETER,
+  ERROR_NOT_ENOUGH_MEMORY,
+  ERROR_NO_DATA,
+
+  // We use this as a placeholder (in our result code UMA histogram)
+  // for any error we don't specifically include in this enumeration.
+  ERROR_GEN_FAILURE,
+};
+
+std::vector<int> GetAllGetAdaptersAddressesErrors() {
+  std::vector<int> errors;
+  for (size_t i = 0; i < arraysize(kGetAdaptersAddressesErrors); ++i) {
+    int error_code = kGetAdaptersAddressesErrors[i];
+    errors.push_back(error_code);
+    // Also add N+1 for each error, so the bucket that contains our expected
+    // error is of size 1. That way if we get unexpected error codes, they
+    // won't fall into the same buckets as the expected ones.
+    errors.push_back(error_code + 1);
+  }
+  return errors;
+}
+
+// Returns the passed-in error code if it is in kGetAdaptersAddressesErrors,
+// otherwise ERROR_GEN_FAILURE.
+unsigned long EnsureGetAdaptersAddressesError(unsigned long error_code) {

[eroman%chromium.org, 2011/05/18 20:33:27]

It shouldn't be necessary to map unexpected errorst o ERROR_GEN_FAILURE, since
in your enumeration list you have already made the buckets of size 1. If the
error falls outside of the expected ranges, we will see it regardless (just
won't know it's exact value.)

[Jói, 2011/05/19 14:39:49]

Ah, I misunderstood how the UMA enumerations worked. Fixed that.

+  for (size_t i = 0; i < arraysize(kGetAdaptersAddressesErrors); ++i) {
+    if (error_code == kGetAdaptersAddressesErrors[i])
+      return error_code;
+  }
+  return ERROR_GEN_FAILURE;
+}
+
 }  // namespace
 
 namespace net {
@@ -33,7 +70,8 @@ DhcpProxyScriptFetcherWin::DhcpProxyScriptFetcherWin(
 }
 
 DhcpProxyScriptFetcherWin::~DhcpProxyScriptFetcherWin() {
-  Cancel();
+  // Count as user-initiated if we are not yet in STATE_DONE.
+  CancelImpl(true);
 }
 
 int DhcpProxyScriptFetcherWin::Fetch(string16* utf16_text,
@@ -44,6 +82,8 @@ int DhcpProxyScriptFetcherWin::Fetch(string16* utf16_text,
     return ERR_UNEXPECTED;
   }
 
+  fetch_start_time_ = base::TimeTicks::Now();
+
   std::set<std::string> adapter_names;
   if (!ImplGetCandidateAdapterNames(&adapter_names)) {
     return ERR_UNEXPECTED;
@@ -70,6 +110,10 @@ int DhcpProxyScriptFetcherWin::Fetch(string16* utf16_text,
 }
 
 void DhcpProxyScriptFetcherWin::Cancel() {
+  CancelImpl(true);
+}
+
+void DhcpProxyScriptFetcherWin::CancelImpl(bool user_initiated) {
   DCHECK(CalledOnValidThread());
 
   if (state_ != STATE_DONE) {
@@ -83,6 +127,13 @@ void DhcpProxyScriptFetcherWin::Cancel() {
     }
 
     fetchers_.reset();
+
+    if (user_initiated) {
+      // We only count this stat if the cancel was explicitly initiated by
+      // our client, and if we weren't already in STATE_DONE.
+      UMA_HISTOGRAM_TIMES("Net.DhcpWpadCancelTime",

[eroman%chromium.org, 2011/05/18 20:33:27]

I am not sure how useful the cancelation info will be, but ok.

[Jói, 2011/05/19 14:39:49]

Yeah - once I start seeing the stats I'll remove things that aren't providing
extra information, but would like to have more for now rather than less.

+                          base::TimeTicks::Now() - fetch_start_time_);
+    }
   }
 }
 
@@ -134,6 +185,14 @@ void DhcpProxyScriptFetcherWin::OnFetcherDone(int result) {
 
 void DhcpProxyScriptFetcherWin::OnWaitTimer() {
   DCHECK_EQ(state_, STATE_SOME_RESULTS);
+
+  // These are intended to help us understand whether our timeout may
+  // be too aggressive or not aggressive enough.
+  UMA_HISTOGRAM_COUNTS_100("Net.DhcpWpadNumAdaptersAtWaitTimer",
+                           fetchers_.size());
+  UMA_HISTOGRAM_COUNTS_100("Net.DhcpWpadNumPendingAdaptersAtWaitTimer",
+                           num_pending_fetchers_);
+
   TransitionToDone();
 }
 
@@ -172,10 +231,18 @@ void DhcpProxyScriptFetcherWin::TransitionToDone() {
     }
   }
 
-  Cancel();
+  CancelImpl(false);
   DCHECK_EQ(state_, STATE_DONE);
   DCHECK(fetchers_.empty());
 
+  UMA_HISTOGRAM_TIMES("Net.DhcpWpadCompletionTime",
+                      base::TimeTicks::Now() - fetch_start_time_);
+
+  if (result != OK) {
+    UMA_HISTOGRAM_CUSTOM_ENUMERATION(
+        "Net.DhcpWpadFetchError", std::abs(result), GetAllErrorCodesForUma());
+  }
+
   client_callback_->Run(result);
 }
 
@@ -204,6 +271,8 @@ bool DhcpProxyScriptFetcherWin::GetCandidateAdapterNames(
   scoped_ptr_malloc<IP_ADAPTER_ADDRESSES> adapters;
   ULONG error = ERROR_SUCCESS;
   int num_tries = 0;
+
+  PerfTimer time_api_access;
   do {
     adapters.reset(
         reinterpret_cast<IP_ADAPTER_ADDRESSES*>(malloc(adapters_size)));
@@ -219,6 +288,17 @@ bool DhcpProxyScriptFetcherWin::GetCandidateAdapterNames(
     ++num_tries;
   } while (error == ERROR_BUFFER_OVERFLOW && num_tries <= 3);
 
+  // This is primarily to validate our belief that the GetAdaptersAddresses API
+  // function is fast enough to call synchronously from the network thread.
+  UMA_HISTOGRAM_TIMES("Net.DhcpWpadGetAdaptersAddressesTime",
+                      time_api_access.Elapsed());
+
+  if (error != ERROR_SUCCESS) {
+    UMA_HISTOGRAM_CUSTOM_ENUMERATION("Net.DhcpWpadGetAdaptersAddressesError",
+                                     EnsureGetAdaptersAddressesError(error),
+                                     GetAllGetAdaptersAddressesErrors());
+  }
+
   if (error == ERROR_NO_DATA) {
     // There are no adapters that we care about.
     return true;