Added DownloadItem::Observer to JobController.

content/browser/background_fetch/background_fetch_job_controller.h

 // Copyright 2017 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 CONTENT_BROWSER_BACKGROUND_FETCH_BACKGROUND_FETCH_JOB_CONTROLLER_H_
 #define CONTENT_BROWSER_BACKGROUND_FETCH_BACKGROUND_FETCH_JOB_CONTROLLER_H_
 
 #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 {
 
 class BackgroundFetchJobData;
 class BackgroundFetchRequestInfo;
 class BrowserContext;
 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.
   void StartProcessing();
 
   // Called by the BackgroundFetchContext when the system is shutting down.
   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
   // (through a sequence of other classes).
   StoragePartition* storage_partition_;
 
   // 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);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_BACKGROUND_FETCH_BACKGROUND_FETCH_JOB_CONTROLLER_H_