Include high-fidelity metadata about a download in incident reports.

This is accomplished by the introduction of the DownloadMetadataManager,
which maintains state about the most recent binary download.

BUG=389123, 386915, 389643

Review URL: https://codereview.chromium.org/663023007

Cr-Commit-Position: refs/heads/master@{#302624}

chrome/browser/download/chrome_download_manager_delegate.cc

@@ -185,6 +185,14 @@ ChromeDownloadManagerDelegate::~ChromeDownloadManagerDelegate() {
 
 void ChromeDownloadManagerDelegate::SetDownloadManager(DownloadManager* dm) {
   download_manager_ = dm;
+
+#if defined(FULL_SAFE_BROWSING) || defined(MOBILE_SAFE_BROWSING)
+  SafeBrowsingService* sb_service = g_browser_process->safe_browsing_service();
+  if (sb_service && !profile_->IsOffTheRecord()) {
+    // Include this download manager in the set monitored by safe browsing.
+    sb_service->AddDownloadManager(dm);
+  }
+#endif
 }
 
 void ChromeDownloadManagerDelegate::Shutdown() {

chrome/browser/safe_browsing/download_protection_service.cc

@@ -593,6 +593,9 @@ class DownloadProtectionService::CheckClientDownloadRequest
     if (url.is_valid() && database_manager_->MatchDownloadWhitelistUrl(url)) {
       VLOG(2) << url << " is on the download whitelist.";
       RecordCountOfSignedOrWhitelistedDownload();
+      // TODO(grt): Continue processing without uploading so that
+      // ClientDownloadRequest callbacks can be run even for this type of safe
+      // download.
       PostFinishTask(SAFE, REASON_WHITELISTED_URL);
       return;
     }
@@ -602,6 +605,9 @@ class DownloadProtectionService::CheckClientDownloadRequest
       for (int i = 0; i < signature_info_.certificate_chain_size(); ++i) {
         if (CertificateChainIsWhitelisted(
                 signature_info_.certificate_chain(i))) {
+          // TODO(grt): Continue processing without uploading so that
+          // ClientDownloadRequest callbacks can be run even for this type of
+          // safe download.
           PostFinishTask(SAFE, REASON_TRUSTED_EXECUTABLE);
           return;
         }
@@ -732,6 +738,8 @@ class DownloadProtectionService::CheckClientDownloadRequest
       return;
     }
 
+    service_->client_download_request_callbacks_.Notify(item_, &request);
+
     VLOG(2) << "Sending a request for URL: "
             << item_->GetUrlChain().back();
     fetcher_.reset(net::URLFetcher::Create(0 /* ID used for testing */,
@@ -782,6 +790,15 @@ class DownloadProtectionService::CheckClientDownloadRequest
                             base::TimeTicks::Now() - timeout_start_time_);
       }
     }
+    if (result == SAFE && (reason == REASON_WHITELISTED_URL ||
+                           reason == REASON_TRUSTED_EXECUTABLE)) {
+      // Due to the short-circuit logic in CheckWhitelists (see TODOs there), a
+      // ClientDownloadRequest was not generated for this download and callbacks
+      // were not run. Run them now with null to indicate that a download has
+      // taken place.
+      // TODO(grt): persist metadata for these downloads as well.
+      service_->client_download_request_callbacks_.Notify(item_, nullptr);
+    }
     if (service_) {
       VLOG(2) << "SafeBrowsing download verdict for: "
               << item_->DebugString(true) << " verdict:" << reason
@@ -961,6 +978,13 @@ bool DownloadProtectionService::IsSupportedDownload(
 #endif
 }
 
+DownloadProtectionService::ClientDownloadRequestSubscription
+DownloadProtectionService::RegisterClientDownloadRequestCallback(
+    const ClientDownloadRequestCallback& callback) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  return client_download_request_callbacks_.Add(callback);
+}
+
 void DownloadProtectionService::CancelPendingRequests() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   for (std::set<scoped_refptr<CheckClientDownloadRequest> >::iterator it =

chrome/browser/safe_browsing/download_protection_service.h

@@ -14,9 +14,11 @@
 
 #include "base/basictypes.h"
 #include "base/callback.h"
+#include "base/callback_list.h"
 #include "base/files/file_path.h"
 #include "base/gtest_prod_util.h"
 #include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
 #include "chrome/browser/safe_browsing/database_manager.h"
 #include "chrome/browser/safe_browsing/ui_manager.h"
 #include "url/gurl.h"
@@ -33,8 +35,9 @@ class X509Certificate;
 }  // namespace net
 
 namespace safe_browsing {
-class DownloadFeedbackService;
 class BinaryFeatureExtractor;
+class ClientDownloadRequest;
+class DownloadFeedbackService;
 
 // This class provides an asynchronous API to check whether a particular
 // client download is malicious or not.
@@ -52,6 +55,22 @@ class DownloadProtectionService {
   // Callback type which is invoked once the download request is done.
   typedef base::Callback<void(DownloadCheckResult)> CheckDownloadCallback;
 
+  // A type of callback run on the main thread when a ClientDownloadRequest has
+  // been formed for a download, or when one has not been formed for a supported
+  // download.
+  typedef base::Callback<void(content::DownloadItem*,
+                              const ClientDownloadRequest*)>
+      ClientDownloadRequestCallback;
+
+  // A list of ClientDownloadRequest callbacks.
+  typedef base::CallbackList<void(content::DownloadItem*,
+                                  const ClientDownloadRequest*)>
+      ClientDownloadRequestCallbackList;
+
+  // A subscription to a registered ClientDownloadRequest callback.
+  typedef scoped_ptr<ClientDownloadRequestCallbackList::Subscription>
+      ClientDownloadRequestSubscription;
+
   // Creates a download service.  The service is initially disabled.  You need
   // to call SetEnabled() to start it.  |sb_service| owns this object; we
   // keep a reference to |request_context_getter|.
@@ -107,6 +126,11 @@ class DownloadProtectionService {
     return feedback_service_.get();
   }
 
+  // Registers a callback that will be run when a ClientDownloadRequest has
+  // been formed.
+  ClientDownloadRequestSubscription RegisterClientDownloadRequestCallback(
+      const ClientDownloadRequestCallback& callback);
+
  protected:
   // Enum to keep track why a particular download verdict was chosen.
   // This is used to keep some stats around.
@@ -198,6 +222,10 @@ class DownloadProtectionService {
 
   scoped_ptr<DownloadFeedbackService> feedback_service_;
 
+  // A list of callbacks to be run on the main thread when a
+  // ClientDownloadRequest has been formed.
+  ClientDownloadRequestCallbackList client_download_request_callbacks_;
+
   DISALLOW_COPY_AND_ASSIGN(DownloadProtectionService);
 };
 }  // namespace safe_browsing

chrome/browser/safe_browsing/incident_reporting/download_metadata_manager.h

+// Copyright 2014 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_INCIDENT_REPORTING_DOWNLOAD_METADATA_MANAGER_H_
+#define CHROME_BROWSER_SAFE_BROWSING_INCIDENT_REPORTING_DOWNLOAD_METADATA_MANAGER_H_
+
+#include <map>
+
+#include "base/callback_forward.h"
+#include "base/macros.h"
+#include "base/memory/ref_counted.h"
+#include "content/public/browser/download_manager.h"
+
+namespace base {
+class SequencedTaskRunner;
+class SequencedWorkerPool;
+}
+
+namespace content {
+class BrowserContext;
+class DownloadItem;
+}
+
+namespace safe_browsing {
+
+class ClientDownloadRequest;
+class ClientIncidentReport_DownloadDetails;
+class DownloadMetadata;
+
+// A browser-wide object that manages the persistent state of metadata
+// pertaining to a download.
+class DownloadMetadataManager : public content::DownloadManager::Observer {
+ public:
+  // A callback run when the results of a call to GetDownloadDetails are ready.
+  // The supplied parameter may be null, indicating that there are no persisted
+  // details for the |browser_context| passed to GetDownloadDetails.
+  typedef base::Callback<void(scoped_ptr<ClientIncidentReport_DownloadDetails>)>
+      GetDownloadDetailsCallback;
+
+  // Constructs a new instance for which disk IO operations will take place in
+  // |worker_pool|.
+  explicit DownloadMetadataManager(
+      const scoped_refptr<base::SequencedWorkerPool>& worker_pool);
+
+  // Constructor that allows tests to provide a specific runner for
+  // asynchronous tasks.
+  explicit DownloadMetadataManager(
+      const scoped_refptr<base::SequencedTaskRunner>& task_runner);
+  virtual ~DownloadMetadataManager();
+
+  // Adds |download_manager| to the set observed by the metadata manager.
+  void AddDownloadManager(content::DownloadManager* download_manager);
+
+  // Sets |request| as the relevant metadata to persist for |download|. If
+  // |request| is null, metadata for the download's BrowserContext are cleared.
+  virtual void SetRequest(content::DownloadItem* download,
+                          const ClientDownloadRequest* request);
+
+  // Gets the persisted DownloadDetails for |browser_context|. |callback| will
+  // be run immediately if the data is available. Otherwise, it will be run
+  // later on the caller's thread.
+  virtual void GetDownloadDetails(content::BrowserContext* browser_context,
+                                  const GetDownloadDetailsCallback& callback);
+
+ protected:
+  // Returns the DownloadManager for a given BrowserContext. Virtual for tests.
+  virtual content::DownloadManager* GetDownloadManagerForBrowserContext(
+      content::BrowserContext* context);
+
+  // content::DownloadManager:Observer methods.
+  void OnDownloadCreated(content::DownloadManager* download_manager,
+                         content::DownloadItem* item) override;
+  void ManagerGoingDown(content::DownloadManager* download_manager) override;
+
+ private:
+  class ManagerContext;
+
+  // A mapping of DownloadManagers to their corresponding contexts.
+  typedef std::map<content::DownloadManager*, ManagerContext*>
+      ManagerToContextMap;
+
+  // A task runner to which read tasks are posted.
+  scoped_refptr<base::SequencedTaskRunner> read_runner_;
+
+  // A task runner to which write tasks are posted.
+  scoped_refptr<base::SequencedTaskRunner> write_runner_;
+
+  // Contexts for each DownloadManager that has been added and has not yet
+  // "gone down".
+  ManagerToContextMap contexts_;
+
+  DISALLOW_COPY_AND_ASSIGN(DownloadMetadataManager);
+};
+
+}  // namespace safe_browsing
+
+#endif  // CHROME_BROWSER_SAFE_BROWSING_INCIDENT_REPORTING_DOWNLOAD_METADATA_MANAGER_H_

chrome/browser/safe_browsing/incident_reporting/incident_reporting_service.cc

@@ -249,8 +249,9 @@ IncidentReportingService::UploadContext::~UploadContext() {
 IncidentReportingService::IncidentReportingService(
     SafeBrowsingService* safe_browsing_service,
     const scoped_refptr<net::URLRequestContextGetter>& request_context_getter)
-    : database_manager_(safe_browsing_service ?
-                        safe_browsing_service->database_manager() : NULL),
+    : database_manager_(safe_browsing_service
+                            ? safe_browsing_service->database_manager()
+                            : NULL),
       url_request_context_getter_(request_context_getter),
       collect_environment_data_fn_(&CollectEnvironmentData),
       environment_collection_task_runner_(
@@ -268,6 +269,7 @@ IncidentReportingService::IncidentReportingService(
           content::BrowserThread::GetBlockingPool()
               ->GetTaskRunnerWithShutdownBehavior(
                   base::SequencedWorkerPool::SKIP_ON_SHUTDOWN)),
+      download_metadata_manager_(content::BrowserThread::GetBlockingPool()),
       receiver_weak_ptr_factory_(this),
       weak_ptr_factory_(this) {
   notification_registrar_.Add(this,
@@ -276,6 +278,16 @@ IncidentReportingService::IncidentReportingService(
   notification_registrar_.Add(this,
                               chrome::NOTIFICATION_PROFILE_DESTROYED,
                               content::NotificationService::AllSources());
+  DownloadProtectionService* download_protection_service =
+      (safe_browsing_service ?
+       safe_browsing_service->download_protection_service() :
+       NULL);
+  if (download_protection_service) {
+    client_download_request_subscription_ =
+        download_protection_service->RegisterClientDownloadRequestCallback(
+            base::Bind(&IncidentReportingService::OnClientDownloadRequest,
+                       base::Unretained(this)));
+  }
 }
 
 IncidentReportingService::~IncidentReportingService() {
@@ -333,13 +345,19 @@ void IncidentReportingService::RegisterDelayedAnalysisCallback(
     delayed_analysis_callbacks_.Start();
 }
 
+void IncidentReportingService::AddDownloadManager(
+    content::DownloadManager* download_manager) {
+  download_metadata_manager_.AddDownloadManager(download_manager);
+}
+
 IncidentReportingService::IncidentReportingService(
     SafeBrowsingService* safe_browsing_service,
     const scoped_refptr<net::URLRequestContextGetter>& request_context_getter,
     base::TimeDelta delayed_task_interval,
     const scoped_refptr<base::TaskRunner>& delayed_task_runner)
-    : database_manager_(safe_browsing_service ?
-                        safe_browsing_service->database_manager() : NULL),
+    : database_manager_(safe_browsing_service
+                            ? safe_browsing_service->database_manager()
+                            : NULL),
       url_request_context_getter_(request_context_getter),
       collect_environment_data_fn_(&CollectEnvironmentData),
       environment_collection_task_runner_(
@@ -353,6 +371,7 @@ IncidentReportingService::IncidentReportingService(
                        this,
                        &IncidentReportingService::OnCollationTimeout),
       delayed_analysis_callbacks_(delayed_task_interval, delayed_task_runner),
+      download_metadata_manager_(content::BrowserThread::GetBlockingPool()),
       receiver_weak_ptr_factory_(this),
       weak_ptr_factory_(this) {
   notification_registrar_.Add(this,
@@ -427,7 +446,10 @@ void IncidentReportingService::OnProfileAdded(Profile* profile) {
 
 scoped_ptr<LastDownloadFinder> IncidentReportingService::CreateDownloadFinder(
     const LastDownloadFinder::LastDownloadCallback& callback) {
-  return LastDownloadFinder::Create(callback).Pass();
+  return LastDownloadFinder::Create(
+             base::Bind(&DownloadMetadataManager::GetDownloadDetails,
+                        base::Unretained(&download_metadata_manager_)),
+             callback).Pass();
 }
 
 scoped_ptr<IncidentReportUploader> IncidentReportingService::StartReportUpload(
@@ -945,6 +967,13 @@ void IncidentReportingService::OnReportUploadResult(
   // else retry?
 }
 
+void IncidentReportingService::OnClientDownloadRequest(
+    content::DownloadItem* download,
+    const ClientDownloadRequest* request) {
+  if (!download->GetBrowserContext()->IsOffTheRecord())
+    download_metadata_manager_.SetRequest(download, request);
+}
+
 void IncidentReportingService::Observe(
     int type,
     const content::NotificationSource& source,

chrome/browser/safe_browsing/incident_reporting/incident_reporting_service.h

@@ -18,9 +18,11 @@
 #include "base/threading/thread_checker.h"
 #include "base/time/time.h"
 #include "base/timer/timer.h"
+#include "chrome/browser/safe_browsing/download_protection_service.h"
 #include "chrome/browser/safe_browsing/incident_reporting/add_incident_callback.h"
 #include "chrome/browser/safe_browsing/incident_reporting/delayed_analysis_callback.h"
 #include "chrome/browser/safe_browsing/incident_reporting/delayed_callback_runner.h"
+#include "chrome/browser/safe_browsing/incident_reporting/download_metadata_manager.h"
 #include "chrome/browser/safe_browsing/incident_reporting/incident_report_uploader.h"
 #include "chrome/browser/safe_browsing/incident_reporting/last_download_finder.h"
 #include "content/public/browser/notification_observer.h"
@@ -36,6 +38,7 @@ class TaskRunner;
 }
 
 namespace content {
+class DownloadManager;
 class NotificationDetails;
 class NotificationSource;
 }
@@ -46,6 +49,7 @@ class URLRequestContextGetter;
 
 namespace safe_browsing {
 
+class ClientDownloadRequest;
 class ClientIncidentReport;
 class ClientIncidentReport_DownloadDetails;
 class ClientIncidentReport_EnvironmentData;
@@ -90,6 +94,10 @@ class IncidentReportingService : public content::NotificationObserver {
   // Registers |callback| to be run after some delay following process launch.
   void RegisterDelayedAnalysisCallback(const DelayedAnalysisCallback& callback);
 
+  // Adds |download_manager| to the set monitored for client download request
+  // storage.
+  void AddDownloadManager(content::DownloadManager* download_manager);
+
  protected:
   // A pointer to a function that populates a protobuf with environment data.
   typedef void (*CollectEnvironmentDataFn)(
@@ -232,6 +240,10 @@ class IncidentReportingService : public content::NotificationObserver {
                             IncidentReportUploader::Result result,
                             scoped_ptr<ClientIncidentResponse> response);
 
+  // DownloadProtectionService::ClientDownloadRequestCallback implementation.
+  void OnClientDownloadRequest(content::DownloadItem* download,
+                               const ClientDownloadRequest* request);
+
   // content::NotificationObserver methods.
   void Observe(int type,
                const content::NotificationSource& source,
@@ -260,6 +272,11 @@ class IncidentReportingService : public content::NotificationObserver {
   // Registrar for observing profile lifecycle notifications.
   content::NotificationRegistrar notification_registrar_;
 
+  // A subscription for ClientDownloadRequests, used to persist them for later
+  // use.
+  DownloadProtectionService::ClientDownloadRequestSubscription
+      client_download_request_subscription_;
+
   // True when the asynchronous environment collection task has been fired off
   // but has not yet completed.
   bool environment_collection_pending_;
@@ -296,6 +313,8 @@ class IncidentReportingService : public content::NotificationObserver {
   // Callbacks registered for performing delayed analysis.
   DelayedCallbackRunner delayed_analysis_callbacks_;
 
+  DownloadMetadataManager download_metadata_manager_;
+
   // The collection of uploads in progress.
   ScopedVector<UploadContext> uploads_;
 

chrome/browser/safe_browsing/incident_reporting/last_download_finder.h

@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_SAFE_BROWSING_INCIDENT_REPORTING_LAST_DOWNLOAD_FINDER_H_
 #define CHROME_BROWSER_SAFE_BROWSING_INCIDENT_REPORTING_LAST_DOWNLOAD_FINDER_H_
 
+#include <map>
 #include <vector>
 
 #include "base/callback.h"
@@ -13,6 +14,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "chrome/browser/history/download_row.h"
+#include "chrome/browser/safe_browsing/incident_reporting/download_metadata_manager.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 
@@ -36,6 +38,11 @@ class ClientIncidentReport_DownloadDetails;
 // history that participates in safe browsing.
 class LastDownloadFinder : public content::NotificationObserver {
  public:
+  typedef base::Callback<void(
+      content::BrowserContext* context,
+      const DownloadMetadataManager::GetDownloadDetailsCallback&)>
+      DownloadDetailsGetter;
+
   // The type of a callback run by the finder upon completion. The argument is a
   // protobuf containing details of the download that was found, or an empty
   // pointer if none was found.
@@ -50,6 +57,7 @@ class LastDownloadFinder : public content::NotificationObserver {
   // Returns NULL without running |callback| if there are no eligible profiles
   // to search.
   static scoped_ptr<LastDownloadFinder> Create(
+      const DownloadDetailsGetter& download_details_getter,
       const LastDownloadCallback& callback);
 
  protected:
@@ -57,14 +65,28 @@ class LastDownloadFinder : public content::NotificationObserver {
   LastDownloadFinder();
 
  private:
-  LastDownloadFinder(const std::vector<Profile*>& profiles,
+  enum ProfileWaitState {
+    WAITING_FOR_METADATA,
+    WAITING_FOR_HISTORY,
+  };
+
+  LastDownloadFinder(const DownloadDetailsGetter& download_details_getter,
+                     const std::vector<Profile*>& profiles,
                      const LastDownloadCallback& callback);
 
   // Adds |profile| to the set of profiles to be searched if it is an
-  // on-the-record profile with history that participates in safe browsing. The
-  // search is initiated if the profile has already loaded history.
+  // on-the-record profile with history that participates in safe browsing. A
+  // search for metadata is initiated immediately.
   void SearchInProfile(Profile* profile);
 
+  // DownloadMetadataManager::GetDownloadDetailsCallback. If |details| are
+  // provided, retrieves them if they are the most relevant results. Otherwise
+  // begins a search in history. Reports results if there are no more pending
+  // queries.
+  void OnMetadataQuery(
+      Profile* profile,
+      scoped_ptr<ClientIncidentReport_DownloadDetails> details);
+
   // Initiates a search in |profile| if it is in the set of profiles to be
   // searched.
   void OnProfileHistoryLoaded(Profile* profile,
@@ -81,9 +103,10 @@ class LastDownloadFinder : public content::NotificationObserver {
       Profile* profile,
       scoped_ptr<std::vector<history::DownloadRow> > downloads);
 
-  // Removes the profile pointed to by |it| from profiles_ and reports results
-  // if there are no more pending queries.
-  void RemoveProfileAndReportIfDone(std::vector<Profile*>::iterator it);
+  // Removes the profile pointed to by |it| from profile_states_ and reports
+  // results if there are no more pending queries.
+  void RemoveProfileAndReportIfDone(
+      std::map<Profile*, ProfileWaitState>::iterator iter);
 
   // Invokes the caller-supplied callback with the download found.
   void ReportResults();
@@ -93,16 +116,24 @@ class LastDownloadFinder : public content::NotificationObserver {
                const content::NotificationSource& source,
                const content::NotificationDetails& details) override;
 
+  // Caller-supplied callback to make an asynchronous request for a profile's
+  // persistent download details.
+  DownloadDetailsGetter download_details_getter_;
+
   // Caller-supplied callback to be invoked when the most recent download is
   // found.
   LastDownloadCallback callback_;
 
-  // The profiles for which a download query is pending.
-  std::vector<Profile*> profiles_;
+  // A mapping of profiles for which a download query is pending to their
+  // respective states.
+  std::map<Profile*, ProfileWaitState> profile_states_;
 
   // Registrar for observing profile lifecycle notifications.
   content::NotificationRegistrar notification_registrar_;
 
+  // The most interesting download details retrieved from download metadata.
+  scoped_ptr<ClientIncidentReport_DownloadDetails> details_;
+
   // The most recent download, updated progressively as query results arrive.
   history::DownloadRow most_recent_row_;
 

chrome/browser/safe_browsing/incident_reporting/last_download_finder_unittest.cc

@@ -61,10 +61,11 @@ KeyedService* BuildHistoryService(content::BrowserContext* context) {
 
 }  // namespace
 
+namespace safe_browsing {
+
 class LastDownloadFinderTest : public testing::Test {
  public:
-  void NeverCalled(scoped_ptr<
-      safe_browsing::ClientIncidentReport_DownloadDetails> download) {
+  void NeverCalled(scoped_ptr<ClientIncidentReport_DownloadDetails> download) {
     FAIL();
   }
 
@@ -80,13 +81,12 @@ class LastDownloadFinderTest : public testing::Test {
                    base::Unretained(this)));
   }
 
-  // safe_browsing::LastDownloadFinder::LastDownloadCallback implementation that
+  // LastDownloadFinder::LastDownloadCallback implementation that
   // passes the found download to |result| and then runs a closure.
   void OnLastDownload(
-      scoped_ptr<safe_browsing::ClientIncidentReport_DownloadDetails>* result,
+      scoped_ptr<ClientIncidentReport_DownloadDetails>* result,
       const base::Closure& quit_closure,
-      scoped_ptr<safe_browsing::ClientIncidentReport_DownloadDetails>
-          download) {
+      scoped_ptr<ClientIncidentReport_DownloadDetails> download) {
     *result = download.Pass();
     quit_closure.Run();
   }
@@ -153,6 +153,11 @@ class LastDownloadFinderTest : public testing::Test {
     return profile;
   }
 
+  LastDownloadFinder::DownloadDetailsGetter GetDownloadDetailsGetter() {
+    return base::Bind(&LastDownloadFinderTest::GetDownloadDetails,
+                      base::Unretained(this));
+  }
+
   void AddDownload(Profile* profile, const history::DownloadRow& download) {
     base::RunLoop run_loop;
 
@@ -186,15 +191,13 @@ class LastDownloadFinderTest : public testing::Test {
 
   // Runs the last download finder on all loaded profiles, returning the found
   // download or an empty pointer if none was found.
-  scoped_ptr<safe_browsing::ClientIncidentReport_DownloadDetails>
-  RunLastDownloadFinder() {
+  scoped_ptr<ClientIncidentReport_DownloadDetails> RunLastDownloadFinder() {
     base::RunLoop run_loop;
 
-    scoped_ptr<safe_browsing::ClientIncidentReport_DownloadDetails>
-        last_download;
+    scoped_ptr<ClientIncidentReport_DownloadDetails> last_download;
 
-    scoped_ptr<safe_browsing::LastDownloadFinder> finder(
-        safe_browsing::LastDownloadFinder::Create(
+    scoped_ptr<LastDownloadFinder> finder(LastDownloadFinder::Create(
+        GetDownloadDetailsGetter(),
         base::Bind(&LastDownloadFinderTest::OnLastDownload,
                    base::Unretained(this),
                    &last_download,
@@ -230,13 +233,13 @@ class LastDownloadFinderTest : public testing::Test {
         std::string());                               // ext_name
   }
 
-  void ExpectNoDownloadFound(scoped_ptr<
-      safe_browsing::ClientIncidentReport_DownloadDetails> download) {
+  void ExpectNoDownloadFound(
+      scoped_ptr<ClientIncidentReport_DownloadDetails> download) {
     EXPECT_FALSE(download);
   }
 
-  void ExpectFoundTestDownload(scoped_ptr<
-      safe_browsing::ClientIncidentReport_DownloadDetails> download) {
+  void ExpectFoundTestDownload(
+      scoped_ptr<ClientIncidentReport_DownloadDetails> download) {
     ASSERT_TRUE(download);
   }
 
@@ -255,6 +258,12 @@ class LastDownloadFinderTest : public testing::Test {
   // created.
   void OnDownloadCreated(bool created) { ASSERT_TRUE(created); }
 
+  void GetDownloadDetails(
+      content::BrowserContext* context,
+      const DownloadMetadataManager::GetDownloadDetailsCallback& callback) {
+    callback.Run(scoped_ptr<ClientIncidentReport_DownloadDetails>());
+  }
+
   int profile_number_;
 };
 
@@ -295,9 +304,9 @@ TEST_F(LastDownloadFinderTest, DeleteBeforeResults) {
   AddDownload(profile, CreateTestDownloadRow());
 
   // Start a finder and kill it before the search completes.
-  safe_browsing::LastDownloadFinder::Create(
-      base::Bind(&LastDownloadFinderTest::NeverCalled, base::Unretained(this)))
-      .reset();
+  LastDownloadFinder::Create(GetDownloadDetailsGetter(),
+                             base::Bind(&LastDownloadFinderTest::NeverCalled,
+                                        base::Unretained(this))).reset();
 
   // Flush tasks on the history backend thread.
   FlushHistoryBackend(profile);
@@ -308,7 +317,7 @@ TEST_F(LastDownloadFinderTest, AddProfileAfterStarting) {
   // Create a profile with a history service that is opted-in.
   CreateProfile(SAFE_BROWSING_OPT_IN);
 
-  scoped_ptr<safe_browsing::ClientIncidentReport_DownloadDetails> last_download;
+  scoped_ptr<ClientIncidentReport_DownloadDetails> last_download;
   base::RunLoop run_loop;
 
   // Post a task that will create a second profile once the main loop is run.
@@ -318,8 +327,8 @@ TEST_F(LastDownloadFinderTest, AddProfileAfterStarting) {
                  base::Unretained(this)));
 
   // Create a finder that we expect will find a download in the second profile.
-  scoped_ptr<safe_browsing::LastDownloadFinder> finder(
-      safe_browsing::LastDownloadFinder::Create(
+  scoped_ptr<LastDownloadFinder> finder(LastDownloadFinder::Create(
+      GetDownloadDetailsGetter(),
       base::Bind(&LastDownloadFinderTest::OnLastDownload,
                  base::Unretained(this),
                  &last_download,
@@ -329,3 +338,5 @@ TEST_F(LastDownloadFinderTest, AddProfileAfterStarting) {
 
   ExpectFoundTestDownload(last_download.Pass());
 }
+
+}  // namespace safe_browsing

chrome/browser/safe_browsing/safe_browsing_service.cc

@@ -280,8 +280,8 @@ void SafeBrowsingService::ShutDown() {
   // dtor executes now since it may call the dtor of URLFetcher which relies
   // on it.
   csd_service_.reset();
-  download_service_.reset();
   incident_service_.reset();
+  download_service_.reset();
 
   url_request_context_getter_ = NULL;
   BrowserThread::PostNonNestableTask(
@@ -347,6 +347,14 @@ void SafeBrowsingService::RegisterDelayedAnalysisCallback(
 #endif
 }
 
+void SafeBrowsingService::AddDownloadManager(
+    content::DownloadManager* download_manager) {
+#if defined(FULL_SAFE_BROWSING)
+  if (incident_service_)
+    incident_service_->AddDownloadManager(download_manager);
+#endif
+}
+
 SafeBrowsingUIManager* SafeBrowsingService::CreateUIManager() {
   return new SafeBrowsingUIManager(this);
 }

chrome/browser/safe_browsing/safe_browsing_service.h

@@ -39,6 +39,10 @@ namespace base {
 class Thread;
 }
 
+namespace content {
+class DownloadManager;
+}
+
 namespace net {
 class URLRequestContext;
 class URLRequestContextGetter;
@@ -131,6 +135,9 @@ class SafeBrowsingService
   void RegisterDelayedAnalysisCallback(
       const safe_browsing::DelayedAnalysisCallback& callback);
 
+  // Adds |download_manager| to the set monitored by safe browsing.
+  void AddDownloadManager(content::DownloadManager* download_manager);
+
  protected:
   // Creates the safe browsing service.  Need to initialize before using.
   SafeBrowsingService();

chrome/chrome_browser.gypi

@@ -2620,6 +2620,8 @@
       'browser/safe_browsing/incident_reporting/delayed_analysis_callback.h',
       'browser/safe_browsing/incident_reporting/delayed_callback_runner.cc',
       'browser/safe_browsing/incident_reporting/delayed_callback_runner.h',
+      'browser/safe_browsing/incident_reporting/download_metadata_manager.cc',
+      'browser/safe_browsing/incident_reporting/download_metadata_manager.h',
       'browser/safe_browsing/incident_reporting/environment_data_collection.cc',
       'browser/safe_browsing/incident_reporting/environment_data_collection.h',
       'browser/safe_browsing/incident_reporting/environment_data_collection_win.cc',

chrome/chrome_tests_unit.gypi

@@ -670,6 +670,7 @@
       'browser/safe_browsing/incident_reporting/blacklist_load_analyzer_win_unittest.cc',
       'browser/safe_browsing/incident_reporting/blacklist_load_incident_handlers_unittest.cc',
       'browser/safe_browsing/incident_reporting/delayed_callback_runner_unittest.cc',
+      'browser/safe_browsing/incident_reporting/download_metadata_manager_unittest.cc',
       'browser/safe_browsing/incident_reporting/environment_data_collection_win_unittest.cc',
       'browser/safe_browsing/incident_reporting/incident_report_uploader_impl_unittest.cc',
       'browser/safe_browsing/incident_reporting/incident_reporting_service_unittest.cc',

chrome/common/safe_browsing/csd.proto

@@ -451,3 +451,9 @@ message ClientIncidentResponse {
 
   repeated EnvironmentRequest environment_requests = 3;
 }
+
+message DownloadMetadata {
+  optional uint32 download_id = 1;
+
+  optional ClientIncidentReport.DownloadDetails download = 2;
+}

tools/metrics/histograms/histograms.xml

@@ -30185,6 +30185,21 @@ Therefore, the affected-histogram name has to have at least one dot in it.
   </summary>
 </histogram>
 
+<histogram name="SBIRS.DownloadMetadata.DeleteSuccess" enum="BooleanSuccess">
+  <owner>grt@chromium.org</owner>
+  <summary>The result of deleting a profile's download metadata file.</summary>
+</histogram>
+
+<histogram name="SBIRS.DownloadMetadata.ReadResult" enum="MetadataReadResult">
+  <owner>grt@chromium.org</owner>
+  <summary>The result of reading a profile's download metadata file.</summary>
+</histogram>
+
+<histogram name="SBIRS.DownloadMetadata.WriteResult" enum="MetadataWriteResult">
+  <owner>grt@chromium.org</owner>
+  <summary>The result of writing a profile's download metadata file.</summary>
+</histogram>
+
 <histogram name="SBIRS.DroppedIncident" enum="IncidentType">
   <owner>grt@google.com</owner>
   <summary>
@@ -48679,6 +48694,23 @@ To add a new entry, add it with any value and run test to compute valid value.
   <int value="1" label="Http Live Stream Type"/>
 </enum>
 
+<enum name="MetadataReadResult" type="int">
+  <int value="0" label="Success"/>
+  <int value="1" label="Open failure"/>
+  <int value="2" label="Not found"/>
+  <int value="3" label="Get info failure"/>
+  <int value="4" label="File too big"/>
+  <int value="5" label="Read failure"/>
+  <int value="6" label="Parse failure"/>
+  <int value="7" label="Malformed data"/>
+</enum>
+
+<enum name="MetadataWriteResult" type="int">
+  <int value="0" label="Success"/>
+  <int value="1" label="Serialization failure"/>
+  <int value="2" label="Write failure"/>
+</enum>
+
 <enum name="MetaTagTypeEnum" type="int">
   <int value="0" label="No viewport tag"/>
   <int value="1" label="Viewport meta with device width"/>