Implement safebrowsing download feedback service, enabled for dev & canary only.

For eligible files, the "Discard" button in the download shelf becomes
"Report & Discard".  (Plain "Discard" is still available in the popup menu.)

BUG=169557

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

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

21 files changed, 1402 insertions, 8 deletions

diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd
index 2c69841..3136880 100644
--- a/chrome/app/generated_resources.grd
+++ b/chrome/app/generated_resources.grd
@@ -2920,6 +2920,10 @@ Psst! Incognito mode <ph name="SHORTCUT_KEY">$1<ex>(Ctrl+Shift+N)</ex></ph> may
                desc="Text for the button used to stop a dangerous download.">
         Discard
       </message>
+      <message name="IDS_REPORT_AND_DISCARD_DOWNLOAD"
+               desc="Text for the button used to upload a malicious file to Google for analysis and then discard it.">
+        Report &amp; Discard
+      </message>
 
       <!-- Download Tab Items -->
       <message name="IDS_DOWNLOAD_LINK_PAUSE"
diff --git a/chrome/browser/download/download_item_model.cc b/chrome/browser/download/download_item_model.cc
index 347a554e..8561c6b 100644
--- a/chrome/browser/download/download_item_model.cc
+++ b/chrome/browser/download/download_item_model.cc
@@ -12,6 +12,7 @@
 #include "base/supports_user_data.h"
 #include "base/time.h"
 #include "chrome/browser/download/download_crx_util.h"
+#include "chrome/browser/safe_browsing/download_feedback_service.h"
 #include "chrome/common/time_format.h"
 #include "content/public/browser/download_danger_type.h"
 #include "content/public/browser/download_interrupt_reasons.h"
@@ -393,6 +394,13 @@ bool DownloadItemModel::IsMalicious() const {
   return false;
 }
 
+bool DownloadItemModel::ShouldAllowDownloadFeedback() const {
+  if (!IsDangerous())
+    return false;
+  return safe_browsing::DownloadFeedbackService::IsEnabledForDownload(
+      *download_);
+}
+
 bool DownloadItemModel::ShouldRemoveFromShelfWhenComplete() const {
   // If the download was already opened automatically, it should be removed.
   if (download_->GetAutoOpened())
diff --git a/chrome/browser/download/download_item_model.h b/chrome/browser/download/download_item_model.h
index 66021d3..157024f 100644
--- a/chrome/browser/download/download_item_model.h
+++ b/chrome/browser/download/download_item_model.h
@@ -79,6 +79,9 @@ class DownloadItemModel {
   // Is this considered a malicious download? Implies IsDangerous().
   bool IsMalicious() const;
 
+  // Is safe browsing download feedback feature available for this download?
+  bool ShouldAllowDownloadFeedback() const;
+
   // Returns |true| if this download is expected to complete successfully and
   // thereafter be removed from the shelf.  Downloads that are opened
   // automatically or are temporary will be removed from the shelf on successful
diff --git a/chrome/browser/safe_browsing/download_feedback.cc b/chrome/browser/safe_browsing/download_feedback.cc
new file mode 100644
index 0000000..fef426a
--- /dev/null
+++ b/chrome/browser/safe_browsing/download_feedback.cc
@@ -0,0 +1,226 @@
+// Copyright 2013 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/download_feedback.h"
+
+#include "base/bind.h"
+#include "base/command_line.h"
+#include "base/files/file_util_proxy.h"
+#include "base/metrics/histogram.h"
+#include "base/task_runner.h"
+#include "chrome/common/chrome_switches.h"
+#include "chrome/common/safe_browsing/csd.pb.h"
+#include "net/base/net_errors.h"
+
+namespace safe_browsing {
+
+namespace {
+
+// The default URL where browser sends download feedback requests.
+const char kSbDefaultFeedbackURL[] =
+    "https://safebrowsing.google.com/safebrowsing/uploads/chrome";
+
+// This enum is used by histograms.  Do not change the ordering or remove items.
+enum UploadResultType {
+  UPLOAD_SUCCESS,
+  UPLOAD_CANCELLED,
+  UPLOAD_METADATA_NET_ERROR,
+  UPLOAD_METADATA_RESPONSE_ERROR,
+  UPLOAD_FILE_NET_ERROR,
+  UPLOAD_FILE_RESPONSE_ERROR,
+  UPLOAD_COMPLETE_RESPONSE_ERROR,
+  // Memory space for histograms is determined by the max.
+  // ALWAYS ADD NEW VALUES BEFORE THIS ONE.
+  UPLOAD_RESULT_MAX
+};
+
+// Handles the uploading of a single downloaded binary to the safebrowsing
+// download feedback service.
+class DownloadFeedbackImpl : public DownloadFeedback {
+ public:
+  DownloadFeedbackImpl(net::URLRequestContextGetter* request_context_getter,
+                       base::TaskRunner* file_task_runner,
+                       const base::FilePath& file_path,
+                       const std::string& ping_request,
+                       const std::string& ping_response);
+  virtual ~DownloadFeedbackImpl();
+
+  virtual void Start(const base::Closure& finish_callback) OVERRIDE;
+
+  virtual const std::string& GetPingRequestForTesting() const OVERRIDE {
+    return ping_request_;
+  }
+
+  virtual const std::string& GetPingResponseForTesting() const OVERRIDE {
+    return ping_response_;
+  }
+
+ private:
+  // Callback for TwoPhaseUploader completion.  Relays the result to the
+  // |finish_callback|.
+  void FinishedUpload(base::Closure finish_callback,
+                      TwoPhaseUploader::State state,
+                      int net_error,
+                      int response_code,
+                      const std::string& response);
+
+  void RecordUploadResult(UploadResultType result);
+
+  scoped_refptr<net::URLRequestContextGetter> request_context_getter_;
+  scoped_refptr<base::TaskRunner> file_task_runner_;
+  const base::FilePath file_path_;
+  int64 file_size_;
+
+  // The safebrowsing request and response of checking that this binary is
+  // unsafe.
+  std::string ping_request_;
+  std::string ping_response_;
+
+  scoped_ptr<TwoPhaseUploader> uploader_;
+
+  DISALLOW_COPY_AND_ASSIGN(DownloadFeedbackImpl);
+};
+
+DownloadFeedbackImpl::DownloadFeedbackImpl(
+    net::URLRequestContextGetter* request_context_getter,
+    base::TaskRunner* file_task_runner,
+    const base::FilePath& file_path,
+    const std::string& ping_request,
+    const std::string& ping_response)
+    : request_context_getter_(request_context_getter),
+      file_task_runner_(file_task_runner),
+      file_path_(file_path),
+      file_size_(-1),
+      ping_request_(ping_request),
+      ping_response_(ping_response) {
+  DVLOG(1) << "DownloadFeedback constructed " << this << " for "
+           << file_path.AsUTF8Unsafe();
+}
+
+DownloadFeedbackImpl::~DownloadFeedbackImpl() {
+  DCHECK(CalledOnValidThread());
+  DVLOG(1) << "DownloadFeedback destructed " << this;
+
+  if (uploader_) {
+    RecordUploadResult(UPLOAD_CANCELLED);
+    // Destroy the uploader before attempting to delete the file.
+    uploader_.reset();
+  }
+
+  base::FileUtilProxy::Delete(file_task_runner_, file_path_, false,
+                              base::FileUtilProxy::StatusCallback());
+}
+
+void DownloadFeedbackImpl::Start(const base::Closure& finish_callback) {
+  DCHECK(CalledOnValidThread());
+  DCHECK(!uploader_);
+
+  CommandLine* cmdline = CommandLine::ForCurrentProcess();
+
+  ClientDownloadReport report_metadata;
+
+  bool r = report_metadata.mutable_download_request()->ParseFromString(
+      ping_request_);
+  DCHECK(r);
+  r = report_metadata.mutable_download_response()->ParseFromString(
+      ping_response_);
+  DCHECK(r);
+  file_size_ = report_metadata.download_request().length();
+
+  GURL url = GURL(cmdline->HasSwitch(switches::kSbDownloadFeedbackURL) ?
+      cmdline->GetSwitchValueASCII(switches::kSbDownloadFeedbackURL) :
+      kSbDefaultFeedbackURL);
+
+  std::string metadata_string;
+  bool ok = report_metadata.SerializeToString(&metadata_string);
+  DCHECK(ok);
+  uploader_.reset(TwoPhaseUploader::Create(
+      request_context_getter_,
+      file_task_runner_,
+      url,
+      metadata_string,
+      file_path_,
+      TwoPhaseUploader::ProgressCallback(),
+      base::Bind(&DownloadFeedbackImpl::FinishedUpload, base::Unretained(this),
+                 finish_callback)));
+  uploader_->Start();
+}
+
+void DownloadFeedbackImpl::FinishedUpload(base::Closure finish_callback,
+                                          TwoPhaseUploader::State state,
+                                          int net_error,
+                                          int response_code,
+                                          const std::string& response_data) {
+  DCHECK(CalledOnValidThread());
+  DVLOG(1) << __FUNCTION__ << " " << state << " rlen=" << response_data.size();
+
+  switch (state) {
+    case TwoPhaseUploader::STATE_SUCCESS: {
+      ClientUploadResponse response;
+      if (!response.ParseFromString(response_data) ||
+          response.status() != ClientUploadResponse::SUCCESS)
+        RecordUploadResult(UPLOAD_COMPLETE_RESPONSE_ERROR);
+      else
+        RecordUploadResult(UPLOAD_SUCCESS);
+      break;
+    }
+    case TwoPhaseUploader::UPLOAD_FILE:
+      if (net_error != net::OK)
+        RecordUploadResult(UPLOAD_FILE_NET_ERROR);
+      else
+        RecordUploadResult(UPLOAD_FILE_RESPONSE_ERROR);
+      break;
+    case TwoPhaseUploader::UPLOAD_METADATA:
+      if (net_error != net::OK)
+        RecordUploadResult(UPLOAD_METADATA_NET_ERROR);
+      else
+        RecordUploadResult(UPLOAD_METADATA_RESPONSE_ERROR);
+      break;
+    default:
+      NOTREACHED();
+  }
+
+  uploader_.reset();
+
+  finish_callback.Run();
+  // We may be deleted here.
+}
+
+void DownloadFeedbackImpl::RecordUploadResult(UploadResultType result) {
+  if (result == UPLOAD_SUCCESS)
+    UMA_HISTOGRAM_CUSTOM_COUNTS(
+        "SBDownloadFeedback.SizeSuccess", file_size_, 1, kMaxUploadSize, 50);
+  else
+    UMA_HISTOGRAM_CUSTOM_COUNTS(
+        "SBDownloadFeedback.SizeFailure", file_size_, 1, kMaxUploadSize, 50);
+  UMA_HISTOGRAM_ENUMERATION(
+      "SBDownloadFeedback.UploadResult", result, UPLOAD_RESULT_MAX);
+}
+
+}  // namespace
+
+// static
+const int64 DownloadFeedback::kMaxUploadSize = 50 * 1024 * 1024;
+
+// static
+DownloadFeedbackFactory* DownloadFeedback::factory_ = NULL;
+
+// static
+DownloadFeedback* DownloadFeedback::Create(
+    net::URLRequestContextGetter* request_context_getter,
+    base::TaskRunner* file_task_runner,
+    const base::FilePath& file_path,
+    const std::string& ping_request,
+    const std::string& ping_response) {
+  if (!DownloadFeedback::factory_)
+    return new DownloadFeedbackImpl(
+        request_context_getter, file_task_runner, file_path, ping_request,
+        ping_response);
+  return DownloadFeedback::factory_->CreateDownloadFeedback(
+        request_context_getter, file_task_runner, file_path, ping_request,
+        ping_response);
+}
+
+}  // namespace safe_browsing
+
diff --git a/chrome/browser/safe_browsing/download_feedback.h b/chrome/browser/safe_browsing/download_feedback.h
new file mode 100644
index 0000000..2a9a6c7
--- /dev/null
+++ b/chrome/browser/safe_browsing/download_feedback.h
@@ -0,0 +1,79 @@
+// Copyright 2013 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_DOWNLOAD_FEEDBACK_H_
+#define CHROME_BROWSER_SAFE_BROWSING_DOWNLOAD_FEEDBACK_H_
+
+#include <string>
+
+#include "base/callback_forward.h"
+#include "base/files/file_path.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/threading/non_thread_safe.h"
+#include "chrome/browser/safe_browsing/two_phase_uploader.h"
+
+namespace content {
+class DownloadItem;
+}
+
+namespace safe_browsing {
+
+class DownloadFeedbackFactory;
+
+// Handles the uploading of a single downloaded binary to the safebrowsing
+// download feedback service.
+class DownloadFeedback : public base::NonThreadSafe {
+ public:
+  // Takes ownership of the file pointed to be |file_path|, it will be deleted
+  // when the DownloadFeedback is destructed.
+  static DownloadFeedback* Create(
+      net::URLRequestContextGetter* request_context_getter,
+      base::TaskRunner* file_task_runner,
+      const base::FilePath& file_path,
+      const std::string& ping_request,
+      const std::string& ping_response);
+
+  // The largest file size we support uploading.
+  // Note: changing this will affect the max size of
+  // SBDownloadFeedback.SizeSuccess and SizeFailure histograms.
+  static const int64 kMaxUploadSize;
+
+  virtual ~DownloadFeedback() {}
+
+  // Makes the passed |factory| the factory used to instantiate
+  // a DownloadFeedback. Useful for tests.
+  static void RegisterFactory(DownloadFeedbackFactory* factory) {
+    factory_ = factory;
+  }
+
+  // Start uploading the file to the download feedback service.
+  // |finish_callback| will be called when the upload completes or fails, but is
+  // not called if the upload is cancelled by deleting the DownloadFeedback
+  // while the upload is in progress.
+  virtual void Start(const base::Closure& finish_callback) = 0;
+
+  virtual const std::string& GetPingRequestForTesting() const = 0;
+  virtual const std::string& GetPingResponseForTesting() const = 0;
+
+ private:
+  // The factory that controls the creation of DownloadFeedback objects.
+  // This is used by tests.
+  static DownloadFeedbackFactory* factory_;
+};
+
+class DownloadFeedbackFactory {
+ public:
+  virtual ~DownloadFeedbackFactory() {}
+
+  virtual DownloadFeedback* CreateDownloadFeedback(
+      net::URLRequestContextGetter* request_context_getter,
+      base::TaskRunner* file_task_runner,
+      const base::FilePath& file_path,
+      const std::string& ping_request,
+      const std::string& ping_response) = 0;
+};
+
+}  // namespace safe_browsing
+
+#endif  // CHROME_BROWSER_SAFE_BROWSING_DOWNLOAD_FEEDBACK_H_
diff --git a/chrome/browser/safe_browsing/download_feedback_service.cc b/chrome/browser/safe_browsing/download_feedback_service.cc
new file mode 100644
index 0000000..3e5b378
--- /dev/null
+++ b/chrome/browser/safe_browsing/download_feedback_service.cc
@@ -0,0 +1,216 @@
+// Copyright 2013 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/download_feedback_service.h"
+
+#include "base/bind.h"
+#include "base/command_line.h"
+#include "base/files/file_path.h"
+#include "base/files/file_util_proxy.h"
+#include "base/metrics/histogram.h"
+#include "base/supports_user_data.h"
+#include "base/task_runner.h"
+#include "chrome/browser/safe_browsing/download_feedback.h"
+#include "chrome/common/chrome_switches.h"
+#include "chrome/common/chrome_version_info.h"
+#include "content/public/browser/download_danger_type.h"
+#include "content/public/browser/download_item.h"
+
+namespace safe_browsing {
+
+namespace {
+
+const void* kPingKey = &kPingKey;
+
+bool IsEnabled() {
+  CommandLine* cmdline = CommandLine::ForCurrentProcess();
+  if (cmdline->HasSwitch(switches::kSbEnableDownloadFeedback))
+    return true;
+
+  chrome::VersionInfo::Channel channel = chrome::VersionInfo::GetChannel();
+  if (channel == chrome::VersionInfo::CHANNEL_UNKNOWN ||
+      channel == chrome::VersionInfo::CHANNEL_CANARY ||
+      channel == chrome::VersionInfo::CHANNEL_DEV)
+    return true;
+
+  return false;
+}
+
+class DownloadFeedbackPings : public base::SupportsUserData::Data {
+ public:
+  DownloadFeedbackPings(const std::string& ping_request,
+                        const std::string& ping_response);
+
+  // Stores the ping data in the given |download|.
+  static void CreateForDownload(content::DownloadItem* download,
+                                const std::string& ping_request,
+                                const std::string& ping_response);
+
+  // Returns the DownloadFeedbackPings object associated with |download|.  May
+  // return NULL.
+  static DownloadFeedbackPings* FromDownload(
+      const content::DownloadItem& download);
+
+
+  const std::string& ping_request() const {
+    return ping_request_;
+  }
+
+  const std::string& ping_response() const {
+    return ping_response_;
+  }
+
+ private:
+  std::string ping_request_;
+  std::string ping_response_;
+};
+
+DownloadFeedbackPings::DownloadFeedbackPings(const std::string& ping_request,
+                                             const std::string& ping_response)
+    : ping_request_(ping_request),
+      ping_response_(ping_response) {
+}
+
+// static
+void DownloadFeedbackPings::CreateForDownload(
+    content::DownloadItem* download,
+    const std::string& ping_request,
+    const std::string& ping_response) {
+  DownloadFeedbackPings* pings = new DownloadFeedbackPings(ping_request,
+                                                           ping_response);
+  download->SetUserData(kPingKey, pings);
+}
+
+// static
+DownloadFeedbackPings* DownloadFeedbackPings::FromDownload(
+    const content::DownloadItem& download) {
+  return static_cast<DownloadFeedbackPings*>(download.GetUserData(kPingKey));
+}
+
+}  // namespace
+
+DownloadFeedbackService::DownloadFeedbackService(
+    net::URLRequestContextGetter* request_context_getter,
+    base::TaskRunner* file_task_runner)
+    : request_context_getter_(request_context_getter),
+      file_task_runner_(file_task_runner),
+      weak_ptr_factory_(this) {
+}
+
+DownloadFeedbackService::~DownloadFeedbackService() {
+  DCHECK(CalledOnValidThread());
+}
+
+// static
+void DownloadFeedbackService::MaybeStorePingsForDownload(
+    DownloadProtectionService::DownloadCheckResult result,
+    content::DownloadItem* download,
+    const std::string& ping,
+    const std::string& response) {
+  if (!IsEnabled() || !(result == DownloadProtectionService::UNCOMMON ||
+                        result == DownloadProtectionService::DANGEROUS_HOST))
+    return;
+  UMA_HISTOGRAM_COUNTS("SBDownloadFeedback.SizeEligibleKB",
+                       download->GetReceivedBytes() / 1024);
+  if (download->GetReceivedBytes() > DownloadFeedback::kMaxUploadSize)
+    return;
+
+  DownloadFeedbackPings::CreateForDownload(download, ping, response);
+}
+
+// static
+bool DownloadFeedbackService::IsEnabledForDownload(
+    const content::DownloadItem& download) {
+  return !!DownloadFeedbackPings::FromDownload(download);
+}
+
+// static
+bool DownloadFeedbackService::GetPingsForDownloadForTesting(
+    const content::DownloadItem& download,
+    std::string* ping,
+    std::string* response) {
+  DownloadFeedbackPings* pings = DownloadFeedbackPings::FromDownload(download);
+  if (!pings)
+    return false;
+
+  *ping = pings->ping_request();
+  *response = pings->ping_response();
+  return true;
+}
+
+// static
+void DownloadFeedbackService::RecordFeedbackButtonShown(
+    content::DownloadDangerType danger_type) {
+  UMA_HISTOGRAM_ENUMERATION("SBDownloadFeedback.Shown",
+                            danger_type,
+                            content::DOWNLOAD_DANGER_TYPE_MAX);
+}
+
+
+void DownloadFeedbackService::BeginFeedbackForDownload(
+    content::DownloadItem* download) {
+  DCHECK(CalledOnValidThread());
+
+  UMA_HISTOGRAM_ENUMERATION("SBDownloadFeedback.Activations",
+                            download->GetDangerType(),
+                            content::DOWNLOAD_DANGER_TYPE_MAX);
+
+  DownloadFeedbackPings* pings = DownloadFeedbackPings::FromDownload(*download);
+  DCHECK(pings);
+
+  download->StealDangerousDownload(
+      base::Bind(&DownloadFeedbackService::BeginFeedbackOrDeleteFile,
+                 file_task_runner_,
+                 weak_ptr_factory_.GetWeakPtr(),
+                 pings->ping_request(),
+                 pings->ping_response()));
+}
+
+// static
+void DownloadFeedbackService::BeginFeedbackOrDeleteFile(
+    const scoped_refptr<base::TaskRunner>& file_task_runner,
+    const base::WeakPtr<DownloadFeedbackService>& service,
+    const std::string& ping_request,
+    const std::string& ping_response,
+    const base::FilePath& path) {
+  if (service) {
+    service->BeginFeedback(ping_request, ping_response, path);
+  } else {
+    base::FileUtilProxy::Delete(file_task_runner, path, false,
+                                base::FileUtilProxy::StatusCallback());
+  }
+}
+
+void DownloadFeedbackService::StartPendingFeedback() {
+  DCHECK(!active_feedback_.empty());
+  active_feedback_.front()->Start(base::Bind(
+      &DownloadFeedbackService::FeedbackComplete, base::Unretained(this)));
+}
+
+void DownloadFeedbackService::BeginFeedback(
+    const std::string& ping_request,
+    const std::string& ping_response,
+    const base::FilePath& path) {
+  DCHECK(CalledOnValidThread());
+  DownloadFeedback* feedback = DownloadFeedback::Create(
+      request_context_getter_, file_task_runner_, path,
+      ping_request, ping_response);
+  active_feedback_.push_back(feedback);
+  UMA_HISTOGRAM_COUNTS_100("SBDownloadFeedback.ActiveFeedbacks",
+                           active_feedback_.size());
+
+  if (active_feedback_.size() == 1)
+    StartPendingFeedback();
+}
+
+void DownloadFeedbackService::FeedbackComplete() {
+  DVLOG(1) << __FUNCTION__;
+  DCHECK(CalledOnValidThread());
+  DCHECK(!active_feedback_.empty());
+  active_feedback_.erase(active_feedback_.begin());
+  if (!active_feedback_.empty())
+    StartPendingFeedback();
+}
+
+}  // namespace safe_browsing
diff --git a/chrome/browser/safe_browsing/download_feedback_service.h b/chrome/browser/safe_browsing/download_feedback_service.h
new file mode 100644
index 0000000..bbfc211
--- /dev/null
+++ b/chrome/browser/safe_browsing/download_feedback_service.h
@@ -0,0 +1,96 @@
+// Copyright 2013 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_DOWNLOAD_FEEDBACK_SERVICE_H_
+#define CHROME_BROWSER_SAFE_BROWSING_DOWNLOAD_FEEDBACK_SERVICE_H_
+
+#include <string>
+
+#include "base/basictypes.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_vector.h"
+#include "base/threading/non_thread_safe.h"
+#include "chrome/browser/safe_browsing/download_protection_service.h"
+#include "content/public/browser/download_danger_type.h"
+
+namespace base {
+class TaskRunner;
+}
+
+namespace content {
+class DownloadItem;
+}
+
+namespace net {
+class URLRequestContextGetter;
+}
+
+namespace safe_browsing {
+
+class DownloadFeedback;
+
+// Tracks active DownloadFeedback objects, provides interface for storing ping
+// data for malicious downloads.
+class DownloadFeedbackService : public base::NonThreadSafe {
+ public:
+  DownloadFeedbackService(net::URLRequestContextGetter* request_context_getter,
+                          base::TaskRunner* file_task_runner);
+  ~DownloadFeedbackService();
+
+  // Stores the request and response ping data from the download check, if the
+  // check result and file size are eligible. This must be called after a
+  // download has been flagged as malicious in order for the download to be
+  // enabled for uploading.
+  static void MaybeStorePingsForDownload(
+      DownloadProtectionService::DownloadCheckResult result,
+      content::DownloadItem* download,
+      const std::string& ping,
+      const std::string& response);
+
+  // Test if pings have been stored for |download|.
+  static bool IsEnabledForDownload(const content::DownloadItem& download);
+
+  // Get the ping values stored in |download|. Returns false if no ping values
+  // are present.
+  static bool GetPingsForDownloadForTesting(
+      const content::DownloadItem& download,
+      std::string* ping,
+      std::string* response);
+
+  // Records histogram for download feedback option shown to user.
+  static void RecordFeedbackButtonShown(
+      content::DownloadDangerType danger_type);
+
+  // Begin download feedback for |download|. The |download| will be deleted
+  // when this function returns. This must only be called if
+  // IsEnabledForDownload is true for |download|.
+  void BeginFeedbackForDownload(content::DownloadItem* download);
+
+ private:
+  static void BeginFeedbackOrDeleteFile(
+      const scoped_refptr<base::TaskRunner>& file_task_runner,
+      const base::WeakPtr<DownloadFeedbackService>& service,
+      const std::string& ping_request,
+      const std::string& ping_response,
+      const base::FilePath& path);
+  void StartPendingFeedback();
+  void BeginFeedback(const std::string& ping_request,
+                     const std::string& ping_response,
+                     const base::FilePath& path);
+  void FeedbackComplete();
+
+  scoped_refptr<net::URLRequestContextGetter> request_context_getter_;
+  scoped_refptr<base::TaskRunner> file_task_runner_;
+
+  // Currently active & pending uploads. The first item is active, remaining
+  // items are pending.
+  ScopedVector<DownloadFeedback> active_feedback_;
+
+  base::WeakPtrFactory<DownloadFeedbackService> weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(DownloadFeedbackService);
+};
+}  // namespace safe_browsing
+
+#endif  // CHROME_BROWSER_SAFE_BROWSING_DOWNLOAD_FEEDBACK_SERVICE_H_
diff --git a/chrome/browser/safe_browsing/download_feedback_service_unittest.cc b/chrome/browser/safe_browsing/download_feedback_service_unittest.cc
new file mode 100644
index 0000000..8f0a874
--- /dev/null
+++ b/chrome/browser/safe_browsing/download_feedback_service_unittest.cc
@@ -0,0 +1,374 @@
+// Copyright 2013 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/download_feedback_service.h"
+
+#include "base/command_line.h"
+#include "base/file_util.h"
+#include "base/files/scoped_temp_dir.h"
+#include "base/run_loop.h"
+#include "base/strings/string_number_conversions.h"
+#include "chrome/browser/safe_browsing/download_feedback.h"
+#include "chrome/common/chrome_switches.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/test/mock_download_item.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "net/url_request/url_request_test_util.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using ::testing::_;
+using ::testing::Return;
+using ::testing::SaveArg;
+
+namespace safe_browsing {
+
+namespace {
+
+class FakeDownloadFeedback : public DownloadFeedback {
+ public:
+  FakeDownloadFeedback(net::URLRequestContextGetter* request_context_getter,
+                       base::TaskRunner* file_task_runner,
+                       const base::FilePath& file_path,
+                       const std::string& ping_request,
+                       const std::string& ping_response,
+                       base::Closure deletion_callback)
+      : ping_request_(ping_request),
+        ping_response_(ping_response),
+        deletion_callback_(deletion_callback),
+        start_called_(false) {
+  }
+
+  virtual ~FakeDownloadFeedback() {
+    deletion_callback_.Run();
+  }
+
+  virtual void Start(const base::Closure& finish_callback) OVERRIDE {
+    start_called_ = true;
+    finish_callback_ = finish_callback;
+  }
+
+  virtual const std::string& GetPingRequestForTesting() const OVERRIDE {
+    return ping_request_;
+  }
+
+  virtual const std::string& GetPingResponseForTesting() const OVERRIDE {
+    return ping_response_;
+  }
+
+  base::Closure finish_callback() const {
+    return finish_callback_;
+  }
+
+  bool start_called() const {
+    return start_called_;
+  }
+
+ private:
+  scoped_refptr<net::URLRequestContextGetter> request_context_getter_;
+  scoped_refptr<base::TaskRunner> file_task_runner_;
+  base::FilePath file_path_;
+  std::string ping_request_;
+  std::string ping_response_;
+
+  base::Closure finish_callback_;
+  base::Closure deletion_callback_;
+  bool start_called_;
+};
+
+class FakeDownloadFeedbackFactory : public DownloadFeedbackFactory {
+ public:
+  virtual ~FakeDownloadFeedbackFactory() {}
+
+  virtual DownloadFeedback* CreateDownloadFeedback(
+      net::URLRequestContextGetter* request_context_getter,
+      base::TaskRunner* file_task_runner,
+      const base::FilePath& file_path,
+      const std::string& ping_request,
+      const std::string& ping_response) OVERRIDE {
+    FakeDownloadFeedback* feedback = new FakeDownloadFeedback(
+        request_context_getter,
+        file_task_runner,
+        file_path,
+        ping_request,
+        ping_response,
+        base::Bind(&FakeDownloadFeedbackFactory::DownloadFeedbackDeleted,
+                   base::Unretained(this),
+                   feedbacks_.size()));
+    feedbacks_.push_back(feedback);
+    return feedback;
+  }
+
+  void DownloadFeedbackDeleted(size_t n) {
+    feedbacks_[n] = NULL;
+  }
+
+  FakeDownloadFeedback* feedback(size_t n) const {
+    return feedbacks_[n];
+  }
+
+  size_t num_feedbacks() const {
+    return feedbacks_.size();
+  }
+
+ private:
+  std::vector<FakeDownloadFeedback*> feedbacks_;
+};
+
+bool WillStorePings(DownloadProtectionService::DownloadCheckResult result,
+                     int64 size) {
+  content::MockDownloadItem item;
+  EXPECT_CALL(item, GetReceivedBytes()).WillRepeatedly(Return(size));
+
+  EXPECT_FALSE(DownloadFeedbackService::IsEnabledForDownload(item));
+  DownloadFeedbackService::MaybeStorePingsForDownload(result, &item, "a", "b");
+  return DownloadFeedbackService::IsEnabledForDownload(item);
+}
+
+}  // namespace
+
+class DownloadFeedbackServiceTest : public testing::Test {
+ public:
+  DownloadFeedbackServiceTest()
+      : file_task_runner_(content::BrowserThread::GetMessageLoopProxyForThread(
+            content::BrowserThread::FILE)),
+        io_task_runner_(content::BrowserThread::GetMessageLoopProxyForThread(
+            content::BrowserThread::IO)),
+        request_context_getter_(
+            new net::TestURLRequestContextGetter(io_task_runner_)) {
+  }
+
+  virtual void SetUp() OVERRIDE {
+    ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
+    CommandLine::ForCurrentProcess()->AppendSwitch(
+        switches::kSbEnableDownloadFeedback);
+    DownloadFeedback::RegisterFactory(&download_feedback_factory_);
+  }
+
+  virtual void TearDown() OVERRIDE {
+    DownloadFeedback::RegisterFactory(NULL);
+  }
+
+  base::FilePath CreateTestFile(int n) const {
+    base::FilePath upload_file_path(
+        temp_dir_.path().AppendASCII("test file " + base::IntToString(n)));
+    const std::string upload_file_data = "data";
+    int wrote = file_util::WriteFile(
+        upload_file_path, upload_file_data.data(), upload_file_data.size());
+    EXPECT_EQ(static_cast<int>(upload_file_data.size()), wrote);
+    return upload_file_path;
+  }
+
+  FakeDownloadFeedback* feedback(size_t n) const {
+    return download_feedback_factory_.feedback(n);
+  }
+
+  size_t num_feedbacks() const {
+    return download_feedback_factory_.num_feedbacks();
+  }
+ protected:
+  base::ScopedTempDir temp_dir_;
+  content::TestBrowserThreadBundle thread_bundle_;
+  scoped_refptr<base::SingleThreadTaskRunner> file_task_runner_;
+  scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_;
+  scoped_refptr<net::TestURLRequestContextGetter> request_context_getter_;
+  FakeDownloadFeedbackFactory download_feedback_factory_;
+
+};
+
+TEST_F(DownloadFeedbackServiceTest, MaybeStorePingsForDownload) {
+  const int64 ok_size = DownloadFeedback::kMaxUploadSize;
+  const int64 bad_size = DownloadFeedback::kMaxUploadSize + 1;
+
+  EXPECT_FALSE(WillStorePings(DownloadProtectionService::SAFE, ok_size));
+  EXPECT_FALSE(WillStorePings(DownloadProtectionService::DANGEROUS, ok_size));
+  EXPECT_TRUE(WillStorePings(DownloadProtectionService::UNCOMMON, ok_size));
+  EXPECT_TRUE(
+      WillStorePings(DownloadProtectionService::DANGEROUS_HOST, ok_size));
+
+  EXPECT_FALSE(WillStorePings(DownloadProtectionService::SAFE, bad_size));
+  EXPECT_FALSE(WillStorePings(DownloadProtectionService::DANGEROUS, bad_size));
+  EXPECT_FALSE(WillStorePings(DownloadProtectionService::UNCOMMON, bad_size));
+  EXPECT_FALSE(
+      WillStorePings(DownloadProtectionService::DANGEROUS_HOST, bad_size));
+}
+
+TEST_F(DownloadFeedbackServiceTest, SingleFeedbackComplete) {
+  const base::FilePath file_path(CreateTestFile(0));
+  const std::string ping_request = "ping";
+  const std::string ping_response = "resp";
+
+  content::DownloadItem::AcquireFileCallback download_discarded_callback;
+
+  content::MockDownloadItem item;
+  EXPECT_CALL(item, GetDangerType())
+      .WillRepeatedly(Return(content::DOWNLOAD_DANGER_TYPE_UNCOMMON_CONTENT));
+  EXPECT_CALL(item, GetReceivedBytes()).WillRepeatedly(Return(1000));
+  EXPECT_CALL(item, StealDangerousDownload(_))
+      .WillOnce(SaveArg<0>(&download_discarded_callback));
+
+  DownloadFeedbackService service(request_context_getter_, file_task_runner_);
+  service.MaybeStorePingsForDownload(
+      DownloadProtectionService::UNCOMMON, &item, ping_request, ping_response);
+  ASSERT_TRUE(DownloadFeedbackService::IsEnabledForDownload(item));
+  service.BeginFeedbackForDownload(&item);
+  ASSERT_FALSE(download_discarded_callback.is_null());
+  EXPECT_EQ(0U, num_feedbacks());
+
+  download_discarded_callback.Run(file_path);
+  ASSERT_EQ(1U, num_feedbacks());
+  ASSERT_TRUE(feedback(0));
+  EXPECT_TRUE(feedback(0)->start_called());
+  EXPECT_EQ(ping_request, feedback(0)->GetPingRequestForTesting());
+  EXPECT_EQ(ping_response, feedback(0)->GetPingResponseForTesting());
+
+  feedback(0)->finish_callback().Run();
+  EXPECT_FALSE(feedback(0));
+
+  // File should still exist since our FakeDownloadFeedback does not delete it.
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(file_util::PathExists(file_path));
+}
+
+TEST_F(DownloadFeedbackServiceTest, MultiplePendingFeedbackComplete) {
+  const std::string ping_request = "ping";
+  const std::string ping_response = "resp";
+  const size_t num_downloads = 3;
+
+  content::DownloadItem::AcquireFileCallback
+      download_discarded_callback[num_downloads];
+
+  base::FilePath file_path[num_downloads];
+  content::MockDownloadItem item[num_downloads];
+  for (size_t i = 0; i < num_downloads; ++i) {
+    file_path[i] = CreateTestFile(i);
+    EXPECT_CALL(item[i], GetDangerType())
+        .WillRepeatedly(Return(content::DOWNLOAD_DANGER_TYPE_UNCOMMON_CONTENT));
+    EXPECT_CALL(item[i], GetReceivedBytes()).WillRepeatedly(Return(1000));
+    EXPECT_CALL(item[i], StealDangerousDownload(_))
+        .WillOnce(SaveArg<0>(&download_discarded_callback[i]));
+    DownloadFeedbackService::MaybeStorePingsForDownload(
+        DownloadProtectionService::UNCOMMON, &item[i], ping_request,
+        ping_response);
+    ASSERT_TRUE(DownloadFeedbackService::IsEnabledForDownload(item[i]));
+  }
+
+  {
+    DownloadFeedbackService service(request_context_getter_, file_task_runner_);
+    for (size_t i = 0; i < num_downloads; ++i) {
+      SCOPED_TRACE(i);
+      service.BeginFeedbackForDownload(&item[i]);
+      ASSERT_FALSE(download_discarded_callback[i].is_null());
+    }
+    EXPECT_EQ(0U, num_feedbacks());
+
+    for (size_t i = 0; i < num_downloads; ++i) {
+      download_discarded_callback[i].Run(file_path[i]);
+    }
+
+    ASSERT_EQ(3U, num_feedbacks());
+    EXPECT_TRUE(feedback(0)->start_called());
+    EXPECT_FALSE(feedback(1)->start_called());
+    EXPECT_FALSE(feedback(2)->start_called());
+
+    feedback(0)->finish_callback().Run();
+
+    EXPECT_FALSE(feedback(0));
+    EXPECT_TRUE(feedback(1)->start_called());
+    EXPECT_FALSE(feedback(2)->start_called());
+
+    feedback(1)->finish_callback().Run();
+
+    EXPECT_FALSE(feedback(0));
+    EXPECT_FALSE(feedback(1));
+    EXPECT_TRUE(feedback(2)->start_called());
+
+    feedback(2)->finish_callback().Run();
+
+    EXPECT_FALSE(feedback(0));
+    EXPECT_FALSE(feedback(1));
+    EXPECT_FALSE(feedback(2));
+  }
+
+  base::RunLoop().RunUntilIdle();
+  // These files should still exist since the FakeDownloadFeedback does not
+  // delete them.
+  EXPECT_TRUE(file_util::PathExists(file_path[0]));
+  EXPECT_TRUE(file_util::PathExists(file_path[1]));
+  EXPECT_TRUE(file_util::PathExists(file_path[2]));
+}
+
+TEST_F(DownloadFeedbackServiceTest, MultiFeedbackWithIncomplete) {
+  const std::string ping_request = "ping";
+  const std::string ping_response = "resp";
+  const size_t num_downloads = 3;
+
+  content::DownloadItem::AcquireFileCallback
+      download_discarded_callback[num_downloads];
+
+  base::FilePath file_path[num_downloads];
+  content::MockDownloadItem item[num_downloads];
+  for (size_t i = 0; i < num_downloads; ++i) {
+    file_path[i] = CreateTestFile(i);
+    EXPECT_CALL(item[i], GetDangerType())
+        .WillRepeatedly(Return(content::DOWNLOAD_DANGER_TYPE_UNCOMMON_CONTENT));
+    EXPECT_CALL(item[i], GetReceivedBytes()).WillRepeatedly(Return(1000));
+    EXPECT_CALL(item[i], StealDangerousDownload(_))
+        .WillOnce(SaveArg<0>(&download_discarded_callback[i]));
+    DownloadFeedbackService::MaybeStorePingsForDownload(
+        DownloadProtectionService::UNCOMMON, &item[i], ping_request,
+        ping_response);
+    ASSERT_TRUE(DownloadFeedbackService::IsEnabledForDownload(item[i]));
+  }
+
+  {
+    DownloadFeedbackService service(request_context_getter_, file_task_runner_);
+    for (size_t i = 0; i < num_downloads; ++i) {
+      SCOPED_TRACE(i);
+      service.BeginFeedbackForDownload(&item[i]);
+      ASSERT_FALSE(download_discarded_callback[i].is_null());
+    }
+    EXPECT_EQ(0U, num_feedbacks());
+
+    download_discarded_callback[0].Run(file_path[0]);
+    ASSERT_EQ(1U, num_feedbacks());
+    ASSERT_TRUE(feedback(0));
+    EXPECT_TRUE(feedback(0)->start_called());
+
+    download_discarded_callback[1].Run(file_path[1]);
+    ASSERT_EQ(2U, num_feedbacks());
+    ASSERT_TRUE(feedback(1));
+    EXPECT_FALSE(feedback(1)->start_called());
+
+    feedback(0)->finish_callback().Run();
+    EXPECT_FALSE(feedback(0));
+    EXPECT_TRUE(feedback(1)->start_called());
+  }
+
+  EXPECT_EQ(2U, num_feedbacks());
+  for (size_t i = 0; i < num_feedbacks(); ++i) {
+    SCOPED_TRACE(i);
+    EXPECT_FALSE(feedback(i));
+  }
+
+  // Running a download acquired callback after the DownloadFeedbackService is
+  // destroyed should delete the file.
+  download_discarded_callback[2].Run(file_path[2]);
+  EXPECT_EQ(2U, num_feedbacks());
+
+  // File should still exist since the FileUtilProxy task hasn't run yet.
+  EXPECT_TRUE(file_util::PathExists(file_path[2]));
+
+  base::RunLoop().RunUntilIdle();
+  // File should be deleted since the AcquireFileCallback ran after the service
+  // was deleted.
+  EXPECT_FALSE(file_util::PathExists(file_path[2]));
+
+  // These files should still exist since the FakeDownloadFeedback does not
+  // delete them.
+  EXPECT_TRUE(file_util::PathExists(file_path[0]));
+  EXPECT_TRUE(file_util::PathExists(file_path[1]));
+}
+
+}  // namespace safe_browsing
diff --git a/chrome/browser/safe_browsing/download_feedback_unittest.cc b/chrome/browser/safe_browsing/download_feedback_unittest.cc
new file mode 100644
index 0000000..4a83868
--- /dev/null
+++ b/chrome/browser/safe_browsing/download_feedback_unittest.cc
@@ -0,0 +1,242 @@
+// Copyright 2013 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/download_feedback.h"
+
+#include "base/command_line.h"
+#include "base/file_util.h"
+#include "base/files/scoped_temp_dir.h"
+#include "base/message_loop.h"
+#include "base/run_loop.h"
+#include "base/single_thread_task_runner.h"
+#include "chrome/browser/safe_browsing/two_phase_uploader.h"
+#include "chrome/common/chrome_switches.h"
+#include "chrome/common/safe_browsing/csd.pb.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "net/url_request/url_request_test_util.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace safe_browsing {
+
+namespace {
+
+class FakeUploader : public TwoPhaseUploader {
+ public:
+  FakeUploader(net::URLRequestContextGetter* url_request_context_getter,
+               base::TaskRunner* file_task_runner,
+               const GURL& base_url,
+               const std::string& metadata,
+               const base::FilePath& file_path,
+               const ProgressCallback& progress_callback,
+               const FinishCallback& finish_callback);
+  virtual ~FakeUploader() {}
+
+  virtual void Start() OVERRIDE {
+    start_called_ = true;
+  }
+
+  scoped_refptr<net::URLRequestContextGetter> url_request_context_getter_;
+  scoped_refptr<base::TaskRunner> file_task_runner_;
+  GURL base_url_;
+  std::string metadata_;
+  base::FilePath file_path_;
+  ProgressCallback progress_callback_;
+  FinishCallback finish_callback_;
+
+  bool start_called_;
+};
+
+FakeUploader::FakeUploader(
+    net::URLRequestContextGetter* url_request_context_getter,
+    base::TaskRunner* file_task_runner,
+    const GURL& base_url,
+    const std::string& metadata,
+    const base::FilePath& file_path,
+    const ProgressCallback& progress_callback,
+    const FinishCallback& finish_callback)
+    : url_request_context_getter_(url_request_context_getter),
+      file_task_runner_(file_task_runner),
+      base_url_(base_url),
+      metadata_(metadata),
+      file_path_(file_path),
+      progress_callback_(progress_callback),
+      finish_callback_(finish_callback),
+      start_called_(false) {
+}
+
+class FakeUploaderFactory : public TwoPhaseUploaderFactory {
+ public:
+  FakeUploaderFactory() : uploader_(NULL) {}
+  virtual ~FakeUploaderFactory() {}
+
+  virtual TwoPhaseUploader* CreateTwoPhaseUploader(
+      net::URLRequestContextGetter* url_request_context_getter,
+      base::TaskRunner* file_task_runner,
+      const GURL& base_url,
+      const std::string& metadata,
+      const base::FilePath& file_path,
+      const TwoPhaseUploader::ProgressCallback& progress_callback,
+      const TwoPhaseUploader::FinishCallback& finish_callback) OVERRIDE;
+
+  FakeUploader* uploader_;
+};
+
+TwoPhaseUploader* FakeUploaderFactory::CreateTwoPhaseUploader(
+    net::URLRequestContextGetter* url_request_context_getter,
+    base::TaskRunner* file_task_runner,
+    const GURL& base_url,
+    const std::string& metadata,
+    const base::FilePath& file_path,
+    const TwoPhaseUploader::ProgressCallback& progress_callback,
+    const TwoPhaseUploader::FinishCallback& finish_callback) {
+  EXPECT_FALSE(uploader_);
+
+  uploader_ = new FakeUploader(url_request_context_getter, file_task_runner,
+                               base_url, metadata, file_path, progress_callback,
+                               finish_callback);
+  return uploader_;
+}
+
+}  // namespace
+
+class DownloadFeedbackTest : public testing::Test {
+ public:
+  DownloadFeedbackTest()
+      : file_task_runner_(content::BrowserThread::GetMessageLoopProxyForThread(
+            content::BrowserThread::FILE)),
+        io_task_runner_(content::BrowserThread::GetMessageLoopProxyForThread(
+            content::BrowserThread::IO)),
+        url_request_context_getter_(
+            new net::TestURLRequestContextGetter(io_task_runner_)),
+        feedback_finish_called_(false) {
+    EXPECT_NE(io_task_runner_, file_task_runner_);
+  }
+
+  virtual void SetUp() OVERRIDE {
+    ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
+    upload_file_path_ = temp_dir_.path().AppendASCII("test file");
+    upload_file_data_ = "data";
+    ASSERT_EQ(static_cast<int>(upload_file_data_.size()),
+              file_util::WriteFile(upload_file_path_, upload_file_data_.data(),
+                                   upload_file_data_.size()));
+    TwoPhaseUploader::RegisterFactory(&two_phase_uploader_factory_);
+  }
+
+  virtual void TearDown() OVERRIDE {
+    TwoPhaseUploader::RegisterFactory(NULL);
+  }
+
+  FakeUploader* uploader() const {
+    return two_phase_uploader_factory_.uploader_;
+  }
+
+  void FinishCallback(DownloadFeedback* feedback) {
+    EXPECT_FALSE(feedback_finish_called_);
+    feedback_finish_called_ = true;
+    delete feedback;
+  }
+
+ protected:
+  base::ScopedTempDir temp_dir_;
+  base::FilePath upload_file_path_;
+  std::string upload_file_data_;
+  content::TestBrowserThreadBundle thread_bundle_;
+  scoped_refptr<base::SingleThreadTaskRunner> file_task_runner_;
+  scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_;
+  FakeUploaderFactory two_phase_uploader_factory_;
+  scoped_refptr<net::TestURLRequestContextGetter> url_request_context_getter_;
+
+  bool feedback_finish_called_;
+};
+
+TEST_F(DownloadFeedbackTest, CompleteUpload) {
+  ClientDownloadReport expected_report_metadata;
+  expected_report_metadata.mutable_download_request()->set_url("http://test");
+  expected_report_metadata.mutable_download_request()->set_length(
+      upload_file_data_.size());
+  expected_report_metadata.mutable_download_request()->mutable_digests(
+      )->set_sha1("hi");
+  expected_report_metadata.mutable_download_response()->set_verdict(
+      ClientDownloadResponse::DANGEROUS_HOST);
+  std::string ping_request(
+      expected_report_metadata.download_request().SerializeAsString());
+  std::string ping_response(
+      expected_report_metadata.download_response().SerializeAsString());
+
+  const char kTestFeedbackURL[] = "https://example.com/test/upload";
+  CommandLine::ForCurrentProcess()->AppendSwitchASCII(
+      switches::kSbDownloadFeedbackURL, kTestFeedbackURL);
+
+  DownloadFeedback* feedback = DownloadFeedback::Create(
+      url_request_context_getter_,
+      file_task_runner_,
+      upload_file_path_,
+      ping_request,
+      ping_response);
+  EXPECT_FALSE(uploader());
+
+  feedback->Start(base::Bind(&DownloadFeedbackTest::FinishCallback,
+                             base::Unretained(this),
+                             feedback));
+  ASSERT_TRUE(uploader());
+  EXPECT_FALSE(feedback_finish_called_);
+  EXPECT_TRUE(uploader()->start_called_);
+
+  EXPECT_EQ(url_request_context_getter_,
+            uploader()->url_request_context_getter_);
+  EXPECT_EQ(file_task_runner_, uploader()->file_task_runner_);
+  EXPECT_EQ(upload_file_path_, uploader()->file_path_);
+  EXPECT_EQ(expected_report_metadata.SerializeAsString(),
+            uploader()->metadata_);
+  EXPECT_EQ(kTestFeedbackURL, uploader()->base_url_.spec());
+
+  EXPECT_TRUE(file_util::PathExists(upload_file_path_));
+
+  EXPECT_FALSE(feedback_finish_called_);
+  uploader()->finish_callback_.Run(
+      TwoPhaseUploader::STATE_SUCCESS, net::OK, 0, "");
+  EXPECT_TRUE(feedback_finish_called_);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_FALSE(file_util::PathExists(upload_file_path_));
+}
+
+TEST_F(DownloadFeedbackTest, CancelUpload) {
+  ClientDownloadReport expected_report_metadata;
+  expected_report_metadata.mutable_download_request()->set_url("http://test");
+  expected_report_metadata.mutable_download_request()->set_length(
+      upload_file_data_.size());
+  expected_report_metadata.mutable_download_request()->mutable_digests(
+      )->set_sha1("hi");
+  expected_report_metadata.mutable_download_response()->set_verdict(
+      ClientDownloadResponse::DANGEROUS_HOST);
+  std::string ping_request(
+      expected_report_metadata.download_request().SerializeAsString());
+  std::string ping_response(
+      expected_report_metadata.download_response().SerializeAsString());
+
+  DownloadFeedback* feedback = DownloadFeedback::Create(
+      url_request_context_getter_,
+      file_task_runner_,
+      upload_file_path_,
+      ping_request,
+      ping_response);
+  EXPECT_FALSE(uploader());
+
+  feedback->Start(base::Bind(&DownloadFeedbackTest::FinishCallback,
+                             base::Unretained(this),
+                             feedback));
+  ASSERT_TRUE(uploader());
+  EXPECT_FALSE(feedback_finish_called_);
+  EXPECT_TRUE(uploader()->start_called_);
+  EXPECT_TRUE(file_util::PathExists(upload_file_path_));
+
+  delete feedback;
+  EXPECT_FALSE(feedback_finish_called_);
+
+  base::RunLoop().RunUntilIdle();
+  EXPECT_FALSE(file_util::PathExists(upload_file_path_));
+}
+
+}  // namespace safe_browsing
diff --git a/chrome/browser/safe_browsing/download_protection_service.cc b/chrome/browser/safe_browsing/download_protection_service.cc
index 901d5e9..9fe83807 100644
--- a/chrome/browser/safe_browsing/download_protection_service.cc
+++ b/chrome/browser/safe_browsing/download_protection_service.cc
@@ -17,6 +17,7 @@
 #include "base/strings/stringprintf.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "base/time.h"
+#include "chrome/browser/safe_browsing/download_feedback_service.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/browser/safe_browsing/sandboxed_zip_analyzer.h"
 #include "chrome/browser/safe_browsing/signature_util.h"
@@ -421,6 +422,8 @@ class DownloadProtectionService::CheckClientDownloadRequest
                     << response.verdict();
         reason = REASON_INVALID_RESPONSE_VERDICT;
       }
+      DownloadFeedbackService::MaybeStorePingsForDownload(
+          result, item_, client_download_request_data_, data);
     }
     // We don't need the fetcher anymore.
     fetcher_.reset();
@@ -636,8 +639,7 @@ class DownloadProtectionService::CheckClientDownloadRequest
         item_->GetTargetFilePath().BaseName().AsUTF8Unsafe());
     request.set_download_type(type_);
     request.mutable_signature()->CopyFrom(signature_info_);
-    std::string request_data;
-    if (!request.SerializeToString(&request_data)) {
+    if (!request.SerializeToString(&client_download_request_data_)) {
       FinishRequest(SAFE, REASON_INVALID_REQUEST_PROTO);
       return;
     }
@@ -651,7 +653,8 @@ class DownloadProtectionService::CheckClientDownloadRequest
     fetcher_->SetLoadFlags(net::LOAD_DISABLE_CACHE);
     fetcher_->SetAutomaticallyRetryOn5xx(false);  // Don't retry on error.
     fetcher_->SetRequestContext(service_->request_context_getter_.get());
-    fetcher_->SetUploadData("application/octet-stream", request_data);
+    fetcher_->SetUploadData("application/octet-stream",
+                            client_download_request_data_);
     fetcher_->Start();
   }
 
@@ -754,6 +757,7 @@ class DownloadProtectionService::CheckClientDownloadRequest
   base::TimeTicks zip_analysis_start_time_;
   bool finished_;
   ClientDownloadRequest::DownloadType type_;
+  std::string client_download_request_data_;
   base::WeakPtrFactory<CheckClientDownloadRequest> weakptr_factory_;
   base::TimeTicks start_time_;  // Used for stats.
 
@@ -766,7 +770,9 @@ DownloadProtectionService::DownloadProtectionService(
     : request_context_getter_(request_context_getter),
       enabled_(false),
       signature_util_(new SignatureUtil()),
-      download_request_timeout_ms_(kDownloadRequestTimeoutMs) {
+      download_request_timeout_ms_(kDownloadRequestTimeoutMs),
+      feedback_service_(new DownloadFeedbackService(
+          request_context_getter, BrowserThread::GetBlockingPool())) {
 
   if (sb_service) {
     ui_manager_ = sb_service->ui_manager();
diff --git a/chrome/browser/safe_browsing/download_protection_service.h b/chrome/browser/safe_browsing/download_protection_service.h
index d7407b2..045bb2f 100644
--- a/chrome/browser/safe_browsing/download_protection_service.h
+++ b/chrome/browser/safe_browsing/download_protection_service.h
@@ -33,6 +33,7 @@ class X509Certificate;
 }  // namespace net
 
 namespace safe_browsing {
+class DownloadFeedbackService;
 class SignatureUtil;
 
 // This class provides an asynchronous API to check whether a particular
@@ -100,6 +101,10 @@ class DownloadProtectionService {
     return download_request_timeout_ms_;
   }
 
+  DownloadFeedbackService* feedback_service() {
+    return feedback_service_.get();
+  }
+
  protected:
   // Enum to keep track why a particular download verdict was chosen.
   // This is used to keep some stats around.
@@ -186,6 +191,8 @@ class DownloadProtectionService {
 
   int64 download_request_timeout_ms_;
 
+  scoped_ptr<DownloadFeedbackService> feedback_service_;
+
   DISALLOW_COPY_AND_ASSIGN(DownloadProtectionService);
 };
 }  // namespace safe_browsing
diff --git a/chrome/browser/safe_browsing/download_protection_service_unittest.cc b/chrome/browser/safe_browsing/download_protection_service_unittest.cc
index e693429..e5baf4c 100644
--- a/chrome/browser/safe_browsing/download_protection_service_unittest.cc
+++ b/chrome/browser/safe_browsing/download_protection_service_unittest.cc
@@ -10,6 +10,7 @@
 #include "base/base_paths.h"
 #include "base/bind.h"
 #include "base/callback.h"
+#include "base/command_line.h"
 #include "base/file_util.h"
 #include "base/files/file_path.h"
 #include "base/files/scoped_temp_dir.h"
@@ -20,8 +21,10 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "chrome/browser/safe_browsing/database_manager.h"
+#include "chrome/browser/safe_browsing/download_feedback_service.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/browser/safe_browsing/signature_util.h"
+#include "chrome/common/chrome_switches.h"
 #include "chrome/common/safe_browsing/csd.pb.h"
 #include "content/public/test/mock_download_item.h"
 #include "content/public/test/test_browser_thread.h"
@@ -146,6 +149,8 @@ ACTION_P(CheckDownloadUrlDone, threat_type) {
 class DownloadProtectionServiceTest : public testing::Test {
  protected:
   virtual void SetUp() {
+    CommandLine::ForCurrentProcess()->AppendSwitch(
+        switches::kSbEnableDownloadFeedback);
     ui_thread_.reset(new content::TestBrowserThread(BrowserThread::UI,
                                                     &msg_loop_));
     // Start real threads for the IO and File threads so that the DCHECKs
@@ -485,6 +490,10 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadSuccess) {
                  base::Unretained(this)));
   msg_loop_.Run();
   EXPECT_TRUE(IsResult(DownloadProtectionService::SAFE));
+  std::string feedback_ping;
+  std::string feedback_response;
+  EXPECT_FALSE(DownloadFeedbackService::GetPingsForDownloadForTesting(
+      item, &feedback_ping, &feedback_response));
 
   // If the response is dangerous the result should also be marked as dangerous.
   response.set_verdict(ClientDownloadResponse::DANGEROUS);
@@ -498,6 +507,8 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadSuccess) {
       base::Bind(&DownloadProtectionServiceTest::CheckDoneCallback,
                  base::Unretained(this)));
   msg_loop_.Run();
+  EXPECT_FALSE(DownloadFeedbackService::GetPingsForDownloadForTesting(
+      item, &feedback_ping, &feedback_response));
 #if defined(OS_WIN)
   EXPECT_TRUE(IsResult(DownloadProtectionService::DANGEROUS));
 #else
@@ -518,6 +529,12 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadSuccess) {
   msg_loop_.Run();
 #if defined(OS_WIN)
   EXPECT_TRUE(IsResult(DownloadProtectionService::UNCOMMON));
+  EXPECT_TRUE(DownloadFeedbackService::GetPingsForDownloadForTesting(
+      item, &feedback_ping, &feedback_response));
+  ClientDownloadRequest decoded_request;
+  EXPECT_TRUE(decoded_request.ParseFromString(feedback_ping));
+  EXPECT_EQ(url_chain.back().spec(), decoded_request.url());
+  EXPECT_EQ(response.SerializeAsString(), feedback_response);
 #else
   EXPECT_TRUE(IsResult(DownloadProtectionService::SAFE));
 #endif
@@ -537,6 +554,9 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadSuccess) {
   msg_loop_.Run();
 #if defined(OS_WIN)
   EXPECT_TRUE(IsResult(DownloadProtectionService::DANGEROUS_HOST));
+  EXPECT_TRUE(DownloadFeedbackService::GetPingsForDownloadForTesting(
+      item, &feedback_ping, &feedback_response));
+  EXPECT_EQ(response.SerializeAsString(), feedback_response);
 #else
   EXPECT_TRUE(IsResult(DownloadProtectionService::SAFE));
 #endif
diff --git a/chrome/browser/safe_browsing/two_phase_uploader.cc b/chrome/browser/safe_browsing/two_phase_uploader.cc
index c5558d1..6fb3dc4 100644
--- a/chrome/browser/safe_browsing/two_phase_uploader.cc
+++ b/chrome/browser/safe_browsing/two_phase_uploader.cc
@@ -153,7 +153,7 @@ void TwoPhaseUploaderImpl::OnURLFetchUploadProgress(
   DCHECK(CalledOnValidThread());
   DVLOG(3) << __FUNCTION__ << " " << source->GetURL().spec()
            << " " << current << "/" << total;
-  if (state_ == UPLOAD_FILE)
+  if (state_ == UPLOAD_FILE && !progress_callback_.is_null())
     progress_callback_.Run(current, total);
 }
 
diff --git a/chrome/browser/safe_browsing/two_phase_uploader.h b/chrome/browser/safe_browsing/two_phase_uploader.h
index 8aced0a..c10cf7b 100644
--- a/chrome/browser/safe_browsing/two_phase_uploader.h
+++ b/chrome/browser/safe_browsing/two_phase_uploader.h
@@ -54,7 +54,7 @@ class TwoPhaseUploader : public base::NonThreadSafe {
   // The first phase request will be sent to |base_url|, with |metadata|
   // included.
   // |progress_callback| will be called periodically as the second phase
-  // progresses.
+  // progresses, if it is non-null.
   // On success |finish_callback| will be called with state = STATE_SUCCESS and
   // the server response in response_data. On failure, state will specify
   // which step the failure occurred in, and net_error, response_code, and
diff --git a/chrome/browser/ui/views/download/download_item_view.cc b/chrome/browser/ui/views/download/download_item_view.cc
index 9b7f385..032d83c 100644
--- a/chrome/browser/ui/views/download/download_item_view.cc
+++ b/chrome/browser/ui/views/download/download_item_view.cc
@@ -21,6 +21,9 @@
 #include "chrome/browser/download/chrome_download_manager_delegate.h"
 #include "chrome/browser/download/download_item_model.h"
 #include "chrome/browser/download/download_util.h"
+#include "chrome/browser/safe_browsing/download_feedback_service.h"
+#include "chrome/browser/safe_browsing/download_protection_service.h"
+#include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/browser/themes/theme_properties.h"
 #include "chrome/browser/ui/views/download/download_shelf_context_menu_view.h"
 #include "chrome/browser/ui/views/download/download_shelf_view.h"
@@ -524,6 +527,8 @@ void DownloadItemView::ShowContextMenuForView(View* source,
 void DownloadItemView::ButtonPressed(
     views::Button* sender, const ui::Event& event) {
   if (sender == discard_button_) {
+    if (model_.ShouldAllowDownloadFeedback() && BeginDownloadFeedback())
+      return;
     UMA_HISTOGRAM_LONG_TIMES("clickjacking.discard_download",
                              base::Time::Now() - creation_time_);
     download()->Remove();
@@ -842,6 +847,22 @@ void DownloadItemView::OpenDownload() {
   UpdateAccessibleName();
 }
 
+bool DownloadItemView::BeginDownloadFeedback() {
+  SafeBrowsingService* sb_service = g_browser_process->safe_browsing_service();
+  if (!sb_service)
+    return false;
+  safe_browsing::DownloadProtectionService* download_protection_service =
+      sb_service->download_protection_service();
+  if (!download_protection_service)
+    return false;
+  UMA_HISTOGRAM_LONG_TIMES("clickjacking.report_and_discard_download",
+                           base::Time::Now() - creation_time_);
+  download_protection_service->feedback_service()->BeginFeedbackForDownload(
+      download());
+  // WARNING: we are deleted at this point.  Don't access 'this'.
+  return true;
+}
+
 void DownloadItemView::LoadIcon() {
   IconManager* im = g_browser_process->icon_manager();
   last_download_item_path_ = download()->GetTargetFilePath();
@@ -1049,8 +1070,15 @@ void DownloadItemView::ShowWarningDialog() {
     save_button_->SetStyle(views::Button::STYLE_NATIVE_TEXTBUTTON);
     AddChildView(save_button_);
   }
-  discard_button_ = new views::LabelButton(
-      this, l10n_util::GetStringUTF16(IDS_DISCARD_DOWNLOAD));
+  if (model_.ShouldAllowDownloadFeedback()) {
+    safe_browsing::DownloadFeedbackService::RecordFeedbackButtonShown(
+        download()->GetDangerType());
+    discard_button_ = new views::LabelButton(
+        this, l10n_util::GetStringUTF16(IDS_REPORT_AND_DISCARD_DOWNLOAD));
+  } else {
+    discard_button_ = new views::LabelButton(
+        this, l10n_util::GetStringUTF16(IDS_DISCARD_DOWNLOAD));
+  }
   discard_button_->SetStyle(views::Button::STYLE_NATIVE_TEXTBUTTON);
   AddChildView(discard_button_);
 
diff --git a/chrome/browser/ui/views/download/download_item_view.h b/chrome/browser/ui/views/download/download_item_view.h
index 47f2684..1c64923 100644
--- a/chrome/browser/ui/views/download/download_item_view.h
+++ b/chrome/browser/ui/views/download/download_item_view.h
@@ -146,6 +146,11 @@ class DownloadItemView : public views::ButtonListener,
 
   void OpenDownload();
 
+  // Submit the downloaded file to the safebrowsing download feedback service.
+  // If true is returned, the DownloadItem and |this| have been deleted.  If
+  // false is returned, nothing has changed.
+  bool BeginDownloadFeedback();
+
   void LoadIcon();
   void LoadIconIfItemPathChanged();
 
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index 668b682..e279a7b 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -1697,6 +1697,10 @@
         'browser/safe_browsing/client_side_detection_service.h',
         'browser/safe_browsing/database_manager.cc',
         'browser/safe_browsing/database_manager.h',
+        'browser/safe_browsing/download_feedback.cc',
+        'browser/safe_browsing/download_feedback.h',
+        'browser/safe_browsing/download_feedback_service.cc',
+        'browser/safe_browsing/download_feedback_service.h',
         'browser/safe_browsing/download_protection_service.cc',
         'browser/safe_browsing/download_protection_service.h',
         'browser/safe_browsing/malware_details.cc',
diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi
index 50d0632..e9383e3 100644
--- a/chrome/chrome_tests_unit.gypi
+++ b/chrome/chrome_tests_unit.gypi
@@ -1085,6 +1085,8 @@
         'browser/safe_browsing/chunk_range_unittest.cc',
         'browser/safe_browsing/client_side_detection_host_unittest.cc',
         'browser/safe_browsing/client_side_detection_service_unittest.cc',
+        'browser/safe_browsing/download_feedback_service_unittest.cc',
+        'browser/safe_browsing/download_feedback_unittest.cc',
         'browser/safe_browsing/download_protection_service_unittest.cc',
         'browser/safe_browsing/local_two_phase_testserver.cc',
         'browser/safe_browsing/malware_details_unittest.cc',
diff --git a/chrome/common/chrome_switches.cc b/chrome/common/chrome_switches.cc
index 33694c5..706c1e7 100644
--- a/chrome/common/chrome_switches.cc
+++ b/chrome/common/chrome_switches.cc
@@ -1248,6 +1248,13 @@ const char kSbDisableExtensionBlacklist[] =
 const char kSbDisableSideEffectFreeWhitelist[] =
     "safebrowsing-disable-side-effect-free-whitelist";
 
+// URL to send safebrowsing download feedback reports to.
+const char kSbDownloadFeedbackURL[] = "safebrowsing-download-feedback-url";
+
+// Enable safebrowsing download feedback.
+const char kSbEnableDownloadFeedback[] =
+    "safebrowsing-enable-download-feedback";
+
 // Enables or disables extension scripts badges in the location bar.
 const char kScriptBadges[]                  = "script-badges";
 
diff --git a/chrome/common/chrome_switches.h b/chrome/common/chrome_switches.h
index 84629d9..083b3d8 100644
--- a/chrome/common/chrome_switches.h
+++ b/chrome/common/chrome_switches.h
@@ -329,6 +329,8 @@ extern const char kSbDisableAutoUpdate[];
 extern const char kSbDisableDownloadProtection[];
 extern const char kSbDisableExtensionBlacklist[];
 extern const char kSbDisableSideEffectFreeWhitelist[];
+extern const char kSbDownloadFeedbackURL[];
+extern const char kSbEnableDownloadFeedback[];
 extern const char kScriptBadges[];
 extern const char kScriptBubble[];
 extern const char kServiceProcess[];
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml
index 186501c..b66fc5d 100644
--- a/tools/metrics/histograms/histograms.xml
+++ b/tools/metrics/histograms/histograms.xml
@@ -941,6 +941,13 @@ other types of suffix sets.
   <summary>TBD</summary>
 </histogram>
 
+<histogram name="clickjacking.report_and_discard_download" units="ms">
+  <summary>
+    Time between &quot;Report and Discard&quot; button being shown and it being
+    clicked.
+  </summary>
+</histogram>
+
 <histogram name="clickjacking.save_download">
   <summary>TBD</summary>
 </histogram>
@@ -10581,6 +10588,53 @@ other types of suffix sets.
   </summary>
 </histogram>
 
+<histogram name="SBDownloadFeedback.Activations" enum="DownloadItem.DangerType">
+  <summary>
+    Count of times download feedback has been started, broken down by danger
+    type.
+  </summary>
+</histogram>
+
+<histogram name="SBDownloadFeedback.ActiveFeedbacks">
+  <summary>
+    When a new download feedback request is added, records the number of
+    download requests currently active and/or pending.
+  </summary>
+</histogram>
+
+<histogram name="SBDownloadFeedback.Shown" enum="DownloadItem.DangerType">
+  <summary>
+    Count of times download feedback button has been shown, broken down by
+    danger type.
+  </summary>
+</histogram>
+
+<histogram name="SBDownloadFeedback.SizeEligibleKB" units="KB">
+  <summary>
+    Size of downloads that were of the correct danger type, regardless if they
+    meet the max file size check or if they are actually uploaded or not.
+  </summary>
+</histogram>
+
+<histogram name="SBDownloadFeedback.SizeFailure" units="bytes">
+  <summary>
+    Size of downloads that failed to be uploaded to the feedback service.
+  </summary>
+</histogram>
+
+<histogram name="SBDownloadFeedback.SizeSuccess" units="bytes">
+  <summary>
+    Size of downloads that were successfully uploaded to the feedback service.
+  </summary>
+</histogram>
+
+<histogram name="SBDownloadFeedback.UploadResult"
+    enum="SBDownloadFeedbackUploadResult">
+  <summary>
+    Final result of attempt to upload binary to download feedback service.
+  </summary>
+</histogram>
+
 <histogram name="Search.DefaultSearchProvider" enum="OmniboxSearchEngine">
   <summary>
     The id of the default search engine that is loaded after Chrome startup. See
@@ -17223,6 +17277,17 @@ other types of suffix sets.
   <int value="5" label="MODEL_MISSING_FIELDS"/>
 </enum>
 
+<enum name="SBDownloadFeedbackUploadResult" type="int">
+  <int value="0" label="SUCCESS"/>
+  <int value="1" label="UPLOAD_SUCCESS"/>
+  <int value="2" label="UPLOAD_CANCELLED"/>
+  <int value="3" label="UPLOAD_METADATA_NET_ERROR"/>
+  <int value="4" label="UPLOAD_METADATA_RESPONSE_ERROR"/>
+  <int value="5" label="UPLOAD_FILE_NET_ERROR"/>
+  <int value="6" label="UPLOAD_FILE_RESPONSE_ERROR"/>
+  <int value="7" label="UPLOAD_COMPLETE_RESPONSE_ERROR"/>
+</enum>
+
 <enum name="ShillTerminationActionResult" type="int">
   <summary>
     The termination action result types come from TerminationActionResult in