Consolidate some safe-browsing histograms.

chrome/browser/safe_browsing/protocol_manager.h

 // Copyright (c) 2010 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.
 
 #ifndef CHROME_BROWSER_SAFE_BROWSING_PROTOCOL_MANAGER_H_
 #define CHROME_BROWSER_SAFE_BROWSING_PROTOCOL_MANAGER_H_
 #pragma once
 
 // A class that implements Chrome's interface with the SafeBrowsing protocol.
 // The SafeBrowsingProtocolManager handles formatting and making requests of,
@@ skipping 140 matching lines @@
   // be changed in the middle of an update, caller (e.g.: SafeBrowsingService)
   // should call this after callbacks triggered in UpdateFinished() or before
   // IssueUpdateRequest().
   void set_additional_query(const std::string& query) {
     additional_query_ = query;
   }
   const std::string& additional_query() const {
     return additional_query_;
   }
 
+  // Enumerate failures for histogramming purposes.  DO NOT CHANGE THE
+  // ORDERING OF THESE VALUES.
+  enum ResultType {
+    // A 204 response code means that the server did not know about
+    // the hash prefix that was sent up.
+    GET_HASH_STATUS_200,
+    GET_HASH_STATUS_204,
+
+    // Responses which returned no full hashes.  This includes the 204
+    // case, and also 200 responses for prefixes which are logically

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

So should we split these up?  GET_HASH_FULL_HASH_EMPTY_200/204?  Without them
being split, I'm not sure how you would know what we were getting more of one
than the other.

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

My understanding is that every GET_HASH_STATUS_204 should be empty (that's what
http status code 204 means), so that shouldn't add any info about what is
already here.  Would it?

[lzheng, 2010/12/22 23:56:51]

That's my understanding too. So, GET_HASH_FULL_HASH_EMPTY = (GET_HASH_STATUS_200
- GET_HASH_FULL_HASH_HIT) + GET_HASH_STATUS_204. I think you could export
(GET_HASH_STATUS_200 - GET_HASH_FULL_HASH_HIT) too (what's Erik wants, I
believe), but since it could be calculated out, I am fine either way.

+    // deleted at the server but not yet updated to the client.
+    GET_HASH_FULL_HASH_EMPTY,
+
+    // Subset of 200 responses for which one or more of the full
+    // hashes matched.
+    GET_HASH_FULL_HASH_HIT,
+
+    // 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|
   // is true, protocol manager won't schedule next update until
   // ForceScheduleNextUpdate is called.
   SafeBrowsingProtocolManager(SafeBrowsingService* sb_service,
                               const std::string& client_name,
                               const std::string& client_key,
                               const std::string& wrapped_key,
                               URLRequestContextGetter* request_context_getter,
@@ skipping 199 matching lines @@
   std::string https_url_prefix_;
 
   // When true, protocol manager will not start an update unless
   // ForceScheduleNextUpdate() is called. This is set for testing purpose.
   bool disable_auto_update_;
 
   DISALLOW_COPY_AND_ASSIGN(SafeBrowsingProtocolManager);
 };
 
 #endif  // CHROME_BROWSER_SAFE_BROWSING_PROTOCOL_MANAGER_H_