Added DownloadItem::Observer to JobController.

content/browser/background_fetch/background_fetch_job_controller.h

Index: content/browser/background_fetch/background_fetch_job_controller.h
diff --git a/content/browser/background_fetch/background_fetch_job_controller.h b/content/browser/background_fetch/background_fetch_job_controller.h
index 6a099c754885fc8f7ff5a6eb852a81a96d87a5f9..6a0b53a9edd28a6863f142d29892dbf0ba5a77b5 100644
--- a/content/browser/background_fetch/background_fetch_job_controller.h
+++ b/content/browser/background_fetch/background_fetch_job_controller.h
@@ -7,9 +7,12 @@
 
 #include <memory>
 #include <string>
+#include <unordered_map>
 
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
 #include "content/common/content_export.h"
+#include "content/public/browser/download_item.h"
 
 namespace content {
 
@@ -21,14 +24,14 @@ class StoragePartition;
 // The JobController will be responsible for coordinating communication with the
 // DownloadManager. It will get requests from the JobData and dispatch them to
 // the DownloadManager.
-// TODO(harkness): The JobController should also observe downloads.
-class CONTENT_EXPORT BackgroundFetchJobController {
+class CONTENT_EXPORT BackgroundFetchJobController
+    : public content::DownloadItem::Observer {

[Peter Beverloo, 2017/03/08 14:58:19]

nit: you are in `content::`

[harkness, 2017/03/10 13:33:53]

Done.

  public:
   BackgroundFetchJobController(const std::string& job_guid,
                                BrowserContext* browser_context,
                                StoragePartition* storage_partition,
                                BackgroundFetchJobData* job_data);
-  ~BackgroundFetchJobController();
+  ~BackgroundFetchJobController() override;
 
   // Start processing on a batch of requests. Some of these may already be in
   // progress or completed from a previous chromium instance.
@@ -38,10 +41,24 @@ class CONTENT_EXPORT BackgroundFetchJobController {
   void Shutdown();
 
  private:
+  // DownloadItem::Observer methods.
+  void OnDownloadUpdated(DownloadItem* item) override;
+  void OnDownloadDestroyed(DownloadItem* item) override;
+
+  // Callback passed to the DownloadManager which will be invoked once the
+  // download starts.
+  void DownloadStarted(const std::string& request_guid,
+                       DownloadItem* item,
+                       DownloadInterruptReason reason);
+
   void ProcessRequest(const BackgroundFetchRequestInfo& request);
 
+  void StopObservations();
+
   // Pointer to the browser context. The BackgroundFetchJobController is owned
   // by the BrowserContext via the StoragePartition.
+  // TODO(harkness): Currently this is only used to lookup the DownloadManager.
+  // Investigate whether the DownloadManager should be passed instead.
   BrowserContext* browser_context_;
 
   // Pointer to the storage partition. This object is owned by the partition
@@ -51,6 +68,11 @@ class CONTENT_EXPORT BackgroundFetchJobController {
   // The JobData which talks to the DataManager for this job_guid.
   std::unique_ptr<BackgroundFetchJobData> job_data_;
 
+  // Map from the GUID assigned by the DownloadManager to the request_guid.
+  std::unordered_map<std::string, std::string> download_guid_map_;

[Peter Beverloo, 2017/03/08 14:58:19]

Let's figure out a way to reduce the number of maps. There's a lot of them, all
are very prone to making unintentional copies, and we may be able to find better
data structures to achieve the same goal.

[harkness, 2017/03/10 13:33:54]

As discussed in person, we're going to switch to using linear time lookup
structures on the assumption that we expect there to be small numbers of them
and the overhead of the more complex structure would swamp any performance gain.

I'll investigate flat_set and converting the maps to that in the future.

+
+  base::WeakPtrFactory<BackgroundFetchJobController> weak_ptr_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(BackgroundFetchJobController);
 };