Implement GetJobResponse and merge JobData into DataManager.

content/browser/background_fetch/background_fetch_data_manager.h

Index: content/browser/background_fetch/background_fetch_data_manager.h
diff --git a/content/browser/background_fetch/background_fetch_data_manager.h b/content/browser/background_fetch/background_fetch_data_manager.h
index 4dd3a86873f71fe89f8c0c514637444bd457981b..002847cb96c9a440571e004ecf93158248055fd3 100644
--- a/content/browser/background_fetch/background_fetch_data_manager.h
+++ b/content/browser/background_fetch/background_fetch_data_manager.h
@@ -10,15 +10,18 @@
 #include <string>
 #include <unordered_map>
 
+#include "base/callback_forward.h"
 #include "base/macros.h"
-#include "content/browser/background_fetch/background_fetch_job_data.h"
 #include "content/browser/background_fetch/background_fetch_job_info.h"
+#include "content/browser/background_fetch/background_fetch_job_response_data.h"
+#include "content/browser/background_fetch/background_fetch_request_info.h"
 #include "content/common/content_export.h"
 #include "url/origin.h"
 
 namespace content {
 
-class BackgroundFetchContext;
+class BrowserContext;
+class BlobHandle;
 
 // The BackgroundFetchDataManager keeps track of all of the outstanding requests
 // which are in process in the DownloadManager. When Chromium restarts, it is
@@ -26,37 +29,76 @@ class BackgroundFetchContext;
 // which will keep the metadata up to date.
 class CONTENT_EXPORT BackgroundFetchDataManager {
  public:
-  explicit BackgroundFetchDataManager(
-      BackgroundFetchContext* background_fetch_context);
+  explicit BackgroundFetchDataManager(BrowserContext* browser_context);
   ~BackgroundFetchDataManager();
 
   // Called by BackgroundFetchContext when a new request is started, this will
   // store all of the necessary metadata to track the request.
-  std::unique_ptr<BackgroundFetchJobData> CreateRequest(
-      std::unique_ptr<BackgroundFetchJobInfo> job_info,
-      BackgroundFetchRequestInfos request_infos);
+  void CreateRequest(std::unique_ptr<BackgroundFetchJobInfo> job_info,
+                     BackgroundFetchRequestInfos request_infos);
+
+  // TODO(harkness): Replace the OnceClosure with a callback to return the
+  // response object once it is decided whether lifetime should be passed to the
+  // caller or stay here.
+  void GetJobResponse(const std::string& job_guid,
+                      const BackgroundFetchResponseCompleteCallback& callback);
+
+  // The following methods are called by the JobController to update job state.
+  // Returns a boolean indicating whether there are more requests to process.
+  virtual bool UpdateRequestState(const std::string& job_guid,

[Peter Beverloo, 2017/03/25 03:38:50]

I'm not a huge fan of this approach. Ideally one would think of a class as an
API that defines a behaviour contract, with a (unit) test verifying that it
behaves according to that contract.

Looking at the JobController test, it would be much better if it would actually
create a new background fetch job, and then just operates on that data. It means
that the test would get some asynchronous aspects to it, but that'll have to
happen anyway.

[harkness, 2017/03/26 16:13:18]

Well, when I had the JobData, it had a very narrowly defined role, and it was
easy to create one for the JobController unittest to work with. Now that it's
the DataManager handling that role, it's a much more complicated object. This
particular code is the same, just moved from the JobData to the DataManager.

+                                  const std::string& request_guid,
+                                  DownloadItem::DownloadState state,
+                                  DownloadInterruptReason interrupt_reason);
+  virtual void UpdateRequestStorageState(const std::string& job_guid,
+                                         const std::string& request_guid,
+                                         const base::FilePath& file_path,
+                                         int64_t received_bytes);
+  virtual void UpdateRequestDownloadGuid(const std::string& job_guid,
+                                         const std::string& request_guid,
+                                         const std::string& download_guid);
+
+  // Called by the JobController to get a BackgroundFetchRequestInfo to
+  // process.
+  virtual const BackgroundFetchRequestInfo& GetNextBackgroundFetchRequestInfo(
+      const std::string& job_guid);
+
+  // Indicates whether all requests have been handed to the JobController.
+  virtual bool HasRequestsRemaining(const std::string& job_guid) const;
+
+  // Indicates whether all requests have been handed out and completed.
+  virtual bool IsComplete(const std::string& job_guid) const;
 
  private:
+  // Storage interface.
   void WriteJobToStorage(std::unique_ptr<BackgroundFetchJobInfo> job_info,
                          BackgroundFetchRequestInfos request_infos);
+  void WriteRequestToStorage(const std::string& job_guid,
+                             BackgroundFetchRequestInfo* request_info);
+  std::unique_ptr<BackgroundFetchRequestInfo> GetRequestInfo(
+      const std::string& job_guid,
+      size_t request_index) const;
 
-  BackgroundFetchRequestInfos& ReadRequestsFromStorage(
-      const std::string& job_guid);
+  void DidGetRequestResponse(const std::string& job_guid,
+                             int request_sequence_number,
+                             std::unique_ptr<BlobHandle> blob_handle);
 
-  // BackgroundFetchContext owns this BackgroundFetchDataManager, so the
-  // DataManager is guaranteed to be destructed before the Context.
-  BackgroundFetchContext* background_fetch_context_;
+  // Indirectly, this is owned by the BrowserContext.
+  BrowserContext* browser_context_;
 
   // Map from <sw_registration_id, tag> to the job_guid for that tag.
   using JobIdentifier = std::pair<int64_t, std::string>;
   std::map<JobIdentifier, std::string> service_worker_tag_map_;
 
-  // Temporary map to hold data which will be written to storage.
   // Map from job_guid to JobInfo.
   std::unordered_map<std::string, std::unique_ptr<BackgroundFetchJobInfo>>
       job_map_;
+
+  // Temporary map to hold data which will be written to storage.
   // Map from job_guid to RequestInfos.
-  std::unordered_map<std::string, BackgroundFetchRequestInfos> request_map_;
+  std::unordered_map<std::string, std::vector<BackgroundFetchRequestInfo>>
+      request_map_;
+
+  base::WeakPtrFactory<BackgroundFetchDataManager> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(BackgroundFetchDataManager);
 };