Consolidate some safe-browsing histograms.

chrome/browser/safe_browsing/protocol_manager.h

Index: chrome/browser/safe_browsing/protocol_manager.h
diff --git a/chrome/browser/safe_browsing/protocol_manager.h b/chrome/browser/safe_browsing/protocol_manager.h
index 7869e0f4ac666ec836f2a837f6b4af0d7b0f922c..515683f291070843ef31587725759afd88695e0c 100644
--- a/chrome/browser/safe_browsing/protocol_manager.h
+++ b/chrome/browser/safe_browsing/protocol_manager.h
@@ -158,6 +158,21 @@ class SafeBrowsingProtocolManager : public URLFetcher::Delegate {
     return additional_query_;
   }
 
+  // Enumerate failures for histogramming purposes.  DO NOT CHANGE THE
+  // ORDERING OF THESE VALUES.
+  enum ResultType {
+    GET_HASH_STATUS_200,

[Erik does not do reviews, 2010/12/22 19:29:19]

Is it worth tracking other status codes here?  It might be interesting to get an
idea of how often people are being affected by odd errors (proxy auth, 500s,
etc.).  Or maybe that should be a separate histogram?

[Scott Hess - ex-Googler, 2010/12/22 20:41:51]

Hmm.  I vote for a separate histogram.  It can enumerate the codes in a generic
fashion so the we could either have a histogram for all safe-browsing requests,
or one for each different request, all using the same layout.  Also, it's really
more of a transit-level bit of information, whereas this is more of a
protocol-level item.

+    GET_HASH_STATUS_204,
+    GET_HASH_NO_HITS,  // None of the full hashes matched.

[lzheng, 2010/12/22 00:24:58]

Can you add some more comments to clarify that GET_HASH_NO_HITS could be hit for
both GET_HASH_STATUS_200 and GET_HASH_STATUS_204? Otherwise, it could be
misunderstood that the three enum covers three exclusive sets.

[Scott Hess - ex-Googler, 2010/12/22 20:41:51]

In thinking this through, my understanding is that 204 has an empty body, so
full_hashes will be empty, so the current value will be GET_HASH_STATUS_204 +
epsilon.  WDYT of inverting this to instead measure the number of hits, which
will be strictly a subset of GET_HASH_STATUS_200?

[Scott Hess - ex-Googler, 2010/12/22 20:49:36]

Glad I thought more, because GET_HASH_STATUS_200 can also return no full hashes.
 This is the case we were recently in, I think, where the prefix was recognized
but had been logically deleted on the server side.  I think that's reasonable to
track separately from real misses.

+
+    // Memory space for histograms is determined by the max.  ALWAYS
+    // ADD NEW VALUES BEFORE THIS ONE.
+    GET_HASH_RESULT_MAX
+  };
+
+  // Record a GetHash result.
+  static void RecordGetHashResult(ResultType result_type);
+
  protected:
   // Constructs a SafeBrowsingProtocolManager for |sb_service| that issues
   // network requests using |request_context_getter|. When |disable_auto_update|