Send malware reports when a user opts-in from the safe browsing interstitial page. In this first iteration, the reports are minimal, but I created a new class because I would like to expand them.

Workflow: A SafeBrowsingBlockingPage is shown. A new MalwareReport object is created. The blocking page goes away, either when the user clicks proceed, go back or closed the tab. We read the user's preference, and if we have a pending report, we pass it on to the SafeBrowsingService so it can send it.

Depends on: http://codereview.chromium.org/4827001/
Also relevant: http://codereview.chromium.org/5102001/
Design doc: https://docs.google.com/document/edit?id=1s-7qMjm23onV68SyqO6yi-xsfFzz4OBSOyHoV3cY8mQ&authkey=CMCF-MID

BUG=60831
TEST=unit_tests,relevant browser_test

Review URL: http://codereview.chromium.org/4822002

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

15 files changed, 835 insertions, 35 deletions

diff --git a/chrome/browser/safe_browsing/malware_details.cc b/chrome/browser/safe_browsing/malware_details.cc
new file mode 100644
index 0000000..0bd829c
--- /dev/null
+++ b/chrome/browser/safe_browsing/malware_details.cc
@@ -0,0 +1,99 @@
+// 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.
+//
+// Implementation of the MalwareDetails class.
+
+#include "chrome/browser/safe_browsing/malware_details.h"
+
+#include "chrome/browser/browser_thread.h"
+#include "chrome/browser/safe_browsing/safe_browsing_service.h"
+#include "chrome/browser/tab_contents/navigation_entry.h"
+#include "chrome/browser/tab_contents/tab_contents.h"
+#include "chrome/browser/safe_browsing/report.pb.h"
+
+// Create a MalwareDetails for the given tab. Runs in the UI thread.
+MalwareDetails::MalwareDetails(
+    TabContents* tab_contents,
+    const SafeBrowsingService::UnsafeResource resource)
+    : tab_contents_(tab_contents),
+      resource_(resource) {
+  StartCollection();
+}
+
+MalwareDetails::~MalwareDetails() {}
+
+bool MalwareDetails::IsPublicUrl(const GURL& url) const {
+  return url.SchemeIs("http");  // TODO(panayiotis): also skip internal urls.
+}
+
+void MalwareDetails::AddNode(const std::string& url,
+                             const std::string& parent) {
+  if (!IsPublicUrl(GURL(url)))
+    return;
+  linked_ptr<safe_browsing::ClientMalwareReportRequest::Resource> resource(
+      new safe_browsing::ClientMalwareReportRequest::Resource());
+  resource->set_url(url);
+  if (!parent.empty() && IsPublicUrl(GURL(parent)))
+    resource->set_parent(parent);
+  urls_[url] = resource;
+}
+
+void MalwareDetails::StartCollection() {
+  DVLOG(1) << "Starting to compute malware details.";
+  report_.reset(new safe_browsing::ClientMalwareReportRequest());
+
+  if (IsPublicUrl(resource_.url)) {
+    report_->set_malware_url(resource_.url.spec());
+  }
+
+  GURL page_url = tab_contents_->GetURL();
+  if (IsPublicUrl(page_url)) {
+    report_->set_page_url(page_url.spec());
+  }
+
+  GURL referrer_url;
+  NavigationEntry* nav_entry = tab_contents_->controller().GetActiveEntry();
+  if (nav_entry) {
+    referrer_url = nav_entry->referrer();
+    if (IsPublicUrl(referrer_url)) {
+      report_->set_referrer_url(referrer_url.spec());
+    }
+  }
+
+  // Add the nodes, starting from the page url.
+  AddNode(page_url.spec(), "");
+
+  // Add the resource_url and its original url, if non-empty and different.
+  if (!resource_.original_url.spec().empty() &&
+      resource_.url != resource_.original_url) {
+    // Add original_url, as the parent of resource_url.
+    AddNode(resource_.original_url.spec(), "");
+    AddNode(resource_.url.spec(), resource_.original_url.spec());
+  } else {
+    AddNode(resource_.url.spec(), "");
+  }
+
+  // Add the referrer url.
+  if (nav_entry && !referrer_url.spec().empty()) {
+    AddNode(referrer_url.spec(), "");
+  }
+
+  // Add all the urls in our |urls_| map to the |report_| protobuf.
+  for (ResourceMap::const_iterator it = urls_.begin();
+       it != urls_.end(); it++) {
+    safe_browsing::ClientMalwareReportRequest::Resource* pb_resource =
+        report_->add_nodes();
+    pb_resource->CopyFrom(*(it->second));
+  }
+}
+
+// Called from the SB Service on the IO thread.
+const std::string* MalwareDetails::GetSerializedReport() {
+  scoped_ptr<std::string> request_data(new std::string());
+  if (!report_->SerializeToString(request_data.get())) {
+    DLOG(ERROR) << "Unable to serialize the malware report.";
+  }
+
+  return request_data.release();
+}
diff --git a/chrome/browser/safe_browsing/malware_details.h b/chrome/browser/safe_browsing/malware_details.h
new file mode 100644
index 0000000..7d21dc4
--- /dev/null
+++ b/chrome/browser/safe_browsing/malware_details.h
@@ -0,0 +1,68 @@
+// 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_MALWARE_DETAILS_H_
+#define CHROME_BROWSER_SAFE_BROWSING_MALWARE_DETAILS_H_
+#pragma once
+
+// A class that encapsulates the detailed malware reports sent when
+// users opt-in to do so from the malware warning page.
+
+// An instance of this class is generated when a malware warning page
+// is shown (SafeBrowsingBlockingPage). It is passed on to the
+// SafeBrowsing service when the warning goes away.
+
+#include <string>
+#include <vector>
+
+#include "base/hash_tables.h"
+#include "base/linked_ptr.h"
+#include "base/scoped_ptr.h"
+#include "chrome/browser/safe_browsing/report.pb.h"
+#include "chrome/browser/safe_browsing/safe_browsing_service.h"
+
+class TabContents;
+
+class MalwareDetails : public base::RefCountedThreadSafe<MalwareDetails> {
+ public:
+  MalwareDetails(TabContents* tab_contents,
+                 const SafeBrowsingService::UnsafeResource resource);
+
+  // The SafeBrowsingService calls this from the IO thread, to get the
+  // serialized report as a string and send it over.
+  const std::string* GetSerializedReport();
+
+ private:
+  friend class base::RefCountedThreadSafe<MalwareDetails>;
+
+  typedef base::hash_map<
+    std::string,
+    linked_ptr<safe_browsing::ClientMalwareReportRequest::Resource> >
+  ResourceMap;
+
+  // Starts the collection of the report.
+  void StartCollection();
+
+  // Whether the url is "public" so we can add it to the report.
+  bool IsPublicUrl(const GURL& url) const;
+
+  // Adds a node to |urls_|. |parent| can be empty.
+  void AddNode(const std::string& url, const std::string& parent);
+
+  ~MalwareDetails();
+
+  TabContents* tab_contents_;
+  const SafeBrowsingService::UnsafeResource resource_;
+
+  // The urls that we collect. We first add them into this map and then
+  // generate a protocol buffer from it.
+  ResourceMap urls_;
+
+  // The report protocol buffer.
+  scoped_ptr<safe_browsing::ClientMalwareReportRequest> report_;
+
+  DISALLOW_COPY_AND_ASSIGN(MalwareDetails);
+};
+
+#endif  // CHROME_BROWSER_SAFE_BROWSING_MALWARE_DETAILS_H_
diff --git a/chrome/browser/safe_browsing/malware_details_unittest.cc b/chrome/browser/safe_browsing/malware_details_unittest.cc
new file mode 100644
index 0000000..85b2d45
--- /dev/null
+++ b/chrome/browser/safe_browsing/malware_details_unittest.cc
@@ -0,0 +1,172 @@
+// 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.
+
+#include <algorithm>
+#include "chrome/browser/renderer_host/test/test_render_view_host.h"
+
+#include "chrome/browser/browser_thread.h"
+#include "chrome/browser/safe_browsing/malware_details.h"
+#include "chrome/browser/safe_browsing/report.pb.h"
+#include "chrome/browser/tab_contents/navigation_entry.h"
+#include "chrome/browser/tab_contents/test_tab_contents.h"
+#include "chrome/common/render_messages.h"
+#include "chrome/common/render_messages_params.h"
+
+static const char* kOriginalLandingURL = "http://www.originallandingpage.com/";
+static const char* kLandingURL = "http://www.landingpage.com/";
+static const char* kMalwareURL = "http://www.malware.com/";
+static const char* kHttpsURL = "https://www.url.com/";
+
+class MalwareDetailsTest : public RenderViewHostTestHarness {
+ public:
+  MalwareDetailsTest()
+      : ui_thread_(BrowserThread::UI, MessageLoop::current()),
+        io_thread_(BrowserThread::IO, MessageLoop::current()) {
+  }
+
+  virtual void SetUp() {
+    RenderViewHostTestHarness::SetUp();
+  }
+
+  static bool ResourceLessThan(
+      const safe_browsing::ClientMalwareReportRequest::Resource* lhs,
+      const safe_browsing::ClientMalwareReportRequest::Resource* rhs) {
+    return lhs->url() < rhs->url();
+  }
+
+ protected:
+  void InitResource(SafeBrowsingService::UnsafeResource* resource,
+                    ResourceType::Type resource_type,
+                    const GURL& url) {
+    resource->client = NULL;
+    resource->url = url;
+    resource->resource_type = resource_type;
+    resource->threat_type = SafeBrowsingService::URL_MALWARE;
+    resource->render_process_host_id = contents_->GetRenderProcessHost()->id();
+    resource->render_view_id = contents_->render_view_host()->routing_id();
+  }
+
+  void VerifyResults(
+      const safe_browsing::ClientMalwareReportRequest& report_pb,
+      const safe_browsing::ClientMalwareReportRequest& expected_pb) {
+    EXPECT_EQ(expected_pb.malware_url(), report_pb.malware_url());
+    EXPECT_EQ(expected_pb.page_url(), report_pb.page_url());
+    EXPECT_EQ(expected_pb.referrer_url(), report_pb.referrer_url());
+
+    ASSERT_EQ(expected_pb.nodes_size(), report_pb.nodes_size());
+    // Sort the nodes, to make the test deterministic
+    std::vector<const safe_browsing::ClientMalwareReportRequest::Resource*>
+        nodes;
+    for (int i = 0; i < report_pb.nodes_size(); ++i) {
+      const safe_browsing::ClientMalwareReportRequest::Resource& resource =
+        report_pb.nodes(i);
+      nodes.push_back(&resource);
+    }
+    std::sort(nodes.begin(), nodes.end(),
+              &MalwareDetailsTest::ResourceLessThan);
+
+    std::vector<const safe_browsing::ClientMalwareReportRequest::Resource*>
+        expected;
+    for (int i = 0; i < report_pb.nodes_size(); ++i) {
+      const safe_browsing::ClientMalwareReportRequest::Resource& resource =
+        expected_pb.nodes(i);
+      expected.push_back(&resource);
+    }
+    std::sort(expected.begin(), expected.end(),
+              &MalwareDetailsTest::ResourceLessThan);
+
+    for (uint32 i = 0; i < expected.size(); ++i) {
+      EXPECT_EQ(expected[i]->url(), nodes[i]->url());
+      EXPECT_EQ(expected[i]->parent(), nodes[i]->parent());
+    }
+  }
+
+  BrowserThread ui_thread_;
+  BrowserThread io_thread_;
+};
+
+// Tests creating a simple malware report.
+TEST_F(MalwareDetailsTest, MalwareSubResource) {
+  // Start a load.
+  controller().LoadURL(GURL(kLandingURL), GURL(), PageTransition::TYPED);
+
+  SafeBrowsingService::UnsafeResource resource;
+  InitResource(&resource, ResourceType::SUB_RESOURCE, GURL(kMalwareURL));
+
+  MalwareDetails* report = new MalwareDetails(contents(), resource);
+
+  scoped_ptr<const std::string> serialized(report->GetSerializedReport());
+  safe_browsing::ClientMalwareReportRequest actual;
+  actual.ParseFromString(*serialized);
+
+  safe_browsing::ClientMalwareReportRequest expected;
+  expected.set_malware_url(kMalwareURL);
+  expected.set_page_url(kLandingURL);
+  expected.set_referrer_url("");
+
+  safe_browsing::ClientMalwareReportRequest::Resource* node =
+      expected.add_nodes();
+  node->set_url(kLandingURL);
+  node = expected.add_nodes();
+  node->set_url(kMalwareURL);
+
+  VerifyResults(actual, expected);
+}
+
+// Tests creating a simple malware report where the subresource has a
+// different original_url.
+TEST_F(MalwareDetailsTest, MalwareSubResourceWithOriginalUrl) {
+  controller().LoadURL(GURL(kLandingURL), GURL(), PageTransition::TYPED);
+
+  SafeBrowsingService::UnsafeResource resource;
+  InitResource(&resource, ResourceType::SUB_RESOURCE, GURL(kMalwareURL));
+  resource.original_url = GURL(kOriginalLandingURL);
+
+  MalwareDetails* report = new MalwareDetails(contents(), resource);
+
+  scoped_ptr<const std::string> serialized(report->GetSerializedReport());
+  safe_browsing::ClientMalwareReportRequest actual;
+  actual.ParseFromString(*serialized);
+
+  safe_browsing::ClientMalwareReportRequest expected;
+  expected.set_malware_url(kMalwareURL);
+  expected.set_page_url(kLandingURL);
+  expected.set_referrer_url("");
+
+  safe_browsing::ClientMalwareReportRequest::Resource* node =
+      expected.add_nodes();
+  node->set_url(kLandingURL);
+
+  // Malware url should have originalurl as parent.
+  node = expected.add_nodes();
+  node->set_url(kMalwareURL);
+  node->set_parent(kOriginalLandingURL);
+
+  node = expected.add_nodes();
+  node->set_url(kOriginalLandingURL);
+
+  VerifyResults(actual, expected);
+}
+
+// Verify that https:// urls are dropped.
+TEST_F(MalwareDetailsTest, NotPublicUrl) {
+  controller().LoadURL(GURL(kHttpsURL), GURL(), PageTransition::TYPED);
+  SafeBrowsingService::UnsafeResource resource;
+  InitResource(&resource, ResourceType::SUB_RESOURCE, GURL(kMalwareURL));
+  MalwareDetails* report = new MalwareDetails(contents(), resource);
+
+  scoped_ptr<const std::string> serialized(report->GetSerializedReport());
+  safe_browsing::ClientMalwareReportRequest actual;
+  actual.ParseFromString(*serialized);
+
+  safe_browsing::ClientMalwareReportRequest expected;
+  expected.set_malware_url(kMalwareURL);  // No page_url
+  expected.set_referrer_url("");
+
+  safe_browsing::ClientMalwareReportRequest::Resource* node =
+      expected.add_nodes();
+  node->set_url(kMalwareURL);  // Only one node
+
+  VerifyResults(actual, expected);
+}
diff --git a/chrome/browser/safe_browsing/protocol_manager.cc b/chrome/browser/safe_browsing/protocol_manager.cc
index 487ec9f..bff53fc 100644
--- a/chrome/browser/safe_browsing/protocol_manager.cc
+++ b/chrome/browser/safe_browsing/protocol_manager.cc
@@ -47,8 +47,8 @@ SafeBrowsingProtocolManager::SafeBrowsingProtocolManager(
     const std::string& client_key,
     const std::string& wrapped_key,
     URLRequestContextGetter* request_context_getter,
-    const std::string& info_url_prefix,
-    const std::string& mackey_url_prefix,
+    const std::string& http_url_prefix,
+    const std::string& https_url_prefix,
     bool disable_auto_update)
     : sb_service_(sb_service),
       request_type_(NO_REQUEST),
@@ -65,10 +65,10 @@ SafeBrowsingProtocolManager::SafeBrowsingProtocolManager(
       update_size_(0),
       client_name_(client_name),
       request_context_getter_(request_context_getter),
-      info_url_prefix_(info_url_prefix),
-      mackey_url_prefix_(mackey_url_prefix),
+      http_url_prefix_(http_url_prefix),
+      https_url_prefix_(https_url_prefix),
       disable_auto_update_(disable_auto_update) {
-  DCHECK(!info_url_prefix_.empty() && !mackey_url_prefix_.empty());
+  DCHECK(!http_url_prefix_.empty() && !https_url_prefix_.empty());
 
   // Set the backoff multiplier fuzz to a random value between 0 and 1.
   back_off_fuzz_ = static_cast<float>(base::RandDouble());
@@ -88,7 +88,7 @@ SafeBrowsingProtocolManager::~SafeBrowsingProtocolManager() {
                                       hash_requests_.end());
   hash_requests_.clear();
 
-  // Delete in-progress safebrowsing reports.
+  // Delete in-progress safebrowsing reports (hits and details).
   STLDeleteContainerPointers(safebrowsing_reports_.begin(),
                              safebrowsing_reports_.end());
   safebrowsing_reports_.clear();
@@ -588,15 +588,16 @@ 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,
     SafeBrowsingService::UrlCheckResult threat_type) {
-  GURL report_url = SafeBrowsingReportUrl(malicious_url, page_url,
-                                          referrer_url, is_subresource,
-                                          threat_type);
+  GURL report_url = SafeBrowsingHitUrl(malicious_url, page_url,
+                                       referrer_url, is_subresource,
+                                       threat_type);
   URLFetcher* report = new URLFetcher(report_url, URLFetcher::GET, this);
   report->set_load_flags(net::LOAD_DISABLE_CACHE);
   report->set_request_context(request_context_getter_);
@@ -604,6 +605,21 @@ void SafeBrowsingProtocolManager::ReportSafeBrowsingHit(
   safebrowsing_reports_.insert(report);
 }
 
+// Sends malware details for users who opt-in.
+void SafeBrowsingProtocolManager::ReportMalwareDetails(
+    const std::string& report) {
+  GURL report_url = MalwareDetailsUrl();
+  URLFetcher* fetcher = new URLFetcher(report_url, URLFetcher::POST, this);
+  fetcher->set_load_flags(net::LOAD_DISABLE_CACHE);
+  fetcher->set_request_context(request_context_getter_);
+  fetcher->set_upload_data("application/octet-stream", report);
+  // Don't try too hard to send reports on failures.
+  fetcher->set_automatically_retry_on_5xx(false);
+  fetcher->Start();
+  safebrowsing_reports_.insert(fetcher);
+}
+
+
 // static
 std::string SafeBrowsingProtocolManager::FormatList(
     const SBListChunkRanges& list, bool use_mac) {
@@ -662,7 +678,7 @@ std::string SafeBrowsingProtocolManager::ComposeUrl(
 }
 
 GURL SafeBrowsingProtocolManager::UpdateUrl(bool use_mac) const {
-  std::string url = ComposeUrl(info_url_prefix_, "downloads", client_name_,
+  std::string url = ComposeUrl(http_url_prefix_, "downloads", client_name_,
                                version_, additional_query_);
   if (use_mac) {
     url.append("&wrkey=");
@@ -672,7 +688,7 @@ GURL SafeBrowsingProtocolManager::UpdateUrl(bool use_mac) const {
 }
 
 GURL SafeBrowsingProtocolManager::GetHashUrl(bool use_mac) const {
-  std::string url= ComposeUrl(info_url_prefix_, "gethash", client_name_,
+  std::string url= ComposeUrl(http_url_prefix_, "gethash", client_name_,
                               version_, additional_query_);
   if (use_mac) {
     url.append("&wrkey=");
@@ -682,17 +698,18 @@ GURL SafeBrowsingProtocolManager::GetHashUrl(bool use_mac) const {
 }
 
 GURL SafeBrowsingProtocolManager::MacKeyUrl() const {
-  return GURL(ComposeUrl(mackey_url_prefix_, "newkey", client_name_, version_,
+  return GURL(ComposeUrl(https_url_prefix_, "newkey", client_name_, version_,
                          additional_query_));
 }
 
-GURL SafeBrowsingProtocolManager::SafeBrowsingReportUrl(
+GURL SafeBrowsingProtocolManager::SafeBrowsingHitUrl(
     const GURL& malicious_url, const GURL& page_url,
     const GURL& referrer_url, bool is_subresource,
     SafeBrowsingService::UrlCheckResult threat_type) const {
   DCHECK(threat_type == SafeBrowsingService::URL_MALWARE ||
          threat_type == SafeBrowsingService::URL_PHISHING);
-  std::string url = ComposeUrl(info_url_prefix_, "report", client_name_,
+  // The malware and phishing hits go over HTTP.
+  std::string url = ComposeUrl(http_url_prefix_, "report", client_name_,
                                version_, additional_query_);
   return GURL(StringPrintf("%s&evts=%s&evtd=%s&evtr=%s&evhr=%s&evtb=%d",
       url.c_str(),
@@ -704,6 +721,16 @@ GURL SafeBrowsingProtocolManager::SafeBrowsingReportUrl(
       is_subresource));
 }
 
+GURL SafeBrowsingProtocolManager::MalwareDetailsUrl() const {
+  // The malware details go over HTTPS.
+  std::string url = StringPrintf(
+          "%s/clientreport/malware?client=%s&appver=%s&pver=1.0",
+          https_url_prefix_.c_str(),
+          client_name_.c_str(),
+          version_.c_str());
+  return GURL(url);
+}
+
 GURL SafeBrowsingProtocolManager::NextChunkUrl(const std::string& url) const {
   std::string next_url;
   if (!StartsWithASCII(url, "http://", false) &&
diff --git a/chrome/browser/safe_browsing/protocol_manager.h b/chrome/browser/safe_browsing/protocol_manager.h
index 8dea7f1..7d5ba50 100644
--- a/chrome/browser/safe_browsing/protocol_manager.h
+++ b/chrome/browser/safe_browsing/protocol_manager.h
@@ -50,7 +50,9 @@ class SafeBrowsingProtocolManager : public URLFetcher::Delegate {
                            TestGetHashBackOffTimes);
   FRIEND_TEST_ALL_PREFIXES(SafeBrowsingProtocolManagerTest, TestMacKeyUrl);
   FRIEND_TEST_ALL_PREFIXES(SafeBrowsingProtocolManagerTest,
-                           TestSafeBrowsingReportUrl);
+                           TestSafeBrowsingHitUrl);
+  FRIEND_TEST_ALL_PREFIXES(SafeBrowsingProtocolManagerTest,
+                           TestMalwareDetailsUrl);
   FRIEND_TEST_ALL_PREFIXES(SafeBrowsingProtocolManagerTest, TestNextChunkUrl);
   FRIEND_TEST_ALL_PREFIXES(SafeBrowsingProtocolManagerTest, TestUpdateUrl);
   friend class SafeBrowsingServiceTest;
@@ -65,8 +67,8 @@ class SafeBrowsingProtocolManager : public URLFetcher::Delegate {
                               const std::string& client_key,
                               const std::string& wrapped_key,
                               URLRequestContextGetter* request_context_getter,
-                              const std::string& info_url_prefix,
-                              const std::string& mackey_url_prefix,
+                              const std::string& http_url_prefix,
+                              const std::string& https_url_prefix,
                               bool disable_auto_update);
   virtual ~SafeBrowsingProtocolManager();
 
@@ -117,6 +119,9 @@ class SafeBrowsingProtocolManager : public URLFetcher::Delegate {
                              bool is_subresource,
                              SafeBrowsingService::UrlCheckResult threat_type);
 
+  // Users can opt-in on the SafeBrowsing interstitial to send detailed
+  // malware reports. |report| is the serialized report.
+  void ReportMalwareDetails(const std::string& report);
 
   // Setter for additional_query_. To make sure the additional_query_ won't
   // be changed in the middle of an update, caller (e.g.: SafeBrowsingService)
@@ -158,11 +163,14 @@ class SafeBrowsingProtocolManager : public URLFetcher::Delegate {
   GURL GetHashUrl(bool use_mac) const;
   // Generates new MAC client key request URL.
   GURL MacKeyUrl() const;
-  // Generates URL for reporting malicious pages.
-  GURL SafeBrowsingReportUrl(
+  // 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,
       SafeBrowsingService::UrlCheckResult 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;
 
@@ -297,24 +305,26 @@ class SafeBrowsingProtocolManager : public URLFetcher::Delegate {
   int update_size_;
 
   // Track outstanding SafeBrowsing report fetchers for clean up.
+  // We add both "hit" and "detail" fetchers in this set.
   std::set<const URLFetcher*> safebrowsing_reports_;
 
   // The safe browsing client name sent in each request.
   std::string client_name_;
 
   // A string that is appended to the end of URLs for download, gethash,
-  // newkey, malware report and chunk update requests.
+  // newkey, safebrowsing hits and chunk update requests.
   std::string additional_query_;
 
   // The context we use to issue network requests.
   scoped_refptr<URLRequestContextGetter> request_context_getter_;
 
   // URL prefix where browser fetches safebrowsing chunk updates, hashes, and
-  // reports malware.
-  std::string info_url_prefix_;
+  // reports hits to the safebrowsing list for UMA users.
+  std::string http_url_prefix_;
 
-  // URL prefix where browser fetches MAC client key.
-  std::string mackey_url_prefix_;
+  // URL prefix where browser fetches MAC client key, and reports detailed
+  // malware reports for users who opt-in.
+  std::string https_url_prefix_;
 
   // When true, protocol manager will not start an update unless
   // ForceScheduleNextUpdate() is called. This is set for testing purpose.
diff --git a/chrome/browser/safe_browsing/protocol_manager_unittest.cc b/chrome/browser/safe_browsing/protocol_manager_unittest.cc
index 6343931..fc119ff 100644
--- a/chrome/browser/safe_browsing/protocol_manager_unittest.cc
+++ b/chrome/browser/safe_browsing/protocol_manager_unittest.cc
@@ -193,7 +193,7 @@ TEST_F(SafeBrowsingProtocolManagerTest, TestUpdateUrl) {
             "9g==", pm.UpdateUrl(true).spec());
 }
 
-TEST_F(SafeBrowsingProtocolManagerTest, TestSafeBrowsingReportUrl) {
+TEST_F(SafeBrowsingProtocolManagerTest, TestSafeBrowsingHitUrl) {
   SafeBrowsingProtocolManager pm(NULL, kClient, kClientKey, kWrappedKey, NULL,
                                  kInfoUrlPrefix, kMacKeyUrlPrefix, false);
   pm.version_ = kAppVer;
@@ -205,7 +205,7 @@ TEST_F(SafeBrowsingProtocolManagerTest, TestSafeBrowsingReportUrl) {
             "pver=2.2&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.SafeBrowsingReportUrl(
+            pm.SafeBrowsingHitUrl(
                 malicious_url, page_url, referrer_url,
                 true, SafeBrowsingService::URL_MALWARE).spec());
 
@@ -215,11 +215,22 @@ TEST_F(SafeBrowsingProtocolManagerTest, TestSafeBrowsingReportUrl) {
             "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.SafeBrowsingReportUrl(
+            pm.SafeBrowsingHitUrl(
                 malicious_url, page_url, referrer_url,
                 false, SafeBrowsingService::URL_PHISHING).spec());
 }
 
+TEST_F(SafeBrowsingProtocolManagerTest, TestMalwareDetailsUrl) {
+  SafeBrowsingProtocolManager pm(NULL, kClient, kClientKey, kWrappedKey, NULL,
+                                 kInfoUrlPrefix, kMacKeyUrlPrefix, false);
+
+  pm.version_ = kAppVer;
+  pm.set_additional_query(kAdditionalQuery);  // AdditionalQuery is not used.
+  EXPECT_EQ("https://key.prefix.com/bar/clientreport/malware?"
+            "client=unittest&appver=1.0&pver=1.0",
+            pm.MalwareDetailsUrl().spec());
+}
+
 TEST_F(SafeBrowsingProtocolManagerTest, TestMacKeyUrl) {
   SafeBrowsingProtocolManager pm(NULL, kClient, kClientKey, kWrappedKey, NULL,
                                  kInfoUrlPrefix, kMacKeyUrlPrefix, false);
diff --git a/chrome/browser/safe_browsing/report.proto b/chrome/browser/safe_browsing/report.proto
new file mode 100644
index 0000000..ea4cfda
--- /dev/null
+++ b/chrome/browser/safe_browsing/report.proto
@@ -0,0 +1,91 @@
+// 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.
+//
+// Safe Browsing reporting protocol buffers.
+//
+// A ClientMalwareReportRequest is sent when a user opts-in to 
+// sending detailed malware reports from the safe browsing interstitial page.
+// 
+// It is a list of Resource messages, which may contain the url of a
+// resource such as the page in the address bar or any other resource
+// that was loaded for this page.
+//
+// In addition to the url, a resource can contain HTTP request and response
+// headers and bodies.
+
+syntax = "proto2";
+
+option optimize_for = LITE_RUNTIME;
+
+package safe_browsing;
+
+message ClientMalwareReportRequest {
+
+  message HTTPHeader {
+    required string name = 1;
+    optional string value = 2;
+  }
+
+  message HTTPRequest {
+    message FirstLine {
+      optional string verb = 1;  // Also known as method, eg "GET"
+      optional string uri = 2;
+      optional string version = 3;
+    }
+    
+    optional FirstLine firstline = 1;
+    repeated HTTPHeader headers = 2;
+    optional string body = 3;
+
+    // bodydigest and bodylength can be useful if the report does not
+    // contain the body itself.
+    optional string bodydigest = 4;  
+    optional int32 bodylength = 5;
+  }
+
+  message HTTPResponse {
+    message FirstLine {
+      optional int32 code = 1;
+      optional string reason = 2;
+      optional string version = 3;
+    }
+
+    optional FirstLine firstline = 1;
+    repeated HTTPHeader headers = 2;
+    optional string body = 3;
+
+    // bodydigest and bodylength can be useful if the report does not
+    // contain the body itself.
+    optional string bodydigest = 4;
+    optional int32 bodylength = 5;
+    optional string remote_ip = 6;
+  }
+
+  message Resource {
+    optional string url = 1;
+
+    // URL of the parent frame.
+    optional string parent = 2;
+
+    // Tag that was used to include this resource, eg "iframe"
+    optional string tag_name = 3;
+
+    optional HTTPRequest request = 4;
+    optional HTTPResponse response = 5;
+
+    // A list of children. The order of the children in this list is
+    // significant. The |parent| field for child nodes can be derived
+    // from this, but this allows us to be more flexible.
+    repeated string children = 6;
+  }
+
+  // URL of the resource that matches the safe browsing list.
+  optional string malware_url = 1;
+
+  // URL of the page in the address bar.
+  optional string page_url = 2;
+
+  optional string referrer_url = 3;
+  repeated Resource nodes = 4;
+}
diff --git a/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc b/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
index bc72883..7ec0303 100644
--- a/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
@@ -20,12 +20,16 @@
 #include "chrome/browser/dom_ui/new_tab_ui.h"
 #include "chrome/browser/google/google_util.h"
 #include "chrome/browser/metrics/user_metrics.h"
+#include "chrome/browser/prefs/pref_service.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/safe_browsing/malware_details.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/browser/tab_contents/navigation_controller.h"
 #include "chrome/browser/tab_contents/navigation_entry.h"
 #include "chrome/browser/tab_contents/tab_util.h"
 #include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/common/jstemplate_builder.h"
+#include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "grit/browser_resources.h"
 #include "grit/generated_resources.h"
@@ -106,7 +110,8 @@ SafeBrowsingBlockingPage::SafeBrowsingBlockingPage(
                        unsafe_resources[0].url),
       sb_service_(sb_service),
       is_main_frame_(IsMainPage(unsafe_resources)),
-      unsafe_resources_(unsafe_resources) {
+      unsafe_resources_(unsafe_resources),
+      malware_details_(NULL) {
   RecordUserAction(SHOW);
   if (!is_main_frame_) {
     navigation_entry_index_to_remove_ =
@@ -114,6 +119,23 @@ SafeBrowsingBlockingPage::SafeBrowsingBlockingPage(
   } else {
     navigation_entry_index_to_remove_ = -1;
   }
+
+  // Start computing malware details. They will be sent only
+  // if the user opts-in on the blocking page later.
+  // If there's more than one malicious resources, it means the user
+  // clicked through the first warning, so we don't prepare additional
+  // reports.
+  if (unsafe_resources.size() == 1 &&
+      unsafe_resources[0].threat_type == SafeBrowsingService::URL_MALWARE &&
+      malware_details_ == NULL &&
+      CanShowMalwareDetailsOption()) {
+    malware_details_ = new MalwareDetails(tab(), unsafe_resources[0]);
+  }
+}
+
+bool SafeBrowsingBlockingPage::CanShowMalwareDetailsOption() {
+  return (!tab()->profile()->IsOffTheRecord() &&
+          tab()->GetURL().SchemeIs(chrome::kHttpScheme));
 }
 
 SafeBrowsingBlockingPage::~SafeBrowsingBlockingPage() {
@@ -399,6 +421,7 @@ void SafeBrowsingBlockingPage::CommandReceived(const std::string& cmd) {
 
 void SafeBrowsingBlockingPage::Proceed() {
   RecordUserAction(PROCEED);
+  FinishMalwareDetails();  // Send the malware details, if we opted to.
 
   NotifySafeBrowsingService(sb_service_, unsafe_resources_, true);
 
@@ -436,6 +459,7 @@ void SafeBrowsingBlockingPage::DontProceed() {
   }
 
   RecordUserAction(DONT_PROCEED);
+  FinishMalwareDetails();  // Send the malware details, if we opted to.
 
   NotifySafeBrowsingService(sb_service_, unsafe_resources_, false);
 
@@ -501,6 +525,25 @@ void SafeBrowsingBlockingPage::RecordUserAction(BlockingPageEvent event) {
   UserMetrics::RecordComputedAction(action);
 }
 
+void SafeBrowsingBlockingPage::FinishMalwareDetails() {
+  if (malware_details_ == NULL)
+    return;  // Not all interstitials have malware details (eg phishing).
+
+  const PrefService::Preference* pref =
+      tab()->profile()->GetPrefs()->FindPreference(
+          prefs::kSafeBrowsingReportingEnabled);
+
+  bool value;
+  if (pref && pref->GetValue()->GetAsBoolean(&value) && value) {
+    // Give the details object to the service class, so it can send it.
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        NewRunnableMethod(
+            sb_service_, &SafeBrowsingService::ReportMalwareDetails,
+            malware_details_));
+  }
+}
+
 // static
 void SafeBrowsingBlockingPage::NotifySafeBrowsingService(
     SafeBrowsingService* sb_service,
diff --git a/chrome/browser/safe_browsing/safe_browsing_blocking_page.h b/chrome/browser/safe_browsing/safe_browsing_blocking_page.h
index 7e7cb22..53b21a0 100644
--- a/chrome/browser/safe_browsing/safe_browsing_blocking_page.h
+++ b/chrome/browser/safe_browsing/safe_browsing_blocking_page.h
@@ -39,6 +39,7 @@
 class DictionaryValue;
 class MessageLoop;
 class SafeBrowsingBlockingPageFactory;
+class MalwareDetails;
 class TabContents;
 
 class SafeBrowsingBlockingPage : public InterstitialPage {
@@ -104,6 +105,16 @@ class SafeBrowsingBlockingPage : public InterstitialPage {
   // SBInterstitial[Phishing|Malware|Multiple][Show|Proceed|DontProceed].
   void RecordUserAction(BlockingPageEvent event);
 
+  // See if we should even show the malware details option. For example, we
+  // don't show it in incognito mode.
+  bool CanShowMalwareDetailsOption();
+
+  // Called when the insterstitial is going away. If there is a
+  // pending malware details object, we look at the user's
+  // preferences, and if the option to send malware details is
+  // enabled, the report is scheduled to be sent on the |sb_service_|.
+  void FinishMalwareDetails();
+
   // A list of SafeBrowsingService::UnsafeResource for a tab that the user
   // should be warned about.  They are queued when displaying more than one
   // interstitial at a time.
@@ -136,6 +147,11 @@ class SafeBrowsingBlockingPage : public InterstitialPage {
   // The list of unsafe resources this page is warning about.
   UnsafeResourceList unsafe_resources_;
 
+  // A MalwareDetails object that we start generating when the
+  // blocking page is shown. The object will be sent when the warning
+  // is gone (if the user enables the feature).
+  scoped_refptr<MalwareDetails> malware_details_;
+
   // The factory used to instanciate SafeBrowsingBlockingPage objects.
   // Usefull for tests, so they can provide their own implementation of
   // SafeBrowsingBlockingPage.
diff --git a/chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc b/chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc
index fa30068..f76dd4a 100644
--- a/chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc
@@ -9,13 +9,17 @@
 
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browser_thread.h"
+#include "chrome/browser/prefs/pref_service.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/renderer_host/render_process_host.h"
 #include "chrome/browser/renderer_host/resource_dispatcher_host.h"
+#include "chrome/browser/safe_browsing/malware_details.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/browser/safe_browsing/safe_browsing_blocking_page.h"
 #include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/browser/tab_contents/tab_contents_view.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/test/in_process_browser_test.h"
 #include "chrome/test/ui_test_utils.h"
@@ -52,6 +56,26 @@ class FakeSafeBrowsingService :  public SafeBrowsingService {
     badurls[url.spec()] = checkresult;
   }
 
+  virtual void ReportMalwareDetails(scoped_refptr<MalwareDetails> details) {
+    details_.push_back(details);
+    // Notify the UI thread, that we got a report.
+    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                            NewRunnableMethod(this,
+                            &FakeSafeBrowsingService::OnMalwareDetailsDone));
+  }
+
+  void OnMalwareDetailsDone() {
+    EXPECT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::UI));
+    MessageLoopForUI::current()->Quit();
+  }
+
+  std::list<scoped_refptr<MalwareDetails> >* GetDetails() {
+    return &details_;
+  }
+
+  std::list<scoped_refptr<MalwareDetails> > details_;
+
+
  private:
   base::hash_map<std::string, UrlCheckResult> badurls;
 };
@@ -102,7 +126,7 @@ class SafeBrowsingBlockingPageTest : public InProcessBrowserTest,
             g_browser_process->resource_dispatcher_host()->
             safe_browsing_service());
 
-    ASSERT_TRUE(service != NULL);
+    ASSERT_TRUE(service);
     service->AddURLResult(url, checkresult);
   }
 
@@ -141,6 +165,17 @@ class SafeBrowsingBlockingPageTest : public InProcessBrowserTest,
     ui_test_utils::WaitForNavigation(controller);
   }
 
+  void AssertReportSent() {
+    // When a report is scheduled in the IO thread we should get notified.
+    ui_test_utils::RunMessageLoop();
+
+    FakeSafeBrowsingService* service =
+        static_cast<FakeSafeBrowsingService*>(
+            g_browser_process->resource_dispatcher_host()->
+            safe_browsing_service());
+    ASSERT_EQ(1u, service->GetDetails()->size());
+  }
+
  private:
   TestSafeBrowsingServiceFactory factory;
 
@@ -258,4 +293,25 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingBlockingPageTest, MalwareIframeProceed) {
   EXPECT_EQ(url, browser()->GetSelectedTabContents()->GetURL());
 }
 
+IN_PROC_BROWSER_TEST_F(SafeBrowsingBlockingPageTest,
+                       MalwareIframeReportDetails) {
+  // Enable reporting of malware details.
+  browser()->GetProfile()->GetPrefs()->SetBoolean(
+      prefs::kSafeBrowsingReportingEnabled, true);
+  EXPECT_TRUE(browser()->GetProfile()->GetPrefs()->GetBoolean(
+      prefs::kSafeBrowsingReportingEnabled));
+
+  GURL url = test_server()->GetURL(kMalwarePage);
+  GURL iframe_url = test_server()->GetURL(kMalwareIframe);
+  AddURLResult(iframe_url, SafeBrowsingService::URL_MALWARE);
+
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  SendCommand("\"proceed\"");    // Simulate the user clicking "back"
+  AssertNoInterstitial();  // Assert the interstitial is gone
+
+  EXPECT_EQ(url, browser()->GetSelectedTabContents()->GetURL());
+  AssertReportSent();
+}
+
 }  // namespace
diff --git a/chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc b/chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc
index 60f1043..195f5c0 100644
--- a/chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc
@@ -5,9 +5,13 @@
 #include "chrome/browser/renderer_host/test/test_render_view_host.h"
 
 #include "chrome/browser/browser_thread.h"
+#include "chrome/browser/prefs/pref_service.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/safe_browsing/malware_details.h"
 #include "chrome/browser/safe_browsing/safe_browsing_blocking_page.h"
 #include "chrome/browser/tab_contents/navigation_entry.h"
 #include "chrome/browser/tab_contents/test_tab_contents.h"
+#include "chrome/common/pref_names.h"
 #include "chrome/common/render_messages.h"
 #include "chrome/common/render_messages_params.h"
 
@@ -32,6 +36,20 @@ class TestSafeBrowsingBlockingPage :  public SafeBrowsingBlockingPage {
   }
 };
 
+class TestSafeBrowsingService: public SafeBrowsingService {
+ public:
+  virtual ~TestSafeBrowsingService() {}
+  virtual void ReportMalwareDetails(scoped_refptr<MalwareDetails> details) {
+    details_.push_back(details);
+  }
+
+  std::list<scoped_refptr<MalwareDetails> >* GetDetails() {
+    return &details_;
+  }
+
+  std::list<scoped_refptr<MalwareDetails> > details_;
+};
+
 class TestSafeBrowsingBlockingPageFactory
     : public SafeBrowsingBlockingPageFactory {
  public:
@@ -61,7 +79,7 @@ class SafeBrowsingBlockingPageTest : public RenderViewHostTestHarness,
       : ui_thread_(BrowserThread::UI, MessageLoop::current()),
         io_thread_(BrowserThread::IO, MessageLoop::current()) {
     ResetUserResponse();
-    service_ = SafeBrowsingService::CreateSafeBrowsingService();
+    service_ = new TestSafeBrowsingService();
   }
 
   virtual void SetUp() {
@@ -128,6 +146,8 @@ class SafeBrowsingBlockingPageTest : public RenderViewHostTestHarness,
     MessageLoop::current()->RunAllPending();
   }
 
+  scoped_refptr<TestSafeBrowsingService> service_;
+
  private:
   void InitResource(SafeBrowsingService::UnsafeResource* resource,
                     ResourceType::Type resource_type,
@@ -141,7 +161,6 @@ class SafeBrowsingBlockingPageTest : public RenderViewHostTestHarness,
   }
 
   UserResponse user_response_;
-  scoped_refptr<SafeBrowsingService> service_;
   TestSafeBrowsingBlockingPageFactory factory_;
   BrowserThread ui_thread_;
   BrowserThread io_thread_;
@@ -149,9 +168,14 @@ class SafeBrowsingBlockingPageTest : public RenderViewHostTestHarness,
 
 // Tests showing a blocking page for a malware page and not proceeding.
 TEST_F(SafeBrowsingBlockingPageTest, MalwarePageDontProceed) {
+  // Enable malware details.
+  contents()->profile()->GetPrefs()->SetBoolean(
+      prefs::kSafeBrowsingReportingEnabled, true);
+
   // Start a load.
   controller().LoadURL(GURL(kBadURL), GURL(), PageTransition::TYPED);
 
+
   // Simulate the load causing a safe browsing interstitial to be shown.
   ShowInterstitial(ResourceType::MAIN_FRAME, kBadURL);
   SafeBrowsingBlockingPage* sb_interstitial = GetSafeBrowsingBlockingPage();
@@ -168,10 +192,18 @@ TEST_F(SafeBrowsingBlockingPageTest, MalwarePageDontProceed) {
 
   // We did not proceed, the pending entry should be gone.
   EXPECT_FALSE(controller().pending_entry());
+
+  // A report should have been sent.
+  EXPECT_EQ(1u, service_->GetDetails()->size());
+  service_->GetDetails()->clear();
 }
 
 // Tests showing a blocking page for a malware page and then proceeding.
 TEST_F(SafeBrowsingBlockingPageTest, MalwarePageProceed) {
+  // Enable malware reports.
+  contents()->profile()->GetPrefs()->SetBoolean(
+      prefs::kSafeBrowsingReportingEnabled, true);
+
   // Start a load.
   controller().LoadURL(GURL(kBadURL), GURL(), PageTransition::TYPED);
 
@@ -189,11 +221,19 @@ TEST_F(SafeBrowsingBlockingPageTest, MalwarePageProceed) {
   Navigate(kBadURL, 1);
   // The interstitial should be gone now.
   ASSERT_FALSE(InterstitialPage::GetInterstitialPage(contents()));
+
+  // A report should have been sent.
+  EXPECT_EQ(1u, service_->GetDetails()->size());
+  service_->GetDetails()->clear();
 }
 
 // Tests showing a blocking page for a page that contains malware subresources
 // and not proceeding.
 TEST_F(SafeBrowsingBlockingPageTest, PageWithMalwareResourceDontProceed) {
+  // Enable malware reports.
+  contents()->profile()->GetPrefs()->SetBoolean(
+      prefs::kSafeBrowsingReportingEnabled, true);
+
   // Navigate somewhere.
   Navigate(kGoogleURL, 1);
 
@@ -215,11 +255,19 @@ TEST_F(SafeBrowsingBlockingPageTest, PageWithMalwareResourceDontProceed) {
   // have been removed from the navigation controller.
   ASSERT_EQ(1, controller().entry_count());
   EXPECT_EQ(kGoogleURL, controller().GetActiveEntry()->url().spec());
+
+  // A report should have been sent.
+  EXPECT_EQ(1u, service_->GetDetails()->size());
+  service_->GetDetails()->clear();
 }
 
 // Tests showing a blocking page for a page that contains malware subresources
 // and proceeding.
 TEST_F(SafeBrowsingBlockingPageTest, PageWithMalwareResourceProceed) {
+  // Enable malware reports.
+  contents()->profile()->GetPrefs()->SetBoolean(
+      prefs::kSafeBrowsingReportingEnabled, true);
+
   // Navigate somewhere.
   Navigate(kGoodURL, 1);
 
@@ -237,6 +285,10 @@ TEST_F(SafeBrowsingBlockingPageTest, PageWithMalwareResourceProceed) {
   // We did proceed, we should be back to showing the page.
   ASSERT_EQ(1, controller().entry_count());
   EXPECT_EQ(kGoodURL, controller().GetActiveEntry()->url().spec());
+
+  // A report should have been sent.
+  EXPECT_EQ(1u, service_->GetDetails()->size());
+  service_->GetDetails()->clear();
 }
 
 // Tests showing a blocking page for a page that contains multiple malware
@@ -244,6 +296,10 @@ TEST_F(SafeBrowsingBlockingPageTest, PageWithMalwareResourceProceed) {
 // subresources (which trigger queued interstitial pages) do not break anything.
 TEST_F(SafeBrowsingBlockingPageTest,
        PageWithMultipleMalwareResourceDontProceed) {
+  // Enable malware reports.
+  contents()->profile()->GetPrefs()->SetBoolean(
+      prefs::kSafeBrowsingReportingEnabled, true);
+
   // Navigate somewhere.
   Navigate(kGoogleURL, 1);
 
@@ -270,12 +326,20 @@ TEST_F(SafeBrowsingBlockingPageTest,
   // have been removed from the navigation controller.
   ASSERT_EQ(1, controller().entry_count());
   EXPECT_EQ(kGoogleURL, controller().GetActiveEntry()->url().spec());
+
+  // A report should have been sent.
+  EXPECT_EQ(1u, service_->GetDetails()->size());
+  service_->GetDetails()->clear();
 }
 
 // Tests showing a blocking page for a page that contains multiple malware
 // subresources and proceeding through the first interstitial, but not the next.
 TEST_F(SafeBrowsingBlockingPageTest,
        PageWithMultipleMalwareResourceProceedThenDontProceed) {
+  // Enable malware reports.
+  contents()->profile()->GetPrefs()->SetBoolean(
+      prefs::kSafeBrowsingReportingEnabled, true);
+
   // Navigate somewhere.
   Navigate(kGoogleURL, 1);
 
@@ -297,6 +361,10 @@ TEST_F(SafeBrowsingBlockingPageTest,
   ProceedThroughInterstitial(sb_interstitial);
   EXPECT_EQ(OK, user_response());
 
+  // A report should have been sent.
+  EXPECT_EQ(1u, service_->GetDetails()->size());
+  service_->GetDetails()->clear();
+
   ResetUserResponse();
 
   // We should land to a 2nd interstitial (aggregating all the malware resources
@@ -313,11 +381,20 @@ TEST_F(SafeBrowsingBlockingPageTest,
   // have been removed from the navigation controller.
   ASSERT_EQ(1, controller().entry_count());
   EXPECT_EQ(kGoogleURL, controller().GetActiveEntry()->url().spec());
+
+  // No report should have been sent -- we don't create a report the
+  // second time.
+  EXPECT_EQ(0u, service_->GetDetails()->size());
+  service_->GetDetails()->clear();
 }
 
 // Tests showing a blocking page for a page that contains multiple malware
 // subresources and proceeding through the multiple interstitials.
 TEST_F(SafeBrowsingBlockingPageTest, PageWithMultipleMalwareResourceProceed) {
+  // Enable malware reports.
+  contents()->profile()->GetPrefs()->SetBoolean(
+      prefs::kSafeBrowsingReportingEnabled, true);
+
   // Navigate somewhere else.
   Navigate(kGoodURL, 1);
 
@@ -336,6 +413,10 @@ TEST_F(SafeBrowsingBlockingPageTest, PageWithMultipleMalwareResourceProceed) {
   ProceedThroughInterstitial(sb_interstitial);
   EXPECT_EQ(OK, user_response());
 
+  // A report should have been sent.
+  EXPECT_EQ(1u, service_->GetDetails()->size());
+  service_->GetDetails()->clear();
+
   ResetUserResponse();
 
   // We should land to a 2nd interstitial (aggregating all the malware resources
@@ -350,11 +431,20 @@ TEST_F(SafeBrowsingBlockingPageTest, PageWithMultipleMalwareResourceProceed) {
   // We did proceed, we should be back to the initial page.
   ASSERT_EQ(1, controller().entry_count());
   EXPECT_EQ(kGoodURL, controller().GetActiveEntry()->url().spec());
+
+  // No report should have been sent -- we don't create a report the
+  // second time.
+  EXPECT_EQ(0u, service_->GetDetails()->size());
+  service_->GetDetails()->clear();
 }
 
 // Tests showing a blocking page then navigating back and forth to make sure the
 // controller entries are OK.  http://crbug.com/17627
 TEST_F(SafeBrowsingBlockingPageTest, NavigatingBackAndForth) {
+  // Enable malware reports.
+  contents()->profile()->GetPrefs()->SetBoolean(
+      prefs::kSafeBrowsingReportingEnabled, true);
+
   // Navigate somewhere.
   Navigate(kGoodURL, 1);
 
@@ -388,11 +478,19 @@ TEST_F(SafeBrowsingBlockingPageTest, NavigatingBackAndForth) {
   ASSERT_FALSE(sb_interstitial);
   ASSERT_EQ(2, controller().entry_count());
   EXPECT_EQ(kBadURL, controller().GetActiveEntry()->url().spec());
+
+  // Two reports should have been sent.
+  EXPECT_EQ(2u, service_->GetDetails()->size());
+  service_->GetDetails()->clear();
 }
 
 // Tests that calling "don't proceed" after "proceed" has been called doesn't
 // cause problems. http://crbug.com/30079
 TEST_F(SafeBrowsingBlockingPageTest, ProceedThenDontProceed) {
+  // Enable malware reports.
+  contents()->profile()->GetPrefs()->SetBoolean(
+      prefs::kSafeBrowsingReportingEnabled, true);
+
   // Start a load.
   controller().LoadURL(GURL(kBadURL), GURL(), PageTransition::TYPED);
 
@@ -414,4 +512,39 @@ TEST_F(SafeBrowsingBlockingPageTest, ProceedThenDontProceed) {
   // The interstitial should be gone.
   EXPECT_EQ(OK, user_response());
   EXPECT_FALSE(GetSafeBrowsingBlockingPage());
+
+  // Only one report should have been sent.
+  EXPECT_EQ(1u, service_->GetDetails()->size());
+  service_->GetDetails()->clear();
+}
+
+// Tests showing a blocking page for a malware page with reports disabled.
+TEST_F(SafeBrowsingBlockingPageTest, MalwareReportsDisabled) {
+  // Disable malware reports.
+  contents()->profile()->GetPrefs()->SetBoolean(
+      prefs::kSafeBrowsingReportingEnabled, false);
+
+  // Start a load.
+  controller().LoadURL(GURL(kBadURL), GURL(), PageTransition::TYPED);
+
+  // Simulate the load causing a safe browsing interstitial to be shown.
+  ShowInterstitial(ResourceType::MAIN_FRAME, kBadURL);
+  SafeBrowsingBlockingPage* sb_interstitial = GetSafeBrowsingBlockingPage();
+  ASSERT_TRUE(sb_interstitial);
+
+  MessageLoop::current()->RunAllPending();
+
+  // Simulate the user clicking "don't proceed".
+  DontProceedThroughInterstitial(sb_interstitial);
+
+  // The interstitial should be gone.
+  EXPECT_EQ(CANCEL, user_response());
+  EXPECT_FALSE(GetSafeBrowsingBlockingPage());
+
+  // We did not proceed, the pending entry should be gone.
+  EXPECT_FALSE(controller().pending_entry());
+
+  // No report should have been sent.
+  EXPECT_EQ(0u, service_->GetDetails()->size());
+  service_->GetDetails()->clear();
 }
diff --git a/chrome/browser/safe_browsing/safe_browsing_service.cc b/chrome/browser/safe_browsing/safe_browsing_service.cc
index 6248624..38ec170 100644
--- a/chrome/browser/safe_browsing/safe_browsing_service.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_service.cc
@@ -14,6 +14,7 @@
 #include "chrome/browser/metrics/metrics_service.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/safe_browsing/malware_details.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"
@@ -35,11 +36,12 @@ using base::Time;
 using base::TimeDelta;
 
 // The default URL prefix where browser fetches chunk updates, hashes,
-// and reports malware.
+// and reports safe browsing hits.
 static const char* const kSbDefaultInfoURLPrefix =
     "http://safebrowsing.clients.google.com/safebrowsing";
 
-// The default URL prefix where browser fetches MAC client key.
+// The default URL prefix where browser fetches MAC client key and reports
+// malware details.
 static const char* const kSbDefaultMacKeyURLPrefix =
     "https://sb-ssl.google.com/safebrowsing";
 
@@ -845,6 +847,8 @@ void SafeBrowsingService::DoDisplayBlockingPage(
   SafeBrowsingBlockingPage::ShowBlockingPage(this, resource);
 }
 
+// A safebrowsing hit is sent right after we create a blocking page,
+// only for UMA users.
 void SafeBrowsingService::ReportSafeBrowsingHit(
     const GURL& malicious_url,
     const GURL& page_url,
@@ -862,3 +866,16 @@ void SafeBrowsingService::ReportSafeBrowsingHit(
                                            referrer_url, is_subresource,
                                            threat_type);
 }
+
+// A MalwareDetails report is sent after the blocking page is going
+// away, at which point we see if the user had opted-in using the
+// checkbox on the blocking page.
+void SafeBrowsingService::ReportMalwareDetails(
+    scoped_refptr<MalwareDetails> details) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  scoped_ptr<const std::string> serialized(details->GetSerializedReport());
+  if (!serialized->empty()) {
+    DVLOG(1) << "Sending serialized malware details.";
+    protocol_manager_->ReportMalwareDetails(*serialized);
+  }
+}
diff --git a/chrome/browser/safe_browsing/safe_browsing_service.h b/chrome/browser/safe_browsing/safe_browsing_service.h
index 78e1cde..b9633f9 100644
--- a/chrome/browser/safe_browsing/safe_browsing_service.h
+++ b/chrome/browser/safe_browsing/safe_browsing_service.h
@@ -23,6 +23,7 @@
 #include "googleurl/src/gurl.h"
 #include "webkit/glue/resource_type.h"
 
+class MalwareDetails;
 class PrefService;
 class SafeBrowsingDatabase;
 class SafeBrowsingProtocolManager;
@@ -181,6 +182,10 @@ class SafeBrowsingService
   // the current page is 'safe'.
   void LogPauseDelay(base::TimeDelta time);
 
+  // When a safebrowsing blocking page goes away, it calls this method
+  // so the service can serialize and send MalwareDetails.
+  virtual void ReportMalwareDetails(scoped_refptr<MalwareDetails> details);
+
  protected:
   // Creates the safe browsing service.  Need to initialize before using.
   SafeBrowsingService();
@@ -286,8 +291,8 @@ class SafeBrowsingService
   // Invoked on the UI thread to show the blocking page.
   void DoDisplayBlockingPage(const UnsafeResource& resource);
 
-  // Report any pages that contain malware or phishing to the SafeBrowsing
-  // service.
+  // As soon as we create a blocking page, we schedule this method to
+  // report hits to the malware or phishing list to the server.
   void ReportSafeBrowsingHit(const GURL& malicious_url,
                              const GURL& page_url,
                              const GURL& referrer_url,
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index 57df519..6698c06 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -21,6 +21,7 @@
         'browser/sync/protocol/sync_proto.gyp:sync_proto_cpp',
         'browser/policy/proto/device_management_proto.gyp:device_management_proto_cpp',
         'safe_browsing_csd_proto',
+        'safe_browsing_report_proto',
         'syncapi',
         'theme_resources',
         'userfeedback_proto',
@@ -2196,6 +2197,10 @@
         'browser/safe_browsing/client_side_detection_service.h',
         '<(protoc_out_dir)/chrome/browser/safe_browsing/csd.pb.cc',
         '<(protoc_out_dir)/chrome/browser/safe_browsing/csd.pb.h',
+        '<(protoc_out_dir)/chrome/browser/safe_browsing/report.pb.cc',
+        '<(protoc_out_dir)/chrome/browser/safe_browsing/report.pb.h',
+        'browser/safe_browsing/malware_details.cc',
+        'browser/safe_browsing/malware_details.h',
         'browser/safe_browsing/protocol_manager.cc',
         'browser/safe_browsing/protocol_manager.h',
         'browser/safe_browsing/protocol_parser.cc',
@@ -4420,6 +4425,52 @@
         '../third_party/protobuf/protobuf.gyp:protobuf_lite',
       ],
     },
+    {
+      # Protobuf compiler / generator for the safebrowsing reporting
+      # protocol buffer.
+      'target_name': 'safe_browsing_report_proto',
+      'type': 'none',
+      'sources': [ 'browser/safe_browsing/report.proto' ],
+      'rules': [
+        {
+          'rule_name': 'genproto',
+          'extension': 'proto',
+          'inputs': [
+            '<(PRODUCT_DIR)/<(EXECUTABLE_PREFIX)protoc<(EXECUTABLE_SUFFIX)',
+          ],
+          'variables': {
+            # The protoc compiler requires a proto_path argument with the
+            # directory containing the .proto file.
+            # There's no generator variable that corresponds to this, so fake
+            # it.
+            'rule_input_relpath': 'browser/safe_browsing',
+          },
+          'outputs': [
+            '<(protoc_out_dir)/chrome/<(rule_input_relpath)/<(RULE_INPUT_ROOT).pb.h',
+            '<(protoc_out_dir)/chrome/<(rule_input_relpath)/<(RULE_INPUT_ROOT).pb.cc',
+          ],
+          'action': [
+            '<(PRODUCT_DIR)/<(EXECUTABLE_PREFIX)protoc<(EXECUTABLE_SUFFIX)',
+            '--proto_path=./<(rule_input_relpath)',
+            './<(rule_input_relpath)/<(RULE_INPUT_ROOT)<(RULE_INPUT_EXT)',
+            '--cpp_out=<(protoc_out_dir)/chrome/<(rule_input_relpath)',
+          ],
+          'message': 'Generating C++ code from <(RULE_INPUT_PATH)',
+        },
+      ],
+      'dependencies': [
+        '../third_party/protobuf/protobuf.gyp:protobuf_lite',
+        '../third_party/protobuf/protobuf.gyp:protoc#host',
+      ],
+      'direct_dependent_settings': {
+        'include_dirs': [
+          '<(protoc_out_dir)',
+        ]
+      },
+      'export_dependent_settings': [
+        '../third_party/protobuf/protobuf.gyp:protobuf_lite',
+      ],
+    },
   ],
 }
 
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index 41d3af9..31ce387 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -1310,6 +1310,7 @@
         'browser/safe_browsing/bloom_filter_unittest.cc',
         'browser/safe_browsing/chunk_range_unittest.cc',
         'browser/safe_browsing/client_side_detection_service_unittest.cc',
+        'browser/safe_browsing/malware_details_unittest.cc',
         'browser/safe_browsing/protocol_manager_unittest.cc',
         'browser/safe_browsing/protocol_parser_unittest.cc',
         'browser/safe_browsing/safe_browsing_blocking_page_unittest.cc',