Consolidate some safe-browsing histograms.

chrome/browser/safe_browsing/safe_browsing_service.cc

Index: chrome/browser/safe_browsing/safe_browsing_service.cc
diff --git a/chrome/browser/safe_browsing/safe_browsing_service.cc b/chrome/browser/safe_browsing/safe_browsing_service.cc
index 9e24b038be1fa67fb2e9614d13bf7b666bcc47dd..c4e8eff06e9fe51a515553c4d71acba962e08049 100644
--- a/chrome/browser/safe_browsing/safe_browsing_service.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_service.cc
@@ -35,23 +35,42 @@
 using base::Time;
 using base::TimeDelta;
 
+namespace {
+
 // The default URL prefix where browser fetches chunk updates, hashes,
 // and reports safe browsing hits.
-static const char* const kSbDefaultInfoURLPrefix =
+const char* const kSbDefaultInfoURLPrefix =
     "http://safebrowsing.clients.google.com/safebrowsing";
 
 // The default URL prefix where browser fetches MAC client key and reports
 // malware details.
-static const char* const kSbDefaultMacKeyURLPrefix =
+const char* const kSbDefaultMacKeyURLPrefix =
     "https://sb-ssl.google.com/safebrowsing";
 
-static Profile* GetDefaultProfile() {
+Profile* GetDefaultProfile() {
   FilePath user_data_dir;
   PathService::Get(chrome::DIR_USER_DATA, &user_data_dir);
   ProfileManager* profile_manager = g_browser_process->profile_manager();
   return profile_manager->GetDefaultProfile(user_data_dir);
 }
 
+// Records disposition information about the check.  |hit| should be
+// |true| if there were any prefix hits in |full_hashes|.
+void RecordGetHashCheckStatus(
+    bool hit, const std::vector<SBFullHashResult>& full_hashes) {
+  SafeBrowsingProtocolManager::ResultType result;
+  if (full_hashes.empty()) {
+    result = SafeBrowsingProtocolManager::GET_HASH_FULL_HASH_EMPTY;
+  } else if (hit) {
+    result = SafeBrowsingProtocolManager::GET_HASH_FULL_HASH_HIT;
+  } else {
+    result = SafeBrowsingProtocolManager::GET_HASH_FULL_HASH_MISS;
+  }
+  SafeBrowsingProtocolManager::RecordGetHashResult(result);
+}
+
+}  // namespace
+
 // static
 SafeBrowsingServiceFactory* SafeBrowsingService::factory_ = NULL;
 
@@ -640,7 +659,8 @@ void SafeBrowsingService::OnCheckDone(SafeBrowsingCheck* check) {
     check->start = Time::Now();
     protocol_manager_->GetFullHash(check, check->prefix_hits);
   } else {
-    // We may have cached results for previous GetHash queries.
+    // We may have cached results for previous GetHash queries.  Since
+    // this data comes from cache, don't histogram hits.
     HandleOneCheck(check, check->full_hits);
   }
 }
@@ -817,36 +837,37 @@ void SafeBrowsingService::OnHandleGetHashResults(
   SBPrefix prefix = check->prefix_hits[0];
   GetHashRequests::iterator it = gethash_requests_.find(prefix);
   if (check->prefix_hits.size() > 1 || it == gethash_requests_.end()) {
-    HandleOneCheck(check, full_hashes);
+    const bool hit = HandleOneCheck(check, full_hashes);
+    RecordGetHashCheckStatus(hit, full_hashes);
     return;
   }
 
-  // Call back all interested parties.
+  // Call back all interested parties, noting if any has a hit.
   GetHashRequestors& requestors = it->second;
+  bool hit = false;
   for (GetHashRequestors::iterator r = requestors.begin();
        r != requestors.end(); ++r) {
-    HandleOneCheck(*r, full_hashes);
+    if (HandleOneCheck(*r, full_hashes))
+      hit = true;
   }
+  RecordGetHashCheckStatus(hit, full_hashes);
 
   gethash_requests_.erase(it);
 }
 
-void SafeBrowsingService::HandleOneCheck(
+bool SafeBrowsingService::HandleOneCheck(
     SafeBrowsingCheck* check,
     const std::vector<SBFullHashResult>& full_hashes) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+
+  // Always calculate the index, for recording hits.
+  int index = safe_browsing_util::CompareFullHashes(check->url, full_hashes);
+
+  // |client| is NULL if the request was cancelled.
   if (check->client) {
     UrlCheckResult result = URL_SAFE;
-    int index = safe_browsing_util::CompareFullHashes(check->url, full_hashes);
-    if (index != -1) {
+    if (index != -1)
       result = GetResultFromListname(full_hashes[index].list_name);
-    } else {
-      // Log the case where the SafeBrowsing servers return full hashes in the
-      // GetHash response that match the prefix we're looking up, but don't
-      // match the full hash of the URL.
-      if (!full_hashes.empty())
-        UMA_HISTOGRAM_COUNTS("SB2.GetHashServerMiss", 1);
-    }
 
     // Let the client continue handling the original request.
     check->client->OnSafeBrowsingResult(check->url, result);
@@ -854,6 +875,8 @@ void SafeBrowsingService::HandleOneCheck(
 
   checks_.erase(check);
   delete check;
+
+  return (index != -1);
 }
 
 void SafeBrowsingService::DoDisplayBlockingPage(