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

chrome/browser/safe_browsing/incident_reporting/download_metadata_manager.h

Index: chrome/browser/safe_browsing/incident_reporting/download_metadata_manager.h
diff --git a/chrome/browser/safe_browsing/incident_reporting/download_metadata_manager.h b/chrome/browser/safe_browsing/incident_reporting/download_metadata_manager.h
new file mode 100644
index 0000000000000000000000000000000000000000..10efe5fc504a34da01a1ecf2c50baea508e2148f
--- /dev/null
+++ b/chrome/browser/safe_browsing/incident_reporting/download_metadata_manager.h
@@ -0,0 +1,98 @@
+// 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 given BrowserContext.

[robertshield, 2014/10/22 02:28:56]

s/given BrowserContext/|browser_context| passed to GetDownloadDetails/

[grt (UTC plus 2), 2014/10/23 19:53:13]

Done.

+  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.

[robertshield, 2014/10/22 02:28:56]

isn't it metatdata for the download's ManagerContext that is removed?

[grt (UTC plus 2), 2014/10/23 19:53:13]

ManagerContext is an implementation detail. The important result of passing in
null is that subsequent calls to GetDownloadDetails for |download|'s
BrowserContext (content::DownloadItem::GetBrowserContext()) will invoke the
callback with null.

[robertshield, 2014/10/23 20:30:27]

Acknowledged.

+  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* GetDownloadManagerForContext(

[robertshield, 2014/10/22 02:28:56]

please rename to GetDownloadManagerForBrowserContext. There are two types of
Context in the .cc file, ManagerContext and BrowserContext and I found reading
calls to this confusing.

[grt (UTC plus 2), 2014/10/23 19:53:13]

Done.

+      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_