[Offline Pages] Converts MarkAttemptCompleted to use TaskQueue

components/offline_pages/background/request_coordinator.cc

Index: components/offline_pages/background/request_coordinator.cc
diff --git a/components/offline_pages/background/request_coordinator.cc b/components/offline_pages/background/request_coordinator.cc
index cfc32ed6421536e1e1db56baee43602706767d15..5a81e66e148d44f943faa5b35a03b65bce007fd8 100644
--- a/components/offline_pages/background/request_coordinator.cc
+++ b/components/offline_pages/background/request_coordinator.cc
@@ -5,9 +5,7 @@
 #include "components/offline_pages/background/request_coordinator.h"
 
 #include <limits>
-#include <string>
 #include <utility>
-#include <vector>
 
 #include "base/bind.h"
 #include "base/callback.h"
@@ -130,6 +128,13 @@ int64_t GenerateOfflineId() {
 // is nothing for it to do.
 void EmptySchedulerCallback(bool started) {}
 
+// Returns whether |result| is a successful result for a single request.
+bool IsSingleSuccessResult(const UpdateRequestsResult* result) {
+  return result->store_state == StoreState::LOADED &&
+         result->item_statuses.size() == 1 &&
+         result->item_statuses.at(0).second == ItemActionStatus::SUCCESS;
+}
+
 }  // namespace
 
 RequestCoordinator::RequestCoordinator(
@@ -300,16 +305,14 @@ void RequestCoordinator::MarkAttemptAbortedDone(
     std::unique_ptr<UpdateRequestsResult> result) {
   // If the request succeeded, nothing to do.  If it failed, we can't really do
   // much, so just log it.
-  if (result->store_state != StoreState::LOADED ||
-      result->item_statuses.size() != 1 ||
-      result->item_statuses.at(0).first != request_id ||
-      result->item_statuses.at(0).second != ItemActionStatus::SUCCESS) {
+  if (!IsSingleSuccessResult(result.get())) {
     DVLOG(1) << "Failed to mark request aborted: " << request_id;
-    event_logger_.RecordUpdateRequestFailed(
-        client_id.name_space,
+    RequestQueue::UpdateRequestResult request_result =
         result->store_state != StoreState::LOADED
             ? RequestQueue::UpdateRequestResult::STORE_FAILURE
-            : RequestQueue::UpdateRequestResult::REQUEST_DOES_NOT_EXIST);
+            : RequestQueue::UpdateRequestResult::REQUEST_DOES_NOT_EXIST;
+    event_logger_.RecordUpdateRequestFailed(client_id.name_space,
+                                            request_result);
   }
 }
 
@@ -368,16 +371,20 @@ void RequestCoordinator::AddRequestResultCallback(
     StartImmediatelyIfConnected();
 }
 
-// Called in response to updating a request in the request queue.
-void RequestCoordinator::UpdateRequestCallback(
+void RequestCoordinator::MarkAttemptCompletedDoneCallback(
+    int64_t request_id,
     const ClientId& client_id,
-    RequestQueue::UpdateRequestResult result) {
-  // If the request succeeded, nothing to do.  If it failed, we can't really do
-  // much, so just log it.
-  if (result != RequestQueue::UpdateRequestResult::SUCCESS) {
-    DVLOG(1) << "Failed to update request attempt details. "
-             << static_cast<int>(result);
-    event_logger_.RecordUpdateRequestFailed(client_id.name_space, result);
+    std::unique_ptr<UpdateRequestsResult> result) {
+  if (IsSingleSuccessResult(result.get())) {
+    NotifyChanged(result->updated_items.at(0));
+  } else {
+    DVLOG(1) << "Failed to mark request completed " << request_id;
+    RequestQueue::UpdateRequestResult request_result =
+        result->store_state != StoreState::LOADED
+            ? RequestQueue::UpdateRequestResult::STORE_FAILURE
+            : RequestQueue::UpdateRequestResult::REQUEST_DOES_NOT_EXIST;
+    event_logger_.RecordUpdateRequestFailed(client_id.name_space,
+                                            request_result);
   }
 }
 
@@ -628,11 +635,11 @@ void RequestCoordinator::StartOffliner(
     // TODO(fgorski): what is the best result? Do we create a new status?
     StopProcessing(Offliner::PRERENDERING_NOT_STARTED);
     DVLOG(1) << "Failed to mark attempt started: " << request_id;
-    event_logger_.RecordUpdateRequestFailed(
-        client_namespace,
+    RequestQueue::UpdateRequestResult request_result =
         update_result->store_state != StoreState::LOADED
             ? RequestQueue::UpdateRequestResult::STORE_FAILURE
-            : RequestQueue::UpdateRequestResult::REQUEST_DOES_NOT_EXIST);
+            : RequestQueue::UpdateRequestResult::REQUEST_DOES_NOT_EXIST;
+    event_logger_.RecordUpdateRequestFailed(client_namespace, request_result);
     return;
   }
 
@@ -711,13 +718,11 @@ void RequestCoordinator::OfflinerDoneCallback(const SavePageRequest& request,
   } else {
     // If we failed, but are not over the limit, update the request in the
     // queue.
-    SavePageRequest updated_request(request);
-    updated_request.MarkAttemptCompleted();
-    queue_->UpdateRequest(updated_request,
-                          base::Bind(&RequestCoordinator::UpdateRequestCallback,
-                                     weak_ptr_factory_.GetWeakPtr(),
-                                     updated_request.client_id()));
-    NotifyChanged(updated_request);
+    queue_->MarkAttemptCompleted(
+        request.request_id(),
+        base::Bind(&RequestCoordinator::MarkAttemptCompletedDoneCallback,
+                   weak_ptr_factory_.GetWeakPtr(), request.request_id(),
+                   request.client_id()));
   }
 
   // Determine whether we might try another request in this