[Background Fetch] Store failed fetches in Cache Storage.

Store failed fetches in the cache, since the download manager returns
useful information, such as the status code (404, 501, etc).

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

content/browser/background_fetch/storage/get_settled_fetches_task.cc

@@ -107,15 +107,11 @@ void GetSettledFetchesTask::GetResponses() {
 
   settled_fetches_.reserve(completed_requests_.size());
   for (const auto& completed_request : completed_requests_) {
-    settled_fetches_.emplace_back(BackgroundFetchSettledFetch());
+    settled_fetches_.emplace_back();
     settled_fetches_.back().request =
         std::move(ServiceWorkerFetchRequest::ParseFromString(
             completed_request.serialized_request()));
-    if (!completed_request.succeeded()) {
-      FillFailedResponse(&settled_fetches_.back().response, barrier_closure);
-      continue;
-    }
-    FillSuccessfulResponse(&settled_fetches_.back(), barrier_closure);
+    FillResponse(&settled_fetches_.back(), barrier_closure);
   }
 
   // The callback within |barrier_closure| eventually calls Finished(), which
@@ -127,18 +123,7 @@ void GetSettledFetchesTask::GetResponses() {
   barrier_closure.Run();
 }
 
-void GetSettledFetchesTask::FillFailedResponse(ServiceWorkerResponse* response,
-                                               base::OnceClosure callback) {
-  DCHECK(response);
-  background_fetch_succeeded_ = false;
-
-  // TODO(rayankans): Fill failed response with error reports.
-  response->response_type = network::mojom::FetchResponseType::kError;
-
-  std::move(callback).Run();
-}
-
-void GetSettledFetchesTask::FillSuccessfulResponse(
+void GetSettledFetchesTask::FillResponse(
     BackgroundFetchSettledFetch* settled_fetch,
     base::OnceClosure callback) {
   DCHECK(settled_fetch);
@@ -147,23 +132,36 @@ void GetSettledFetchesTask::FillSuccessfulResponse(
   auto request =
       std::make_unique<ServiceWorkerFetchRequest>(settled_fetch->request);
 
-  handle_.value()->Match(
-      std::move(request), nullptr /* match_params */,
-      base::BindOnce(&GetSettledFetchesTask::DidMatchRequest,
-                     weak_factory_.GetWeakPtr(), &settled_fetch->response,
-                     std::move(callback)));
+  handle_.value()->Match(std::move(request), nullptr /* match_params */,
+                         base::BindOnce(&GetSettledFetchesTask::DidMatchRequest,
+                                        weak_factory_.GetWeakPtr(),
+                                        settled_fetch, std::move(callback)));
 }
 
 void GetSettledFetchesTask::DidMatchRequest(
-    ServiceWorkerResponse* response,
+    BackgroundFetchSettledFetch* settled_fetch,
     base::OnceClosure callback,
     blink::mojom::CacheStorageError error,
     std::unique_ptr<ServiceWorkerResponse> cache_response) {
   if (error != blink::mojom::CacheStorageError::kSuccess) {
-    FillFailedResponse(response, std::move(callback));
+    DCHECK(settled_fetch);
+    FillUncachedResponse(settled_fetch, std::move(callback));
     return;
   }
-  *response = std::move(*cache_response);
+  settled_fetch->response = std::move(*cache_response);
+  std::move(callback).Run();
+}
+
+void GetSettledFetchesTask::FillUncachedResponse(
+    BackgroundFetchSettledFetch* settled_fetch,
+    base::OnceClosure callback) {
+  background_fetch_succeeded_ = false;
+
+  // TODO(rayankans): Fill unmatched response with error reports.
+  settled_fetch->response.response_type =
+      network::mojom::FetchResponseType::kError;
+  settled_fetch->response.url_list.push_back(settled_fetch->request.url);
+
   std::move(callback).Run();
 }
 

content/browser/background_fetch/storage/get_settled_fetches_task.h

@@ -47,13 +47,13 @@ class GetSettledFetchesTask : public DatabaseTask {
 
   void GetResponses();
 
-  void FillFailedResponse(ServiceWorkerResponse* response,
-                          base::OnceClosure callback);
+  void FillUncachedResponse(BackgroundFetchSettledFetch* settled_fetch,
+                            base::OnceClosure callback);
 
-  void FillSuccessfulResponse(BackgroundFetchSettledFetch* settled_fetch,
-                              base::OnceClosure callback);
+  void FillResponse(BackgroundFetchSettledFetch* settled_fetch,
+                    base::OnceClosure callback);
 
-  void DidMatchRequest(ServiceWorkerResponse* response,
+  void DidMatchRequest(BackgroundFetchSettledFetch* settled_fetch,
                        base::OnceClosure callback,
                        blink::mojom::CacheStorageError error,
                        std::unique_ptr<ServiceWorkerResponse> cache_response);

content/browser/background_fetch/storage/mark_request_complete_task.cc

@@ -37,14 +37,16 @@ void MarkRequestCompleteTask::Start() {
 
 void MarkRequestCompleteTask::StoreResponse() {
   auto response = std::make_unique<ServiceWorkerResponse>();
-  bool is_response_valid = data_manager()->FillServiceWorkerResponse(
+
+  is_response_successful_ = data_manager()->FillServiceWorkerResponse(
       *request_info_, registration_id_.origin(), response.get());
 
-  if (!is_response_valid) {
-    // No point in caching the response, just do the request state transition.
-    CreateAndStoreCompletedRequest(/* succeeded = */ false);
+  // A valid non-empty url is needed if we want to write to the cache.
+  if (!request_info_->fetch_request().url.is_valid()) {
+    CreateAndStoreCompletedRequest();
     return;
   }
+
   cache_manager_->OpenCache(
       registration_id_.origin(), CacheStorageOwner::kBackgroundFetch,
       registration_id_.unique_id() /* cache_name */,
@@ -58,7 +60,7 @@ void MarkRequestCompleteTask::DidOpenCache(
     blink::mojom::CacheStorageError error) {
   if (error != blink::mojom::CacheStorageError::kSuccess) {
     // TODO(crbug.com/780025): Log failures to UMA.
-    CreateAndStoreCompletedRequest(false);
+    CreateAndStoreCompletedRequest();
     return;
   }
   DCHECK(handle.value());
@@ -78,17 +80,16 @@ void MarkRequestCompleteTask::DidWriteToCache(
     CacheStorageCacheHandle handle,
     blink::mojom::CacheStorageError error) {
   // TODO(crbug.com/780025): Log failures to UMA.
-  CreateAndStoreCompletedRequest(
-      /* succeeded = */ error == blink::mojom::CacheStorageError::kSuccess);
+  CreateAndStoreCompletedRequest();
 }
 
-void MarkRequestCompleteTask::CreateAndStoreCompletedRequest(bool succeeded) {
+void MarkRequestCompleteTask::CreateAndStoreCompletedRequest() {
   completed_request_.set_unique_id(registration_id_.unique_id());
   completed_request_.set_request_index(request_info_->request_index());
   completed_request_.set_serialized_request(
       request_info_->fetch_request().Serialize());
   completed_request_.set_download_guid(request_info_->download_guid());
-  completed_request_.set_succeeded(succeeded);
+  completed_request_.set_succeeded(is_response_successful_);
 
   service_worker_context()->StoreRegistrationUserData(
       registration_id_.service_worker_registration_id(),

content/browser/background_fetch/storage/mark_request_complete_task.h

@@ -47,7 +47,7 @@ class MarkRequestCompleteTask : public DatabaseTask {
   void DidWriteToCache(CacheStorageCacheHandle handle,
                        blink::mojom::CacheStorageError error);
 
-  void CreateAndStoreCompletedRequest(bool succeeded);
+  void CreateAndStoreCompletedRequest();
 
   void DidStoreCompletedRequest(ServiceWorkerStatusCode status);
 
@@ -59,6 +59,7 @@ class MarkRequestCompleteTask : public DatabaseTask {
   MarkedCompleteCallback callback_;
 
   proto::BackgroundFetchCompletedRequest completed_request_;
+  bool is_response_successful_;
 
   base::WeakPtrFactory<MarkRequestCompleteTask> weak_factory_;  // Keep as last.