[Offline pages] Removing active_request_ from request coordinator

components/offline_pages/core/background/request_coordinator.cc

Index: components/offline_pages/core/background/request_coordinator.cc
diff --git a/components/offline_pages/core/background/request_coordinator.cc b/components/offline_pages/core/background/request_coordinator.cc
index e1a7170a7da2ba79eec4380d8850a3f58d8026d2..c8ef7aeb6144ed9e655231e5534daa92163fd5fe 100644
--- a/components/offline_pages/core/background/request_coordinator.cc
+++ b/components/offline_pages/core/background/request_coordinator.cc
@@ -207,7 +207,7 @@ RequestCoordinator::RequestCoordinator(
       scheduler_(std::move(scheduler)),
       policy_controller_(new ClientPolicyController()),
       network_quality_estimator_(network_quality_estimator),
-      active_request_(nullptr),
+      active_request_id_(0),
       last_offlining_status_(Offliner::RequestStatus::UNKNOWN),
       scheduler_callback_(base::Bind(&EmptySchedulerCallback)),
       internal_start_processing_callback_(base::Bind(&EmptySchedulerCallback)),
@@ -284,21 +284,19 @@ void RequestCoordinator::GetQueuedRequestsCallback(
   callback.Run(std::move(requests));
 }
 
-void RequestCoordinator::StopPrerendering(
-    const Offliner::CancelCallback& final_callback,
-    Offliner::RequestStatus stop_status) {
+void RequestCoordinator::StopPrerendering(const CancelCallback& final_callback,
+                                          Offliner::RequestStatus stop_status) {
   if (offliner_ && state_ == RequestCoordinatorState::OFFLINING) {
-    DCHECK(active_request_.get());
-    offliner_->Cancel(base::Bind(
-        &RequestCoordinator::HandleCancelUpdateStatusCallback,
-        weak_ptr_factory_.GetWeakPtr(), final_callback, stop_status));
-    return;
+    DCHECK_NE(active_request_id_, 0);
+    if (offliner_->Cancel(base::Bind(
+            &RequestCoordinator::HandleCancelUpdateStatusCallback,
+            weak_ptr_factory_.GetWeakPtr(), final_callback, stop_status))) {
+      return;
+    }
   }
 
   UpdateStatusForCancel(stop_status);
-  int64_t request_id =
-      active_request_.get() ? active_request_->request_id() : 0LL;
-  final_callback.Run(request_id);
+  final_callback.Run(active_request_id_);
 }
 
 void RequestCoordinator::GetRequestsForSchedulingCallback(
@@ -326,9 +324,9 @@ bool RequestCoordinator::CancelActiveRequestIfItMatches(
   // If we have a request in progress and need to cancel it, call the
   // pre-renderer to cancel.  TODO Make sure we remove any page created by the
   // prerenderer if it doesn't get the cancel in time.
-  if (active_request_ != nullptr) {
-    if (request_ids.end() != std::find(request_ids.begin(), request_ids.end(),
-                                       active_request_->request_id())) {
+  if (active_request_id_ != 0) {
+    if (request_ids.end() !=
+        std::find(request_ids.begin(), request_ids.end(), active_request_id_)) {
       StopPrerendering(
           base::Bind(&RequestCoordinator::ResetActiveRequestCallback,
                      weak_ptr_factory_.GetWeakPtr()),
@@ -526,42 +524,33 @@ void RequestCoordinator::HandleRemovedRequests(
 }
 
 void RequestCoordinator::HandleCancelUpdateStatusCallback(
-    const Offliner::CancelCallback& final_callback,
+    const CancelCallback& final_callback,
     Offliner::RequestStatus stop_status,
-    int64_t offline_id) {
+    const SavePageRequest& canceled_request) {
   if (stop_status == Offliner::RequestStatus::REQUEST_COORDINATOR_TIMED_OUT ||
       stop_status == Offliner::RequestStatus::BACKGROUND_SCHEDULER_CANCELED) {
     // Consider watchdog timeout a completed attempt.
-    SavePageRequest request(*active_request_.get());
-    UpdateRequestForCompletedAttempt(request, stop_status);
+    UpdateRequestForCompletedAttempt(canceled_request, stop_status);
   } else {
     // Otherwise consider this stop an aborted attempt.
-    UpdateRequestForAbortedAttempt(*active_request_.get());
+    UpdateRequestForAbortedAttempt(canceled_request);
   }
 
+  RecordOfflinerResult(canceled_request, stop_status);
   UpdateStatusForCancel(stop_status);
-  final_callback.Run(offline_id);
+  final_callback.Run(canceled_request.request_id());
 }
 
 void RequestCoordinator::UpdateStatusForCancel(
     Offliner::RequestStatus stop_status) {
   // Stopping offliner means it will not call callback so set last status.
   last_offlining_status_ = stop_status;
-
-  if (active_request_) {
-    event_logger_.RecordOfflinerResult(active_request_->client_id().name_space,
-                                       last_offlining_status_,
-                                       active_request_->request_id());
-    RecordOfflinerResultUMA(active_request_->client_id(),
-                            active_request_->creation_time(),
-                            last_offlining_status_);
-    active_request_.reset();
-  }
+  active_request_id_ = 0;
   state_ = RequestCoordinatorState::IDLE;
 }
 
 void RequestCoordinator::ResetActiveRequestCallback(int64_t offline_id) {
-  active_request_.reset();
+  active_request_id_ = 0;
 }
 
 void RequestCoordinator::StartSchedulerCallback(int64_t offline_id) {
@@ -589,7 +578,7 @@ void RequestCoordinator::StopProcessing(Offliner::RequestStatus stop_status) {
 void RequestCoordinator::HandleWatchdogTimeout() {
   Offliner::RequestStatus watchdog_status =
       Offliner::REQUEST_COORDINATOR_TIMED_OUT;
-  if (offliner_->HandleTimeout(*active_request_.get()))
+  if (offliner_->HandleTimeout(active_request_id_))
     return;
   StopPrerendering(base::Bind(&RequestCoordinator::TryNextRequestCallback,
                               weak_ptr_factory_.GetWeakPtr()),
@@ -886,10 +875,7 @@ void RequestCoordinator::StartOffliner(
     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)));
+  active_request_id_ = request_id;
 
   // Start the load and save process in the offliner (Async).
   if (offliner_->LoadAndSave(
@@ -909,7 +895,7 @@ void RequestCoordinator::StartOffliner(
     }
 
     // Inform observer of active request.
-    NotifyChanged(*active_request_.get());
+    NotifyChanged(update_result->updated_items.at(0));
 
     // Start a watchdog timer to catch pre-renders running too long
     watchdog_timer_.Start(FROM_HERE, timeout, this,
@@ -932,14 +918,11 @@ void RequestCoordinator::OfflinerDoneCallback(const SavePageRequest& request,
            << ", status: " << static_cast<int>(status) << ", " << __func__;
   DCHECK_NE(status, Offliner::RequestStatus::UNKNOWN);
   DCHECK_NE(status, Offliner::RequestStatus::LOADED);
-  event_logger_.RecordOfflinerResult(request.client_id().name_space, status,
-                                     request.request_id());
+  RecordOfflinerResult(request, status);
   last_offlining_status_ = status;
-  RecordOfflinerResultUMA(request.client_id(), request.creation_time(),
-                          last_offlining_status_);
   watchdog_timer_.Stop();
   state_ = RequestCoordinatorState::IDLE;
-  active_request_.reset(nullptr);
+  active_request_id_ = 0;
 
   UpdateRequestForCompletedAttempt(request, status);
   if (ShouldTryNextRequest(status))
@@ -1103,6 +1086,13 @@ ClientPolicyController* RequestCoordinator::GetPolicyController() {
   return policy_controller_.get();
 }
 
+void RequestCoordinator::RecordOfflinerResult(const SavePageRequest& request,
+                                              Offliner::RequestStatus status) {
+  event_logger_.RecordOfflinerResult(request.client_id().name_space, status,
+                                     request.request_id());
+  RecordOfflinerResultUMA(request.client_id(), request.creation_time(), status);
+}
+
 void RequestCoordinator::Shutdown() {
   network_quality_estimator_ = nullptr;
 }