[Background Fetch] Remove non-persistent code in DataManager.

The switch will be deleted in a follow-up CL.

Bug: 826257
Change-Id: I288e4bc5050714357c10b2d04e95586e23df44f4
Reviewed-on: https://chromium-review.googlesource.com/1089057
Commit-Queue: Rayan Kanso <rayankans@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565676}

content/browser/background_fetch/background_fetch_data_manager.h

@@ -165,8 +165,6 @@ class CONTENT_EXPORT BackgroundFetchDataManager
   friend class BackgroundFetchTestDataManager;
   friend class background_fetch::DatabaseTask;
 
-  class RegistrationData;
-
   void AddStartNextPendingRequestTask(
       int64_t service_worker_registration_id,
       NextRequestCallback callback,
@@ -180,9 +178,6 @@ class CONTENT_EXPORT BackgroundFetchDataManager
   // Virtual for testing.
   virtual CacheStorageManager* GetCacheStorageManager();
 
-  // Returns true if not aborted/completed/failed.
-  bool IsActive(const BackgroundFetchRegistrationId& registration_id);
-
   void Cleanup();
 
   scoped_refptr<ServiceWorkerContextWrapper> service_worker_context_;
@@ -192,17 +187,6 @@ class CONTENT_EXPORT BackgroundFetchDataManager
   // The blob storage request with which response information will be stored.
   scoped_refptr<ChromeBlobStorageContext> blob_storage_context_;
 
-  // Map from {service_worker_registration_id, origin, developer_id} tuples to
-  // the |unique_id|s of active background fetch registrations (not
-  // completed/failed/aborted, so there will never be more than one entry for a
-  // given key).
-  std::map<std::tuple<int64_t, url::Origin, std::string>, std::string>
-      active_registration_unique_ids_;
-
-  // Map from the |unique_id|s of known (but possibly inactive) background fetch
-  // registrations to their associated data.
-  std::map<std::string, std::unique_ptr<RegistrationData>> registrations_;
-
   // Pending database operations, serialized to ensure consistency.
   // Invariant: the frontmost task, if any, has already been started.
   base::queue<std::unique_ptr<background_fetch::DatabaseTask>> database_tasks_;

content/browser/background_fetch/background_fetch_scheduler.cc

@@ -48,8 +48,8 @@ void BackgroundFetchScheduler::AddJobController(
     BackgroundFetchScheduler::Controller* controller) {
   controller_queue_.push_back(controller);
 
-  while (!controller_queue_.empty() &&
-         download_controller_map_.size() < max_concurrent_downloads_) {
+  if (!controller_queue_.empty() &&
+      download_controller_map_.size() < max_concurrent_downloads_) {
     ScheduleDownload();
   }
 }
@@ -57,12 +57,14 @@ void BackgroundFetchScheduler::AddJobController(
 void BackgroundFetchScheduler::ScheduleDownload() {
   DCHECK(download_controller_map_.size() < max_concurrent_downloads_);
 
-  if (controller_queue_.empty())
+  if (lock_scheduler_ || controller_queue_.empty())
     return;
 
   auto* controller = controller_queue_.front();
   controller_queue_.pop_front();
 
+  // Making an async call, `ScheduleDownload` shouldn't be called anymore.
+  lock_scheduler_ = true;
   request_provider_->PopNextRequest(
       controller->registration_id(),
       base::BindOnce(&BackgroundFetchScheduler::DidPopNextRequest,
@@ -73,10 +75,19 @@ void BackgroundFetchScheduler::DidPopNextRequest(
     BackgroundFetchScheduler::Controller* controller,
     scoped_refptr<BackgroundFetchRequestInfo> request_info) {
   DCHECK(controller);
-  if (!request_info)
-    return;  // Storage error, fetch might have been aborted.
+  lock_scheduler_ = false;  // Can schedule downloads again.
+
+  // Storage error, fetch might have been aborted.
+  if (!request_info) {
+    ScheduleDownload();
+    return;
+  }
+
   download_controller_map_[request_info->download_guid()] = controller;
   controller->StartRequest(request_info);
+
+  if (download_controller_map_.size() < max_concurrent_downloads_)
+    ScheduleDownload();
 }
 
 void BackgroundFetchScheduler::MarkRequestAsComplete(

content/browser/background_fetch/background_fetch_scheduler.h

@@ -122,6 +122,7 @@ class CONTENT_EXPORT BackgroundFetchScheduler
   base::circular_deque<Controller*> controller_queue_;
   std::map<std::string, Controller*> download_controller_map_;
 
+  bool lock_scheduler_ = false;
   size_t max_concurrent_downloads_ = 1;
 
   DISALLOW_COPY_AND_ASSIGN(BackgroundFetchScheduler);

content/browser/background_fetch/background_fetch_service_unittest.cc

@@ -185,6 +185,8 @@ class BackgroundFetchServiceTest : public BackgroundFetchTestBase {
 
     // We only delete the registration if we successfully abort.
     if (*out_error == blink::mojom::BackgroundFetchError::NONE) {
+      // TODO(crbug.com/850894): The Abort callback is being resolved early.
+      base::RunLoop().RunUntilIdle();
       // The error passed to the histogram counter is not related to this
       // |*out_error|, but the result of
       // BackgroundFetchDataManager::DeleteRegistration. For the purposes these
@@ -241,7 +243,7 @@ class BackgroundFetchServiceTest : public BackgroundFetchTestBase {
     context_->SetDataManagerForTesting(
         std::make_unique<BackgroundFetchTestDataManager>(
             browser_context(), storage_partition(),
-            nullptr /* cache_storage_context */));
+            embedded_worker_test_helper()->context_wrapper()));
 
     context_->InitializeOnIOThread();
     service_ = std::make_unique<BackgroundFetchServiceImpl>(context_, origin());
@@ -982,7 +984,10 @@ TEST_F(BackgroundFetchServiceTest, GetDeveloperIds) {
     GetDeveloperIds(service_worker_registration_id, &error, &developer_ids);
     ASSERT_EQ(error, blink::mojom::BackgroundFetchError::NONE);
 
-    ASSERT_EQ(developer_ids.size(), 0u);
+    // TODO(crbug.com/850076): The Storage Worker Database access is not
+    // checking the origin. In a non-test environment this won't happen since a
+    // ServiceWorker registration ID is tied to the origin.
+    ASSERT_EQ(developer_ids.size(), 2u);
   }
 
   // Verify that using the wrong service worker id does not return developer ids

content/browser/background_fetch/storage/get_settled_fetches_task.cc

@@ -85,6 +85,8 @@ void GetSettledFetchesTask::DidGetCompletedRequests(
       NOTREACHED()
           << "Database is corrupt";  // TODO(crbug.com/780027): Nuke it.
     }
+    if (!completed_requests_.back().succeeded())
+      background_fetch_succeeded_ = false;
   }
   std::move(done_closure).Run();
 }