[Background Fetch] Update download total after fetch is complete.

After a fetch is complete, the metadata in the ServiceWorker database
is updated with the new total_downloaded value.

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

content/browser/background_fetch/background_fetch_data_manager_unittest.cc

@@ -15,6 +15,7 @@
 #include "base/command_line.h"
 #include "base/guid.h"
 #include "base/run_loop.h"
+#include "base/strings/string_number_conversions.h"
 #include "content/browser/background_fetch/background_fetch_request_info.h"
 #include "content/browser/background_fetch/background_fetch_test_base.h"
 #include "content/browser/background_fetch/background_fetch_test_data_manager.h"
@@ -44,6 +45,8 @@ const char kAlternativeUniqueId[] = "bb48a9fb-c21f-4c2d-a9ae-58bd48a9fb53";
 const char kInitialTitle[] = "Initial Title";
 const char kUpdatedTitle[] = "Updated Title";
 
+constexpr size_t kResponseFileSize = 42u;
+
 void DidCreateRegistration(
     base::Closure quit_closure,
     blink::mojom::BackgroundFetchError* out_error,
@@ -84,7 +87,7 @@ void AnnotateRequestInfoWithFakeDownloadManagerData(
         base::Time::Now(), BackgroundFetchResult::FailureReason::UNKNOWN));
   } else {
     request_info->SetResult(std::make_unique<BackgroundFetchResult>(
-        base::Time::Now(), base::FilePath(), 42u /* file_size */));
+        base::Time::Now(), base::FilePath(), kResponseFileSize));
   }
 }
 
@@ -110,6 +113,17 @@ bool operator==(const ResponseStateStats& s1, const ResponseStateStats& s2) {
          s1.completed_requests == s2.completed_requests;
 }
 
+std::vector<ServiceWorkerFetchRequest> CreateValidRequests(
+    const url::Origin& origin,
+    size_t num_requests = 1u) {
+  std::vector<ServiceWorkerFetchRequest> requests(num_requests);
+  for (size_t i = 0; i < requests.size(); i++) {
+    // Creates a URL of the form: `http://example.com/x`
+    requests[i].url = GURL(origin.GetURL().spec() + base::NumberToString(i));
+  }
+  return requests;
+}
+
 }  // namespace
 
 class BackgroundFetchDataManagerTest : public BackgroundFetchTestBase {
@@ -885,21 +899,66 @@ TEST_F(BackgroundFetchDataManagerTest, PopNextRequestAndMarkAsComplete) {
                           2 /* completed_requests */}));
 }
 
+TEST_F(BackgroundFetchDataManagerTest, DownloadTotalUpdated) {
+  int64_t sw_id = RegisterServiceWorker();
+  ASSERT_NE(blink::mojom::kInvalidServiceWorkerRegistrationId, sw_id);
+
+  BackgroundFetchRegistrationId registration_id(
+      sw_id, origin(), kExampleDeveloperId, kExampleUniqueId);
+  auto requests = CreateValidRequests(origin(), 3u /* num_requests */);
+
+  BackgroundFetchOptions options;
+  blink::mojom::BackgroundFetchError error;
+  CreateRegistration(registration_id, requests, options, &error);
+  EXPECT_EQ(error, blink::mojom::BackgroundFetchError::NONE);
+
+  auto registration =
+      GetRegistration(sw_id, origin(), kExampleDeveloperId, &error);
+  ASSERT_EQ(error, blink::mojom::BackgroundFetchError::NONE);
+  EXPECT_EQ(registration->download_total, 0u);
+
+  scoped_refptr<BackgroundFetchRequestInfo> request_info;
+  PopNextRequest(registration_id, &request_info);
+  AnnotateRequestInfoWithFakeDownloadManagerData(request_info.get(),
+                                                 true /* succeeded */);
+  MarkRequestAsComplete(registration_id, request_info.get());
+
+  registration = GetRegistration(sw_id, origin(), kExampleDeveloperId, &error);
+  ASSERT_EQ(error, blink::mojom::BackgroundFetchError::NONE);
+  EXPECT_EQ(registration->download_total, kResponseFileSize);
+
+  PopNextRequest(registration_id, &request_info);
+  AnnotateRequestInfoWithFakeDownloadManagerData(request_info.get(),
+                                                 true /* succeeded */);
+  MarkRequestAsComplete(registration_id, request_info.get());
+
+  registration = GetRegistration(sw_id, origin(), kExampleDeveloperId, &error);
+  ASSERT_EQ(error, blink::mojom::BackgroundFetchError::NONE);
+  EXPECT_EQ(registration->download_total, 2 * kResponseFileSize);
+
+  PopNextRequest(registration_id, &request_info);
+  AnnotateRequestInfoWithFakeDownloadManagerData(request_info.get(),
+                                                 false /* succeeded */);
+  MarkRequestAsComplete(registration_id, request_info.get());
+
+  registration = GetRegistration(sw_id, origin(), kExampleDeveloperId, &error);
+  ASSERT_EQ(error, blink::mojom::BackgroundFetchError::NONE);
+  // |download_total| is unchanged.
+  EXPECT_EQ(registration->download_total, 2 * kResponseFileSize);
+}
+
 TEST_F(BackgroundFetchDataManagerTest, WriteToCache) {
   int64_t sw_id = RegisterServiceWorker();
   ASSERT_NE(blink::mojom::kInvalidServiceWorkerRegistrationId, sw_id);
 
   BackgroundFetchRegistrationId registration_id(
       sw_id, origin(), kExampleDeveloperId, kExampleUniqueId);
-  ServiceWorkerFetchRequest request1;
-  request1.url = GURL(origin().GetURL().spec() + "1");
-  ServiceWorkerFetchRequest request2;
-  request2.url = GURL(origin().GetURL().spec() + "2");
+  auto requests = CreateValidRequests(origin(), 2u /* num_requests */);
 
   BackgroundFetchOptions options;
   blink::mojom::BackgroundFetchError error;
 
-  CreateRegistration(registration_id, {request1, request2}, options, &error);
+  CreateRegistration(registration_id, requests, options, &error);
   EXPECT_EQ(error, blink::mojom::BackgroundFetchError::NONE);
 
   scoped_refptr<BackgroundFetchRequestInfo> request_info;
@@ -913,8 +972,8 @@ TEST_F(BackgroundFetchDataManagerTest, WriteToCache) {
   EXPECT_TRUE(HasCache(kExampleUniqueId));
   EXPECT_FALSE(HasCache("foo"));
 
-  EXPECT_TRUE(MatchCache(request1));
-  EXPECT_FALSE(MatchCache(request2));
+  EXPECT_TRUE(MatchCache(requests[0]));
+  EXPECT_FALSE(MatchCache(requests[1]));
 
   PopNextRequest(registration_id, &request_info);
   ASSERT_TRUE(request_info);
@@ -922,12 +981,12 @@ TEST_F(BackgroundFetchDataManagerTest, WriteToCache) {
   AnnotateRequestInfoWithFakeDownloadManagerData(request_info.get(),
                                                  true /* success */);
   MarkRequestAsComplete(registration_id, request_info.get());
-  EXPECT_TRUE(MatchCache(request1));
-  EXPECT_TRUE(MatchCache(request2));
+  EXPECT_TRUE(MatchCache(requests[0]));
+  EXPECT_TRUE(MatchCache(requests[1]));
 
   RestartDataManagerFromPersistentStorage();
-  EXPECT_TRUE(MatchCache(request1));
-  EXPECT_TRUE(MatchCache(request2));
+  EXPECT_TRUE(MatchCache(requests[0]));
+  EXPECT_TRUE(MatchCache(requests[1]));
 }
 
 TEST_F(BackgroundFetchDataManagerTest, CacheDeleted) {
@@ -1021,14 +1080,11 @@ TEST_F(BackgroundFetchDataManagerTest, GetSettledFetchesFromCache) {
 
   BackgroundFetchRegistrationId registration_id(
       sw_id, origin(), kExampleDeveloperId, kExampleUniqueId);
-  ServiceWorkerFetchRequest request1;
-  request1.url = GURL(origin().GetURL().spec() + "1");
-  ServiceWorkerFetchRequest request2;
-  request2.url = GURL(origin().GetURL().spec() + "2");
+  auto requests = CreateValidRequests(origin(), 2u /* num_requests */);
 
   BackgroundFetchOptions options;
   blink::mojom::BackgroundFetchError error;
-  CreateRegistration(registration_id, {request1, request2}, options, &error);
+  CreateRegistration(registration_id, requests, options, &error);
   EXPECT_EQ(error, blink::mojom::BackgroundFetchError::NONE);
 
   bool succeeded = false;
@@ -1066,8 +1122,8 @@ TEST_F(BackgroundFetchDataManagerTest, GetSettledFetchesFromCache) {
   ASSERT_EQ(settled_fetches.size(), 2u);
 
   // Sanity check that the responses are written to / read from the cache.
-  EXPECT_TRUE(MatchCache(request1));
-  EXPECT_TRUE(MatchCache(request2));
+  EXPECT_TRUE(MatchCache(requests[0]));
+  EXPECT_TRUE(MatchCache(requests[1]));
   EXPECT_EQ(settled_fetches[0].response.cache_storage_cache_name,
             kExampleUniqueId);
   EXPECT_EQ(settled_fetches[1].response.cache_storage_cache_name,

content/browser/background_fetch/storage/mark_request_complete_task.cc

@@ -4,6 +4,7 @@
 
 #include "content/browser/background_fetch/storage/mark_request_complete_task.h"
 
+#include "base/barrier_closure.h"
 #include "content/browser/background_fetch/background_fetch_data_manager.h"
 #include "content/browser/background_fetch/storage/database_helpers.h"
 #include "content/browser/cache_storage/cache_storage_manager.h"
@@ -32,10 +33,15 @@ MarkRequestCompleteTask::MarkRequestCompleteTask(
 MarkRequestCompleteTask::~MarkRequestCompleteTask() = default;
 
 void MarkRequestCompleteTask::Start() {
-  StoreResponse();
+  base::RepeatingClosure barrier_closure = base::BarrierClosure(
+      2u, base::BindOnce(&MarkRequestCompleteTask::CheckAndCallFinished,
+                         weak_factory_.GetWeakPtr()));
+
+  StoreResponse(barrier_closure);
+  UpdateMetadata(barrier_closure);
 }
 
-void MarkRequestCompleteTask::StoreResponse() {
+void MarkRequestCompleteTask::StoreResponse(base::OnceClosure done_closure) {
   auto response = std::make_unique<ServiceWorkerResponse>();
 
   is_response_successful_ = data_manager()->FillServiceWorkerResponse(
@@ -43,7 +49,7 @@ void MarkRequestCompleteTask::StoreResponse() {
 
   // A valid non-empty url is needed if we want to write to the cache.
   if (!request_info_->fetch_request().url.is_valid()) {
-    CreateAndStoreCompletedRequest();
+    CreateAndStoreCompletedRequest(std::move(done_closure));
     return;
   }
 
@@ -51,16 +57,18 @@ void MarkRequestCompleteTask::StoreResponse() {
       registration_id_.origin(), CacheStorageOwner::kBackgroundFetch,
       registration_id_.unique_id() /* cache_name */,
       base::BindOnce(&MarkRequestCompleteTask::DidOpenCache,
-                     weak_factory_.GetWeakPtr(), std::move(response)));
+                     weak_factory_.GetWeakPtr(), std::move(response),
+                     std::move(done_closure)));
 }
 
 void MarkRequestCompleteTask::DidOpenCache(
     std::unique_ptr<ServiceWorkerResponse> response,
+    base::OnceClosure done_closure,
     CacheStorageCacheHandle handle,
     blink::mojom::CacheStorageError error) {
   if (error != blink::mojom::CacheStorageError::kSuccess) {
     // TODO(crbug.com/780025): Log failures to UMA.
-    CreateAndStoreCompletedRequest();
+    CreateAndStoreCompletedRequest(std::move(done_closure));
     return;
   }
   DCHECK(handle.value());
@@ -73,17 +81,20 @@ void MarkRequestCompleteTask::DidOpenCache(
   handle.value()->Put(
       std::move(request), std::move(response),
       base::BindOnce(&MarkRequestCompleteTask::DidWriteToCache,
-                     weak_factory_.GetWeakPtr(), std::move(handle)));
+                     weak_factory_.GetWeakPtr(), std::move(handle),
+                     std::move(done_closure)));
 }
 
 void MarkRequestCompleteTask::DidWriteToCache(
     CacheStorageCacheHandle handle,
+    base::OnceClosure done_closure,
     blink::mojom::CacheStorageError error) {
   // TODO(crbug.com/780025): Log failures to UMA.
-  CreateAndStoreCompletedRequest();
+  CreateAndStoreCompletedRequest(std::move(done_closure));
 }
 
-void MarkRequestCompleteTask::CreateAndStoreCompletedRequest() {
+void MarkRequestCompleteTask::CreateAndStoreCompletedRequest(
+    base::OnceClosure done_closure) {
   completed_request_.set_unique_id(registration_id_.unique_id());
   completed_request_.set_request_index(request_info_->request_index());
   completed_request_.set_serialized_request(
@@ -97,11 +108,12 @@ void MarkRequestCompleteTask::CreateAndStoreCompletedRequest() {
       {{CompletedRequestKey(completed_request_.unique_id(),
                             completed_request_.request_index()),
         completed_request_.SerializeAsString()}},
-      base::BindRepeating(&MarkRequestCompleteTask::DidStoreCompletedRequest,
-                          weak_factory_.GetWeakPtr()));
+      base::BindOnce(&MarkRequestCompleteTask::DidStoreCompletedRequest,
+                     weak_factory_.GetWeakPtr(), std::move(done_closure)));
 }
 
 void MarkRequestCompleteTask::DidStoreCompletedRequest(
+    base::OnceClosure done_closure,
     ServiceWorkerStatusCode status) {
   switch (ToDatabaseStatus(status)) {
     case DatabaseStatus::kOk:
@@ -109,7 +121,7 @@ void MarkRequestCompleteTask::DidStoreCompletedRequest(
     case DatabaseStatus::kFailed:
     case DatabaseStatus::kNotFound:
       // TODO(crbug.com/780025): Log failures to UMA.
-      Finished();  // Destroys |this|.
+      std::move(done_closure).Run();
       return;
   }
 
@@ -119,24 +131,70 @@ void MarkRequestCompleteTask::DidStoreCompletedRequest(
       {ActiveRequestKey(completed_request_.unique_id(),
                         completed_request_.request_index())},
       base::BindOnce(&MarkRequestCompleteTask::DidDeleteActiveRequest,
-                     weak_factory_.GetWeakPtr()));
+                     weak_factory_.GetWeakPtr(), std::move(done_closure)));
 }
 
 void MarkRequestCompleteTask::DidDeleteActiveRequest(
+    base::OnceClosure done_closure,
+    ServiceWorkerStatusCode status) {
+  // TODO(crbug.com/780025): Log failures to UMA.
+  std::move(done_closure).Run();
+}
+
+void MarkRequestCompleteTask::UpdateMetadata(base::OnceClosure done_closure) {
+  if (!request_info_->IsResultSuccess() || request_info_->GetFileSize() == 0u) {
+    std::move(done_closure).Run();
+    return;
+  }
+
+  service_worker_context()->GetRegistrationUserData(
+      registration_id_.service_worker_registration_id(),
+      {RegistrationKey(registration_id_.unique_id())},
+      base::BindOnce(&MarkRequestCompleteTask::DidGetMetadata,
+                     weak_factory_.GetWeakPtr(), std::move(done_closure)));
+}
+
+void MarkRequestCompleteTask::DidGetMetadata(
+    base::OnceClosure done_closure,
+    const std::vector<std::string>& data,
     ServiceWorkerStatusCode status) {
   switch (ToDatabaseStatus(status)) {
     case DatabaseStatus::kNotFound:
     case DatabaseStatus::kFailed:
       // TODO(crbug.com/780025): Log failures to UMA.
-      break;
+      std::move(done_closure).Run();
+      return;
     case DatabaseStatus::kOk:
-      std::move(callback_).Run();
+      DCHECK_EQ(1u, data.size());
       break;
   }
 
-  Finished();  // Destroys |this|.
+  proto::BackgroundFetchMetadata metadata;
+  if (!metadata.ParseFromString(data.front())) {
+    NOTREACHED() << "Database is corrupt";  // TODO(crbug.com/780027): Nuke it.
+  }
+
+  metadata.mutable_registration()->set_download_total(
+      metadata.registration().download_total() + request_info_->GetFileSize());
+
+  service_worker_context()->StoreRegistrationUserData(
+      registration_id_.service_worker_registration_id(),
+      registration_id_.origin().GetURL(),
+      {{RegistrationKey(registration_id_.unique_id()),
+        metadata.SerializeAsString()}},
+      base::BindOnce(&MarkRequestCompleteTask::DidStoreMetadata,
+                     weak_factory_.GetWeakPtr(), std::move(done_closure)));
+}
+
+void MarkRequestCompleteTask::DidStoreMetadata(base::OnceClosure done_closure,
+                                               ServiceWorkerStatusCode status) {
+  // TODO(crbug.com/780025): Log failures to UMA.
+  std::move(done_closure).Run();
+}
 
-  // TODO(rayankans): Update download stats.
+void MarkRequestCompleteTask::CheckAndCallFinished() {
+  std::move(callback_).Run();
+  Finished();
 }
 
 }  // namespace background_fetch

content/browser/background_fetch/storage/mark_request_complete_task.h

@@ -38,20 +38,35 @@ class MarkRequestCompleteTask : public DatabaseTask {
   void Start() override;
 
  private:
-  void StoreResponse();
+  void StoreResponse(base::OnceClosure done_closure);
 
   void DidOpenCache(std::unique_ptr<ServiceWorkerResponse> response,
+                    base::OnceClosure done_closure,
                     CacheStorageCacheHandle handle,
                     blink::mojom::CacheStorageError error);
 
   void DidWriteToCache(CacheStorageCacheHandle handle,
+                       base::OnceClosure done_closure,
                        blink::mojom::CacheStorageError error);
 
-  void CreateAndStoreCompletedRequest();
+  void CreateAndStoreCompletedRequest(base::OnceClosure done_closure);
 
-  void DidStoreCompletedRequest(ServiceWorkerStatusCode status);
+  void DidStoreCompletedRequest(base::OnceClosure done_closure,
+                                ServiceWorkerStatusCode status);
 
-  void DidDeleteActiveRequest(ServiceWorkerStatusCode status);
+  void DidDeleteActiveRequest(base::OnceClosure done_closure,
+                              ServiceWorkerStatusCode status);
+
+  void UpdateMetadata(base::OnceClosure done_closure);
+
+  void DidGetMetadata(base::OnceClosure done_closure,
+                      const std::vector<std::string>& data,
+                      ServiceWorkerStatusCode status);
+
+  void DidStoreMetadata(base::OnceClosure done_closure,
+                        ServiceWorkerStatusCode status);
+
+  void CheckAndCallFinished();
 
   BackgroundFetchRegistrationId registration_id_;
   scoped_refptr<BackgroundFetchRequestInfo> request_info_;