[Offline Pages] Converts MarkAttemptCompleted to use TaskQueue

components/offline_pages/background/request_coordinator.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/offline_pages/background/request_coordinator.h"
 
 #include <limits>
-#include <string>
 #include <utility>
-#include <vector>
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/logging.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/rand_util.h"
 #include "base/sys_info.h"
 #include "base/time/time.h"
 #include "components/offline_pages/background/offliner_factory.h"
 #include "components/offline_pages/background/offliner_policy.h"
@@ skipping 102 matching lines @@
 // This should use the same algorithm as we use for OfflinePageItem, so the IDs
 // are similar.
 int64_t GenerateOfflineId() {
   return base::RandGenerator(std::numeric_limits<int64_t>::max()) + 1;
 }
 
 // In case we start processing from SavePageLater, we need a callback, but there
 // 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(
     std::unique_ptr<OfflinerPolicy> policy,
     std::unique_ptr<OfflinerFactory> factory,
     std::unique_ptr<RequestQueue> queue,
     std::unique_ptr<Scheduler> scheduler,
     net::NetworkQualityEstimator::NetworkQualityProvider*
         network_quality_estimator)
     : is_low_end_device_(base::SysInfo::IsLowEndDevice()),
@@ skipping 150 matching lines @@
                                     weak_ptr_factory_.GetWeakPtr(), result));
   RecordAttemptCount(request, result);
 }
 
 void RequestCoordinator::MarkAttemptAbortedDone(
     int64_t request_id,
     const ClientId& client_id,
     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);
   }
 }
 
 void RequestCoordinator::RemoveRequests(
     const std::vector<int64_t>& request_ids,
     const RemoveRequestsCallback& callback) {
   bool canceled = CancelActiveRequestIfItMatches(request_ids);
   queue_->RemoveRequests(
       request_ids,
       base::Bind(&RequestCoordinator::HandleRemovedRequestsAndCallback,
@@ skipping 38 matching lines @@
     RequestQueue::AddRequestResult result,
     const SavePageRequest& request) {
   NotifyAdded(request);
   // Inform the scheduler that we have an outstanding task.
   scheduler_->Schedule(GetTriggerConditions(kUserRequest));
 
   if (request.user_requested())
     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);
   }
 }
 
 void RequestCoordinator::UpdateMultipleRequestsCallback(
     std::unique_ptr<UpdateRequestsResult> result) {
   for (const auto& request : result->updated_items)
     NotifyChanged(request);
 
   bool available_user_request = false;
   for (const auto& request : result->updated_items) {
@@ skipping 230 matching lines @@
     const std::string& client_namespace,
     std::unique_ptr<UpdateRequestsResult> update_result) {
   if (update_result->store_state != StoreState::LOADED ||
       update_result->item_statuses.size() != 1 ||
       update_result->item_statuses.at(0).first != request_id ||
       update_result->item_statuses.at(0).second != ItemActionStatus::SUCCESS) {
     is_busy_ = false;
     // 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;
   }
 
   // TODO(fgorski): Switch to request_id only, so that this value is not written
   // back to the store.
   active_request_.reset(
       new SavePageRequest(update_result->updated_items.at(0)));
 
   // Start the load and save process in the offliner (Async).
   if (offliner_->LoadAndSave(
@@ skipping 58 matching lines @@
     // MarkAttemptCompleted within the if branches, the completed_attempt_count
     // has not yet been updated when we are checking the if condition.
     const BackgroundSavePageResult result(
         BackgroundSavePageResult::RETRY_COUNT_EXCEEDED);
     event_logger_.RecordDroppedSavePageRequest(request.client_id().name_space,
                                                result, request.request_id());
     RemoveAttemptedRequest(request, result);
   } 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
   // processing window based on how the previous request completed.
   switch (status) {
     case Offliner::RequestStatus::SAVED:
     case Offliner::RequestStatus::SAVE_FAILED:
     case Offliner::RequestStatus::REQUEST_COORDINATOR_CANCELED:
     case Offliner::RequestStatus::REQUEST_COORDINATOR_TIMED_OUT:
     case Offliner::RequestStatus::PRERENDERING_FAILED_NO_RETRY:
@@ skipping 83 matching lines @@
 
 ClientPolicyController* RequestCoordinator::GetPolicyController() {
   return policy_controller_.get();
 }
 
 void RequestCoordinator::Shutdown() {
   network_quality_estimator_ = nullptr;
 }
 
 }  // namespace offline_pages