Split safe browsing protocol manager. Currently the protocol manager has two jobs: it downloads safebrowsing information from Google servers, and it provides safebrowsing stats. These two are orthogonal so we split the functionality into separate entities.

BUG=158532


Review URL: https://chromiumcodereview.appspot.com/11273106

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@166607 0039d316-1c4b-4281-b951-d872f2087c98

12 files changed, 476 insertions, 259 deletions

diff --git a/chrome/browser/safe_browsing/ping_manager.cc b/chrome/browser/safe_browsing/ping_manager.cc
new file mode 100644
index 0000000..6e73985
--- /dev/null
+++ b/chrome/browser/safe_browsing/ping_manager.cc
@@ -0,0 +1,157 @@
+// Copyright (c) 2012 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.
+
+#include "chrome/browser/safe_browsing/ping_manager.h"
+
+#include "base/logging.h"
+#include "base/stl_util.h"
+#include "base/string_util.h"
+#include "base/stringprintf.h"
+#include "chrome/common/env_vars.h"
+#include "content/public/browser/browser_thread.h"
+#include "google_apis/google_api_keys.h"
+#include "net/base/escape.h"
+#include "net/base/load_flags.h"
+#include "net/url_request/url_fetcher.h"
+#include "net/url_request/url_request_context_getter.h"
+#include "net/url_request/url_request_status.h"
+
+using content::BrowserThread;
+
+// SafeBrowsingPingManager implementation ----------------------------------
+
+// static
+SafeBrowsingPingManager* SafeBrowsingPingManager::Create(
+    net::URLRequestContextGetter* request_context_getter,
+    const SafeBrowsingProtocolConfig& config) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  return new SafeBrowsingPingManager(request_context_getter, config);
+}
+
+SafeBrowsingPingManager::SafeBrowsingPingManager(
+    net::URLRequestContextGetter* request_context_getter,
+    const SafeBrowsingProtocolConfig& config)
+    : client_name_(config.client_name),
+      request_context_getter_(request_context_getter),
+      url_prefix_(config.url_prefix) {
+  DCHECK(!url_prefix_.empty());
+
+  version_ = SafeBrowsingProtocolManagerHelper::Version();
+}
+
+SafeBrowsingPingManager::~SafeBrowsingPingManager() {
+  // Delete in-progress safebrowsing reports (hits and details).
+  STLDeleteContainerPointers(safebrowsing_reports_.begin(),
+                             safebrowsing_reports_.end());
+  safebrowsing_reports_.clear();
+}
+
+// net::URLFetcherDelegate implementation ----------------------------------
+
+// All SafeBrowsing request responses are handled here.
+void SafeBrowsingPingManager::OnURLFetchComplete(
+    const net::URLFetcher* source) {
+  scoped_ptr<const net::URLFetcher> fetcher;
+
+  std::set<const net::URLFetcher*>::iterator sit =
+      safebrowsing_reports_.find(source);
+  if (sit != safebrowsing_reports_.end()) {
+    const net::URLFetcher* report = *sit;
+    safebrowsing_reports_.erase(sit);
+    delete report;
+    return;
+  }
+  NOTREACHED();
+}
+
+// Sends a SafeBrowsing "hit" for UMA users.
+void SafeBrowsingPingManager::ReportSafeBrowsingHit(
+    const GURL& malicious_url,
+    const GURL& page_url,
+    const GURL& referrer_url,
+    bool is_subresource,
+    SBThreatType threat_type,
+    const std::string& post_data) {
+  GURL report_url = SafeBrowsingHitUrl(malicious_url, page_url,
+                                       referrer_url, is_subresource,
+                                       threat_type);
+  net::URLFetcher* report = net::URLFetcher::Create(
+      report_url,
+      post_data.empty() ? net::URLFetcher::GET : net::URLFetcher::POST,
+      this);
+  report->SetLoadFlags(net::LOAD_DISABLE_CACHE);
+  report->SetRequestContext(request_context_getter_);
+  if (!post_data.empty())
+    report->SetUploadData("text/plain", post_data);
+  safebrowsing_reports_.insert(report);
+  report->Start();
+}
+
+// Sends malware details for users who opt-in.
+void SafeBrowsingPingManager::ReportMalwareDetails(
+    const std::string& report) {
+  GURL report_url = MalwareDetailsUrl();
+  net::URLFetcher* fetcher = net::URLFetcher::Create(
+      report_url, net::URLFetcher::POST, this);
+  fetcher->SetLoadFlags(net::LOAD_DISABLE_CACHE);
+  fetcher->SetRequestContext(request_context_getter_);
+  fetcher->SetUploadData("application/octet-stream", report);
+  // Don't try too hard to send reports on failures.
+  fetcher->SetAutomaticallyRetryOn5xx(false);
+  fetcher->Start();
+  safebrowsing_reports_.insert(fetcher);
+}
+
+GURL SafeBrowsingPingManager::SafeBrowsingHitUrl(
+    const GURL& malicious_url, const GURL& page_url,
+    const GURL& referrer_url, bool is_subresource,
+    SBThreatType threat_type) const {
+  DCHECK(threat_type == SB_THREAT_TYPE_URL_MALWARE ||
+         threat_type == SB_THREAT_TYPE_URL_PHISHING ||
+         threat_type == SB_THREAT_TYPE_BINARY_MALWARE_URL ||
+         threat_type == SB_THREAT_TYPE_BINARY_MALWARE_HASH ||
+         threat_type == SB_THREAT_TYPE_CLIENT_SIDE_PHISHING_URL);
+  std::string url = SafeBrowsingProtocolManagerHelper::ComposeUrl(
+      url_prefix_, "report", client_name_, version_, "");
+  std::string threat_list = "none";
+  switch (threat_type) {
+    case SB_THREAT_TYPE_URL_MALWARE:
+      threat_list = "malblhit";
+      break;
+    case SB_THREAT_TYPE_URL_PHISHING:
+      threat_list = "phishblhit";
+      break;
+    case SB_THREAT_TYPE_BINARY_MALWARE_URL:
+      threat_list = "binurlhit";
+      break;
+    case SB_THREAT_TYPE_BINARY_MALWARE_HASH:
+      threat_list = "binhashhit";
+      break;
+    case SB_THREAT_TYPE_CLIENT_SIDE_PHISHING_URL:
+      threat_list = "phishcsdhit";
+      break;
+    default:
+      NOTREACHED();
+  }
+  return GURL(base::StringPrintf("%s&evts=%s&evtd=%s&evtr=%s&evhr=%s&evtb=%d",
+      url.c_str(), threat_list.c_str(),
+      net::EscapeQueryParamValue(malicious_url.spec(), true).c_str(),
+      net::EscapeQueryParamValue(page_url.spec(), true).c_str(),
+      net::EscapeQueryParamValue(referrer_url.spec(), true).c_str(),
+      is_subresource));
+}
+
+GURL SafeBrowsingPingManager::MalwareDetailsUrl() const {
+  std::string url = base::StringPrintf(
+          "%s/clientreport/malware?client=%s&appver=%s&pver=1.0",
+          url_prefix_.c_str(),
+          client_name_.c_str(),
+          version_.c_str());
+  std::string api_key = google_apis::GetAPIKey();
+  if (!api_key.empty()) {
+    base::StringAppendF(&url, "&key=%s",
+                        net::EscapeQueryParamValue(api_key, true).c_str());
+  }
+  return GURL(url);
+}
diff --git a/chrome/browser/safe_browsing/ping_manager.h b/chrome/browser/safe_browsing/ping_manager.h
new file mode 100644
index 0000000..de0548a
--- /dev/null
+++ b/chrome/browser/safe_browsing/ping_manager.h
@@ -0,0 +1,92 @@
+// Copyright (c) 2012 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_PING_MANAGER_H_
+#define CHROME_BROWSER_SAFE_BROWSING_PING_MANAGER_H_
+
+// A class that reports safebrowsing statistics to Google's SafeBrowsing
+// servers.
+#include <set>
+#include <string>
+#include <vector>
+
+#include "base/gtest_prod_util.h"
+#include "base/memory/scoped_ptr.h"
+#include "chrome/browser/safe_browsing/protocol_manager_helper.h"
+#include "chrome/browser/safe_browsing/safe_browsing_util.h"
+#include "googleurl/src/gurl.h"
+#include "net/url_request/url_fetcher_delegate.h"
+
+namespace net {
+class URLRequestContextGetter;
+}  // namespace net
+
+
+class SafeBrowsingPingManager : public net::URLFetcherDelegate {
+ public:
+  virtual ~SafeBrowsingPingManager();
+
+  // Create an instance of the safe browsing ping manager.
+  static SafeBrowsingPingManager* Create(
+      net::URLRequestContextGetter* request_context_getter,
+      const SafeBrowsingProtocolConfig& config);
+
+  // net::URLFetcherDelegate interface.
+  virtual void OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE;
+
+  // For UMA users we report to Google when a SafeBrowsing interstitial is shown
+  // to the user.  |threat_type| should be one of the types known by
+  // SafeBrowsingHitUrl.
+  void ReportSafeBrowsingHit(const GURL& malicious_url,
+                             const GURL& page_url,
+                             const GURL& referrer_url,
+                             bool is_subresource,
+                             SBThreatType threat_type,
+                             const std::string& post_data);
+
+  // Users can opt-in on the SafeBrowsing interstitial to send detailed
+  // malware reports. |report| is the serialized report.
+  void ReportMalwareDetails(const std::string& report);
+
+ private:
+  FRIEND_TEST_ALL_PREFIXES(SafeBrowsingPingManagerTest,
+                           TestSafeBrowsingHitUrl);
+  FRIEND_TEST_ALL_PREFIXES(SafeBrowsingPingManagerTest,
+                           TestMalwareDetailsUrl);
+
+  // Constructs a SafeBrowsingPingManager that issues network requests
+  // using |request_context_getter|.
+  SafeBrowsingPingManager(
+      net::URLRequestContextGetter* request_context_getter,
+      const SafeBrowsingProtocolConfig& config);
+
+  // Generates URL for reporting safe browsing hits for UMA users.
+  GURL SafeBrowsingHitUrl(
+      const GURL& malicious_url, const GURL& page_url, const GURL& referrer_url,
+      bool is_subresource,
+      SBThreatType threat_type) const;
+  // Generates URL for reporting malware details for users who opt-in.
+  GURL MalwareDetailsUrl() const;
+
+  // Current product version sent in each request.
+  std::string version_;
+
+  // The safe browsing client name sent in each request.
+  std::string client_name_;
+
+  // The context we use to issue network requests.
+  scoped_refptr<net::URLRequestContextGetter> request_context_getter_;
+
+  // URL prefix where browser reports hits to the safebrowsing list and
+  // sends detaild malware reports for UMA users.
+  std::string url_prefix_;
+
+  // Track outstanding SafeBrowsing report fetchers for clean up.
+  // We add both "hit" and "detail" fetchers in this set.
+  std::set<const net::URLFetcher*> safebrowsing_reports_;
+
+  DISALLOW_COPY_AND_ASSIGN(SafeBrowsingPingManager);
+};
+
+#endif  // CHROME_BROWSER_SAFE_BROWSING_PING_MANAGER_H_
diff --git a/chrome/browser/safe_browsing/ping_manager_unittest.cc b/chrome/browser/safe_browsing/ping_manager_unittest.cc
new file mode 100644
index 0000000..fd8c888
--- /dev/null
+++ b/chrome/browser/safe_browsing/ping_manager_unittest.cc
@@ -0,0 +1,102 @@
+// Copyright (c) 2012 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.
+//
+
+#include "base/logging.h"
+#include "base/stringprintf.h"
+#include "base/time.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "chrome/browser/safe_browsing/ping_manager.h"
+#include "google_apis/google_api_keys.h"
+#include "net/base/escape.h"
+
+using base::Time;
+using base::TimeDelta;
+
+static const char kUrlPrefix[] = "https://prefix.com/foo";
+static const char kClient[] = "unittest";
+static const char kAppVer[] = "1.0";
+
+class SafeBrowsingPingManagerTest : public testing::Test {
+ protected:
+  std::string key_param_;
+
+  virtual void SetUp() {
+    std::string key = google_apis::GetAPIKey();
+    if (!key.empty()) {
+      key_param_ = base::StringPrintf(
+          "&key=%s",
+          net::EscapeQueryParamValue(key, true).c_str());
+    }
+  }
+};
+
+TEST_F(SafeBrowsingPingManagerTest, TestSafeBrowsingHitUrl) {
+  SafeBrowsingProtocolConfig config;
+  config.client_name = kClient;
+  config.url_prefix = kUrlPrefix;
+  SafeBrowsingPingManager pm(NULL, config);
+
+  pm.version_ = kAppVer;
+
+  GURL malicious_url("http://malicious.url.com");
+  GURL page_url("http://page.url.com");
+  GURL referrer_url("http://referrer.url.com");
+  EXPECT_EQ("https://prefix.com/foo/report?client=unittest&appver=1.0&"
+            "pver=2.2" + key_param_ +
+            "&evts=malblhit&evtd=http%3A%2F%2Fmalicious.url.com%2F&"
+            "evtr=http%3A%2F%2Fpage.url.com%2F&evhr=http%3A%2F%2Freferrer."
+            "url.com%2F&evtb=1",
+            pm.SafeBrowsingHitUrl(
+                malicious_url, page_url, referrer_url,
+                true, SB_THREAT_TYPE_URL_MALWARE).spec());
+
+  EXPECT_EQ("https://prefix.com/foo/report?client=unittest&appver=1.0&"
+            "pver=2.2" + key_param_ + "&evts=phishblhit&"
+            "evtd=http%3A%2F%2Fmalicious.url.com%2F&"
+            "evtr=http%3A%2F%2Fpage.url.com%2F&evhr=http%3A%2F%2Freferrer."
+            "url.com%2F&evtb=0",
+            pm.SafeBrowsingHitUrl(
+                malicious_url, page_url, referrer_url,
+                false, SB_THREAT_TYPE_URL_PHISHING).spec());
+
+  EXPECT_EQ("https://prefix.com/foo/report?client=unittest&appver=1.0&"
+            "pver=2.2" + key_param_ + "&evts=binurlhit&"
+            "evtd=http%3A%2F%2Fmalicious.url.com%2F&"
+            "evtr=http%3A%2F%2Fpage.url.com%2F&evhr=http%3A%2F%2Freferrer."
+            "url.com%2F&evtb=0",
+            pm.SafeBrowsingHitUrl(
+                malicious_url, page_url, referrer_url,
+                false, SB_THREAT_TYPE_BINARY_MALWARE_URL).spec());
+
+  EXPECT_EQ("https://prefix.com/foo/report?client=unittest&appver=1.0&"
+            "pver=2.2" + key_param_ + "&evts=binhashhit&"
+            "evtd=http%3A%2F%2Fmalicious.url.com%2F&"
+            "evtr=http%3A%2F%2Fpage.url.com%2F&evhr=http%3A%2F%2Freferrer."
+            "url.com%2F&evtb=0",
+            pm.SafeBrowsingHitUrl(
+                malicious_url, page_url, referrer_url,
+                false, SB_THREAT_TYPE_BINARY_MALWARE_HASH).spec());
+
+  EXPECT_EQ("https://prefix.com/foo/report?client=unittest&appver=1.0&"
+            "pver=2.2" + key_param_ + "&evts=phishcsdhit&"
+            "evtd=http%3A%2F%2Fmalicious.url.com%2F&"
+            "evtr=http%3A%2F%2Fpage.url.com%2F&evhr=http%3A%2F%2Freferrer."
+            "url.com%2F&evtb=0",
+            pm.SafeBrowsingHitUrl(
+                malicious_url, page_url, referrer_url,
+                false, SB_THREAT_TYPE_CLIENT_SIDE_PHISHING_URL).spec());
+}
+
+TEST_F(SafeBrowsingPingManagerTest, TestMalwareDetailsUrl) {
+  SafeBrowsingProtocolConfig config;
+  config.client_name = kClient;
+  config.url_prefix = kUrlPrefix;
+  SafeBrowsingPingManager pm(NULL, config);
+
+  pm.version_ = kAppVer;
+  EXPECT_EQ("https://prefix.com/foo/clientreport/malware?"
+            "client=unittest&appver=1.0&pver=1.0" + key_param_,
+            pm.MalwareDetailsUrl().spec());
+}
diff --git a/chrome/browser/safe_browsing/protocol_manager.cc b/chrome/browser/safe_browsing/protocol_manager.cc
index 41f24c8..8807c78 100644
--- a/chrome/browser/safe_browsing/protocol_manager.cc
+++ b/chrome/browser/safe_browsing/protocol_manager.cc
@@ -16,7 +16,6 @@
 #include "base/stringprintf.h"
 #include "base/timer.h"
 #include "chrome/browser/safe_browsing/protocol_parser.h"
-#include "chrome/common/chrome_version_info.h"
 #include "chrome/common/env_vars.h"
 #include "content/public/browser/browser_thread.h"
 #include "google_apis/google_api_keys.h"
@@ -95,12 +94,7 @@ SafeBrowsingProtocolManager::SafeBrowsingProtocolManager(
 
   // Set the backoff multiplier fuzz to a random value between 0 and 1.
   back_off_fuzz_ = static_cast<float>(base::RandDouble());
-
-  chrome::VersionInfo version_info;
-  if (!version_info.is_valid() || version_info.Version().empty())
-    version_ = "0.1";
-  else
-    version_ = version_info.Version();
+  version_ = SafeBrowsingProtocolManagerHelper::Version();
 }
 
 // static
@@ -120,11 +114,6 @@ SafeBrowsingProtocolManager::~SafeBrowsingProtocolManager() {
   STLDeleteContainerPairFirstPointers(hash_requests_.begin(),
                                       hash_requests_.end());
   hash_requests_.clear();
-
-  // Delete in-progress safebrowsing reports (hits and details).
-  STLDeleteContainerPointers(safebrowsing_reports_.begin(),
-                             safebrowsing_reports_.end());
-  safebrowsing_reports_.clear();
 }
 
 // We can only have one update or chunk request outstanding, but there may be
@@ -178,17 +167,6 @@ void SafeBrowsingProtocolManager::OnURLFetchComplete(
   bool parsed_ok = true;
   bool must_back_off = false;  // Reduce SafeBrowsing service query frequency.
 
-  // See if this is a safebrowsing report fetcher. We don't take any action for
-  // the response to those.
-  std::set<const net::URLFetcher*>::iterator sit =
-      safebrowsing_reports_.find(source);
-  if (sit != safebrowsing_reports_.end()) {
-    const net::URLFetcher* report = *sit;
-    safebrowsing_reports_.erase(sit);
-    delete report;
-    return;
-  }
-
   HashRequests::iterator it = hash_requests_.find(source);
   if (it != hash_requests_.end()) {
     // GetHash response.
@@ -564,45 +542,6 @@ void SafeBrowsingProtocolManager::OnChunkInserted() {
   }
 }
 
-// Sends a SafeBrowsing "hit" for UMA users.
-void SafeBrowsingProtocolManager::ReportSafeBrowsingHit(
-    const GURL& malicious_url,
-    const GURL& page_url,
-    const GURL& referrer_url,
-    bool is_subresource,
-    SBThreatType threat_type,
-    const std::string& post_data) {
-  GURL report_url = SafeBrowsingHitUrl(malicious_url, page_url,
-                                       referrer_url, is_subresource,
-                                       threat_type);
-  net::URLFetcher* report = net::URLFetcher::Create(
-      report_url,
-      post_data.empty() ? net::URLFetcher::GET : net::URLFetcher::POST,
-      this);
-  report->SetLoadFlags(net::LOAD_DISABLE_CACHE);
-  report->SetRequestContext(request_context_getter_);
-  if (!post_data.empty())
-    report->SetUploadData("text/plain", post_data);
-  report->Start();
-  safebrowsing_reports_.insert(report);
-}
-
-// Sends malware details for users who opt-in.
-void SafeBrowsingProtocolManager::ReportMalwareDetails(
-    const std::string& report) {
-  GURL report_url = MalwareDetailsUrl();
-  net::URLFetcher* fetcher = net::URLFetcher::Create(
-      report_url, net::URLFetcher::POST, this);
-  fetcher->SetLoadFlags(net::LOAD_DISABLE_CACHE);
-  fetcher->SetRequestContext(request_context_getter_);
-  fetcher->SetUploadData("application/octet-stream", report);
-  // Don't try too hard to send reports on failures.
-  fetcher->SetAutomaticallyRetryOn5xx(false);
-  fetcher->Start();
-  safebrowsing_reports_.insert(fetcher);
-}
-
-
 // static
 std::string SafeBrowsingProtocolManager::FormatList(
     const SBListChunkRanges& list) {
@@ -634,83 +573,15 @@ void SafeBrowsingProtocolManager::UpdateFinished(bool success) {
   delegate_->UpdateFinished(success);
 }
 
-std::string SafeBrowsingProtocolManager::ComposeUrl(
-    const std::string& prefix, const std::string& method,
-    const std::string& client_name, const std::string& version,
-    const std::string& additional_query) {
-  DCHECK(!prefix.empty() && !method.empty() &&
-         !client_name.empty() && !version.empty());
-  std::string url = base::StringPrintf("%s/%s?client=%s&appver=%s&pver=2.2",
-                                       prefix.c_str(), method.c_str(),
-                                       client_name.c_str(), version.c_str());
-  std::string api_key = google_apis::GetAPIKey();
-  if (!api_key.empty()) {
-    base::StringAppendF(&url, "&key=%s",
-                        net::EscapeQueryParamValue(api_key, true).c_str());
-  }
-  if (!additional_query.empty()) {
-    DCHECK(url.find("?") != std::string::npos);
-    url.append("&");
-    url.append(additional_query);
-  }
-  return url;
-}
-
 GURL SafeBrowsingProtocolManager::UpdateUrl() const {
-  return GURL(ComposeUrl(url_prefix_, "downloads", client_name_, version_,
-                         additional_query_));
+  std::string url = SafeBrowsingProtocolManagerHelper::ComposeUrl(
+      url_prefix_, "downloads", client_name_, version_, additional_query_);
+  return GURL(url);
 }
 
 GURL SafeBrowsingProtocolManager::GetHashUrl() const {
-  return GURL(ComposeUrl(url_prefix_, "gethash", client_name_, version_,
-                         additional_query_));
-}
-
-GURL SafeBrowsingProtocolManager::SafeBrowsingHitUrl(
-    const GURL& malicious_url, const GURL& page_url,
-    const GURL& referrer_url, bool is_subresource,
-    SBThreatType threat_type) const {
-  std::string url = ComposeUrl(url_prefix_, "report", client_name_,
-                               version_, additional_query_);
-  std::string threat_list = "none";
-  switch (threat_type) {
-    case SB_THREAT_TYPE_URL_MALWARE:
-      threat_list = "malblhit";
-      break;
-    case SB_THREAT_TYPE_URL_PHISHING:
-      threat_list = "phishblhit";
-      break;
-    case SB_THREAT_TYPE_BINARY_MALWARE_URL:
-      threat_list = "binurlhit";
-      break;
-    case SB_THREAT_TYPE_BINARY_MALWARE_HASH:
-      threat_list = "binhashhit";
-      break;
-    case SB_THREAT_TYPE_CLIENT_SIDE_PHISHING_URL:
-      threat_list = "phishcsdhit";
-      break;
-    default:
-      NOTREACHED();
-  }
-  return GURL(base::StringPrintf("%s&evts=%s&evtd=%s&evtr=%s&evhr=%s&evtb=%d",
-      url.c_str(), threat_list.c_str(),
-      net::EscapeQueryParamValue(malicious_url.spec(), true).c_str(),
-      net::EscapeQueryParamValue(page_url.spec(), true).c_str(),
-      net::EscapeQueryParamValue(referrer_url.spec(), true).c_str(),
-      is_subresource));
-}
-
-GURL SafeBrowsingProtocolManager::MalwareDetailsUrl() const {
-  std::string url = base::StringPrintf(
-          "%s/clientreport/malware?client=%s&appver=%s&pver=1.0",
-          url_prefix_.c_str(),
-          client_name_.c_str(),
-          version_.c_str());
-  std::string api_key = google_apis::GetAPIKey();
-  if (!api_key.empty()) {
-    base::StringAppendF(&url, "&key=%s",
-                        net::EscapeQueryParamValue(api_key, true).c_str());
-  }
+  std::string url = SafeBrowsingProtocolManagerHelper::ComposeUrl(
+      url_prefix_, "gethash", client_name_, version_, additional_query_);
   return GURL(url);
 }
 
diff --git a/chrome/browser/safe_browsing/protocol_manager.h b/chrome/browser/safe_browsing/protocol_manager.h
index 3042682..2f82ece 100644
--- a/chrome/browser/safe_browsing/protocol_manager.h
+++ b/chrome/browser/safe_browsing/protocol_manager.h
@@ -22,6 +22,7 @@
 #include "base/timer.h"
 #include "chrome/browser/safe_browsing/chunk_range.h"
 #include "chrome/browser/safe_browsing/protocol_parser.h"
+#include "chrome/browser/safe_browsing/protocol_manager_helper.h"
 #include "chrome/browser/safe_browsing/safe_browsing_util.h"
 #include "googleurl/src/gurl.h"
 #include "net/url_request/url_fetcher_delegate.h"
@@ -44,13 +45,6 @@ struct hash<const net::URLFetcher*> {
 }
 #endif
 
-struct SafeBrowsingProtocolConfig {
-  SafeBrowsingProtocolConfig() : disable_auto_update(false) {}
-  std::string client_name;
-  std::string url_prefix;
-  bool disable_auto_update;
-};
-
 class SBProtocolManagerFactory;
 class SafeBrowsingProtocolManagerDelegate;
 
@@ -73,14 +67,14 @@ class SafeBrowsingProtocolManager : public net::URLFetcherDelegate {
     factory_ = factory;
   }
 
-  // Create an instance of the safe browsing service.
+  // Create an instance of the safe browsing protocol manager.
   static SafeBrowsingProtocolManager* Create(
       SafeBrowsingProtocolManagerDelegate* delegate,
       net::URLRequestContextGetter* request_context_getter,
       const SafeBrowsingProtocolConfig& config);
 
   // Sets up the update schedule and internal state for making periodic requests
-  // of the SafeBrowsing service.
+  // of the Safebrowsing servers.
   virtual void Initialize();
 
   // net::URLFetcherDelegate interface.
@@ -109,20 +103,6 @@ class SafeBrowsingProtocolManager : public net::URLFetcherDelegate {
   // Called after the chunks that were parsed were inserted in the database.
   void OnChunkInserted();
 
-  // For UMA users we report to Google when a SafeBrowsing interstitial is shown
-  // to the user.  |threat_type| should be one of the types known by
-  // SafeBrowsingHitUrl.
-  void ReportSafeBrowsingHit(const GURL& malicious_url,
-                             const GURL& page_url,
-                             const GURL& referrer_url,
-                             bool is_subresource,
-                             SBThreatType threat_type,
-                             const std::string& post_data);
-
-  // Users can opt-in on the SafeBrowsing interstitial to send detailed
-  // malware reports. |report| is the serialized report.
-  void ReportMalwareDetails(const std::string& report);
-
   // The last time we received an update.
   base::Time last_update() const { return last_update_; }
 
@@ -184,10 +164,6 @@ class SafeBrowsingProtocolManager : public net::URLFetcherDelegate {
   FRIEND_TEST_ALL_PREFIXES(SafeBrowsingProtocolManagerTest, TestGetHashUrl);
   FRIEND_TEST_ALL_PREFIXES(SafeBrowsingProtocolManagerTest,
                            TestGetHashBackOffTimes);
-  FRIEND_TEST_ALL_PREFIXES(SafeBrowsingProtocolManagerTest,
-                           TestSafeBrowsingHitUrl);
-  FRIEND_TEST_ALL_PREFIXES(SafeBrowsingProtocolManagerTest,
-                           TestMalwareDetailsUrl);
   FRIEND_TEST_ALL_PREFIXES(SafeBrowsingProtocolManagerTest, TestNextChunkUrl);
   FRIEND_TEST_ALL_PREFIXES(SafeBrowsingProtocolManagerTest, TestUpdateUrl);
   friend class SafeBrowsingServiceTest;
@@ -202,26 +178,11 @@ class SafeBrowsingProtocolManager : public net::URLFetcherDelegate {
     CHUNK_REQUEST,      // Request for a specific chunk
   };
 
-  // Composes a URL using |prefix|, |method| (e.g.: gethash, download, report).
-  // |client_name| and |version|. When not empty, |additional_query| is
-  // appended to the URL with an additional "&" in the front.
-  static std::string ComposeUrl(const std::string& prefix,
-                                const std::string& method,
-                                const std::string& client_name,
-                                const std::string& version,
-                                const std::string& additional_query);
-
   // Generates Update URL for querying about the latest set of chunk updates.
   GURL UpdateUrl() const;
   // Generates GetHash request URL for retrieving full hashes.
   GURL GetHashUrl() const;
   // Generates URL for reporting safe browsing hits for UMA users.
-  GURL SafeBrowsingHitUrl(
-      const GURL& malicious_url, const GURL& page_url, const GURL& referrer_url,
-      bool is_subresource,
-      SBThreatType threat_type) const;
-  // Generates URL for reporting malware details for users who opt-in.
-  GURL MalwareDetailsUrl() const;
 
   // Composes a ChunkUrl based on input string.
   GURL NextChunkUrl(const std::string& input) const;
@@ -352,10 +313,6 @@ class SafeBrowsingProtocolManager : public net::URLFetcherDelegate {
   // Tracks the size of each update (in bytes).
   int update_size_;
 
-  // Track outstanding SafeBrowsing report fetchers for clean up.
-  // We add both "hit" and "detail" fetchers in this set.
-  std::set<const net::URLFetcher*> safebrowsing_reports_;
-
   // The safe browsing client name sent in each request.
   std::string client_name_;
 
@@ -366,9 +323,7 @@ class SafeBrowsingProtocolManager : public net::URLFetcherDelegate {
   // The context we use to issue network requests.
   scoped_refptr<net::URLRequestContextGetter> request_context_getter_;
 
-  // URL prefix where browser fetches safebrowsing chunk updates, hashes, and
-  // reports hits to the safebrowsing list and sends detaild malware reports
-  // for UMA users.
+  // URL prefix where browser fetches safebrowsing chunk updates, and hashes.
   std::string url_prefix_;
 
   // When true, protocol manager will not start an update unless
diff --git a/chrome/browser/safe_browsing/protocol_manager_helper.cc b/chrome/browser/safe_browsing/protocol_manager_helper.cc
new file mode 100644
index 0000000..c0cc245
--- /dev/null
+++ b/chrome/browser/safe_browsing/protocol_manager_helper.cc
@@ -0,0 +1,51 @@
+// Copyright (c) 2012 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.
+
+#include "chrome/browser/safe_browsing/protocol_manager_helper.h"
+
+#ifndef NDEBUG
+#include "base/base64.h"
+#endif
+#include "base/environment.h"
+#include "base/logging.h"
+#include "base/stl_util.h"
+#include "base/string_util.h"
+#include "base/stringprintf.h"
+#include "chrome/common/chrome_version_info.h"
+#include "chrome/common/env_vars.h"
+#include "content/public/browser/browser_thread.h"
+#include "google_apis/google_api_keys.h"
+#include "net/base/escape.h"
+
+// static
+std::string SafeBrowsingProtocolManagerHelper::Version() {
+  chrome::VersionInfo version_info;
+  if (!version_info.is_valid() || version_info.Version().empty())
+    return "0.1";
+  else
+    return version_info.Version();
+}
+
+// static
+std::string SafeBrowsingProtocolManagerHelper::ComposeUrl(
+    const std::string& prefix, const std::string& method,
+    const std::string& client_name, const std::string& version,
+    const std::string& additional_query) {
+  DCHECK(!prefix.empty() && !method.empty() &&
+         !client_name.empty() && !version.empty());
+  std::string url = base::StringPrintf("%s/%s?client=%s&appver=%s&pver=2.2",
+                                       prefix.c_str(), method.c_str(),
+                                       client_name.c_str(), version.c_str());
+  std::string api_key = google_apis::GetAPIKey();
+  if (!api_key.empty()) {
+    base::StringAppendF(&url, "&key=%s",
+                        net::EscapeQueryParamValue(api_key, true).c_str());
+  }
+  if (!additional_query.empty()) {
+    DCHECK(url.find("?") != std::string::npos);
+    url.append("&");
+    url.append(additional_query);
+  }
+  return url;
+}
diff --git a/chrome/browser/safe_browsing/protocol_manager_helper.h b/chrome/browser/safe_browsing/protocol_manager_helper.h
new file mode 100644
index 0000000..fdd0ee0
--- /dev/null
+++ b/chrome/browser/safe_browsing/protocol_manager_helper.h
@@ -0,0 +1,43 @@
+// Copyright (c) 2012 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_HELPER_H_
+#define CHROME_BROWSER_SAFE_BROWSING_PROTOCOL_MANAGER_HELPER_H_
+
+// A class that provides common functionality for safebrowsing protocol managers
+// that communicate with Google servers.
+
+#include <string>
+
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
+
+#include "base/gtest_prod_util.h"
+
+struct SafeBrowsingProtocolConfig {
+  SafeBrowsingProtocolConfig() : disable_auto_update(false) {}
+  std::string client_name;
+  std::string url_prefix;
+  bool disable_auto_update;
+};
+
+class SafeBrowsingProtocolManagerHelper {
+ public:
+  // returns version
+  static std::string Version();
+
+  // Composes a URL using |prefix|, |method| (e.g.: gethash, download, report).
+  // |client_name| and |version|. When not empty, |additional_query| is
+  // appended to the URL with an additional "&" in the front.
+  static std::string ComposeUrl(const std::string& prefix,
+                                const std::string& method,
+                                const std::string& client_name,
+                                const std::string& version,
+                                const std::string& additional_query);
+
+ private:
+  DISALLOW_IMPLICIT_CONSTRUCTORS(SafeBrowsingProtocolManagerHelper);
+};
+
+#endif  // CHROME_BROWSER_SAFE_BROWSING_PROTOCOL_MANAGER_HELPER_H_
diff --git a/chrome/browser/safe_browsing/protocol_manager_unittest.cc b/chrome/browser/safe_browsing/protocol_manager_unittest.cc
index bb18d92..0f22e61 100644
--- a/chrome/browser/safe_browsing/protocol_manager_unittest.cc
+++ b/chrome/browser/safe_browsing/protocol_manager_unittest.cc
@@ -201,78 +201,6 @@ TEST_F(SafeBrowsingProtocolManagerTest, TestUpdateUrl) {
             pm.UpdateUrl().spec());
 }
 
-TEST_F(SafeBrowsingProtocolManagerTest, TestSafeBrowsingHitUrl) {
-  SafeBrowsingProtocolConfig config;
-  config.client_name = kClient;
-  config.url_prefix = kUrlPrefix;
-  SafeBrowsingProtocolManager pm(NULL, NULL, config);
-
-  pm.version_ = kAppVer;
-
-  GURL malicious_url("http://malicious.url.com");
-  GURL page_url("http://page.url.com");
-  GURL referrer_url("http://referrer.url.com");
-  EXPECT_EQ("https://prefix.com/foo/report?client=unittest&appver=1.0&"
-            "pver=2.2" + key_param_ +
-            "&evts=malblhit&evtd=http%3A%2F%2Fmalicious.url.com%2F&"
-            "evtr=http%3A%2F%2Fpage.url.com%2F&evhr=http%3A%2F%2Freferrer."
-            "url.com%2F&evtb=1",
-            pm.SafeBrowsingHitUrl(
-                malicious_url, page_url, referrer_url,
-                true, SB_THREAT_TYPE_URL_MALWARE).spec());
-
-  pm.set_additional_query(kAdditionalQuery);
-  EXPECT_EQ("https://prefix.com/foo/report?client=unittest&appver=1.0&"
-            "pver=2.2" + key_param_ + "&additional_query&evts=phishblhit&"
-            "evtd=http%3A%2F%2Fmalicious.url.com%2F&"
-            "evtr=http%3A%2F%2Fpage.url.com%2F&evhr=http%3A%2F%2Freferrer."
-            "url.com%2F&evtb=0",
-            pm.SafeBrowsingHitUrl(
-                malicious_url, page_url, referrer_url,
-                false, SB_THREAT_TYPE_URL_PHISHING).spec());
-
-  EXPECT_EQ("https://prefix.com/foo/report?client=unittest&appver=1.0&"
-            "pver=2.2" + key_param_ + "&additional_query&evts=binurlhit&"
-            "evtd=http%3A%2F%2Fmalicious.url.com%2F&"
-            "evtr=http%3A%2F%2Fpage.url.com%2F&evhr=http%3A%2F%2Freferrer."
-            "url.com%2F&evtb=0",
-            pm.SafeBrowsingHitUrl(
-                malicious_url, page_url, referrer_url,
-                false, SB_THREAT_TYPE_BINARY_MALWARE_URL).spec());
-
-  EXPECT_EQ("https://prefix.com/foo/report?client=unittest&appver=1.0&"
-            "pver=2.2" + key_param_ + "&additional_query&evts=binhashhit&"
-            "evtd=http%3A%2F%2Fmalicious.url.com%2F&"
-            "evtr=http%3A%2F%2Fpage.url.com%2F&evhr=http%3A%2F%2Freferrer."
-            "url.com%2F&evtb=0",
-            pm.SafeBrowsingHitUrl(
-                malicious_url, page_url, referrer_url,
-                false, SB_THREAT_TYPE_BINARY_MALWARE_HASH).spec());
-
-  EXPECT_EQ("https://prefix.com/foo/report?client=unittest&appver=1.0&"
-            "pver=2.2" + key_param_ + "&additional_query&evts=phishcsdhit&"
-            "evtd=http%3A%2F%2Fmalicious.url.com%2F&"
-            "evtr=http%3A%2F%2Fpage.url.com%2F&evhr=http%3A%2F%2Freferrer."
-            "url.com%2F&evtb=0",
-            pm.SafeBrowsingHitUrl(
-                malicious_url, page_url, referrer_url,
-                false, SB_THREAT_TYPE_CLIENT_SIDE_PHISHING_URL).spec());
-}
-
-TEST_F(SafeBrowsingProtocolManagerTest, TestMalwareDetailsUrl) {
-  SafeBrowsingProtocolConfig config;
-  config.client_name = kClient;
-  config.url_prefix = kUrlPrefix;
-
-  SafeBrowsingProtocolManager pm(NULL, NULL, config);
-
-  pm.version_ = kAppVer;
-  pm.set_additional_query(kAdditionalQuery);  // AdditionalQuery is not used.
-  EXPECT_EQ("https://prefix.com/foo/clientreport/malware?"
-            "client=unittest&appver=1.0&pver=1.0" + key_param_,
-            pm.MalwareDetailsUrl().spec());
-}
-
 TEST_F(SafeBrowsingProtocolManagerTest, TestNextChunkUrl) {
   SafeBrowsingProtocolConfig config;
   config.client_name = kClient;
diff --git a/chrome/browser/safe_browsing/safe_browsing_service.cc b/chrome/browser/safe_browsing/safe_browsing_service.cc
index 3b112d2..f22ac5e 100644
--- a/chrome/browser/safe_browsing/safe_browsing_service.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_service.cc
@@ -25,6 +25,7 @@
 #include "chrome/browser/safe_browsing/client_side_detection_service.h"
 #include "chrome/browser/safe_browsing/download_protection_service.h"
 #include "chrome/browser/safe_browsing/malware_details.h"
+#include "chrome/browser/safe_browsing/ping_manager.h"
 #include "chrome/browser/safe_browsing/protocol_manager.h"
 #include "chrome/browser/safe_browsing/safe_browsing_blocking_page.h"
 #include "chrome/browser/safe_browsing/safe_browsing_database.h"
@@ -227,6 +228,7 @@ SafeBrowsingService* SafeBrowsingService::CreateSafeBrowsingService() {
 SafeBrowsingService::SafeBrowsingService()
     : database_(NULL),
       protocol_manager_(NULL),
+      ping_manager_(NULL),
       enabled_(false),
       enable_download_protection_(false),
       enable_csd_whitelist_(false),
@@ -673,8 +675,12 @@ void SafeBrowsingService::StartOnIOThread() {
       SafeBrowsingProtocolManager::Create(this,
                                           url_request_context_getter_,
                                           config);
-
   protocol_manager_->Initialize();
+
+  DCHECK(!ping_manager_);
+  ping_manager_ =
+      SafeBrowsingPingManager::Create(url_request_context_getter_,
+                                      config);
 }
 
 void SafeBrowsingService::StopOnIOThread() {
@@ -690,6 +696,9 @@ void SafeBrowsingService::StopOnIOThread() {
   delete protocol_manager_;
   protocol_manager_ = NULL;
 
+  delete ping_manager_;
+  ping_manager_ = NULL;
+
   // Delete queued checks, calling back any clients with 'SB_THREAT_TYPE_SAFE'.
   // If we don't do this here we may fail to close the database below.
   while (!queued_checks_.empty()) {
@@ -1208,7 +1217,7 @@ void SafeBrowsingService::ReportSafeBrowsingHitOnIOThread(
   DVLOG(1) << "ReportSafeBrowsingHit: " << malicious_url << " " << page_url
            << " " << referrer_url << " " << is_subresource << " "
            << threat_type;
-  protocol_manager_->ReportSafeBrowsingHit(malicious_url, page_url,
+  ping_manager_->ReportSafeBrowsingHit(malicious_url, page_url,
                                            referrer_url, is_subresource,
                                            threat_type, post_data);
 }
@@ -1223,7 +1232,7 @@ void SafeBrowsingService::SendSerializedMalwareDetails(
 
   if (!serialized.empty()) {
     DVLOG(1) << "Sending serialized malware details.";
-    protocol_manager_->ReportMalwareDetails(serialized);
+    ping_manager_->ReportMalwareDetails(serialized);
   }
 }
 
diff --git a/chrome/browser/safe_browsing/safe_browsing_service.h b/chrome/browser/safe_browsing/safe_browsing_service.h
index e75b4d8..801bf44 100644
--- a/chrome/browser/safe_browsing/safe_browsing_service.h
+++ b/chrome/browser/safe_browsing/safe_browsing_service.h
@@ -35,6 +35,7 @@ class MalwareDetails;
 class PrefChangeRegistrar;
 class PrefService;
 class SafeBrowsingDatabase;
+class SafeBrowsingPingManager;
 class SafeBrowsingServiceFactory;
 class SafeBrowsingURLRequestContextGetter;
 
@@ -514,6 +515,9 @@ class SafeBrowsingService
   // Handles interaction with SafeBrowsing servers.
   SafeBrowsingProtocolManager* protocol_manager_;
 
+  // Provides phishing and malware statistics.
+  SafeBrowsingPingManager* ping_manager_;
+
   // Only access this whitelist from the UI thread.
   std::vector<WhiteListedEntry> white_listed_entries_;
 
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index c94c206..263308d 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -1646,10 +1646,14 @@
         'browser/safe_browsing/malware_details_cache.h',
         'browser/safe_browsing/malware_details_history.cc',
         'browser/safe_browsing/malware_details_history.h',
+        'browser/safe_browsing/ping_manager.cc',
+        'browser/safe_browsing/ping_manager.h',
         'browser/safe_browsing/prefix_set.cc',
         'browser/safe_browsing/prefix_set.h',
         'browser/safe_browsing/protocol_manager.cc',
         'browser/safe_browsing/protocol_manager.h',
+        'browser/safe_browsing/protocol_manager_helper.cc',
+        'browser/safe_browsing/protocol_manager_helper.h',
         'browser/safe_browsing/protocol_parser.cc',
         'browser/safe_browsing/protocol_parser.h',
         'browser/safe_browsing/safe_browsing_blocking_page.cc',
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index 2227e0c..e53caea 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -1657,6 +1657,7 @@
         'browser/safe_browsing/client_side_detection_service_unittest.cc',
         'browser/safe_browsing/download_protection_service_unittest.cc',
         'browser/safe_browsing/malware_details_unittest.cc',
+        'browser/safe_browsing/ping_manager_unittest.cc',
         'browser/safe_browsing/prefix_set_unittest.cc',
         'browser/safe_browsing/protocol_manager_unittest.cc',
         'browser/safe_browsing/protocol_parser_unittest.cc',