Use a vector of smart pointers for callback return type.

We can't use references to large objects with std::move,
so change the code to use the vector of pointers itself
instead  of a reference to a vector of pointers.

BUG=637077

Review-Url: https://codereview.chromium.org/2262423002
Cr-Commit-Position: refs/heads/master@{#417462}

chrome/browser/android/offline_pages/downloads/offline_page_download_bridge.cc

@@ -69,14 +69,14 @@ ScopedJavaLocalRef<jobject> ToJavaOfflinePageDownloadItem(
 }
 
 std::vector<int64_t> FilterRequestsByGuid(
-    const std::vector<SavePageRequest>& requests,
+    std::vector<std::unique_ptr<SavePageRequest>> requests,
     const std::string& guid) {
   std::vector<int64_t> request_ids;
-  for (const SavePageRequest& request : requests) {
-    if (request.client_id().id == guid &&
-        (request.client_id().name_space == kDownloadNamespace ||
-         request.client_id().name_space == kAsyncNamespace)) {
-      request_ids.push_back(request.request_id());
+  for (const auto& request : requests) {
+    if (request->client_id().id == guid &&
+        (request->client_id().name_space == kDownloadNamespace ||
+         request->client_id().name_space == kAsyncNamespace)) {
+      request_ids.push_back(request->request_id());
     }
   }
   return request_ids;
@@ -86,13 +86,15 @@ void CancelRequestCallback(const RequestQueue::UpdateMultipleRequestResults&) {
   // Results ignored here, as UI uses observer to update itself.
 }
 
-void CancelRequestsContinuation(content::BrowserContext* browser_context,
-                                const std::string& guid,
-                                const std::vector<SavePageRequest>& requests) {
+void CancelRequestsContinuation(
+    content::BrowserContext* browser_context,
+    const std::string& guid,
+    std::vector<std::unique_ptr<SavePageRequest>> requests) {
   RequestCoordinator* coordinator =
       RequestCoordinatorFactory::GetForBrowserContext(browser_context);
   if (coordinator) {
-    std::vector<int64_t> request_ids = FilterRequestsByGuid(requests, guid);
+    std::vector<int64_t> request_ids =
+        FilterRequestsByGuid(std::move(requests), guid);
     coordinator->RemoveRequests(request_ids,
                                 base::Bind(&CancelRequestCallback));
   } else {
@@ -100,24 +102,27 @@ void CancelRequestsContinuation(content::BrowserContext* browser_context,
   }
 }
 
-void PauseRequestsContinuation(content::BrowserContext* browser_context,
-                               const std::string& guid,
-                               const std::vector<SavePageRequest>& requests) {
+void PauseRequestsContinuation(
+    content::BrowserContext* browser_context,
+    const std::string& guid,
+    std::vector<std::unique_ptr<SavePageRequest>> requests) {
   RequestCoordinator* coordinator =
       RequestCoordinatorFactory::GetForBrowserContext(browser_context);
   if (coordinator)
-    coordinator->PauseRequests(FilterRequestsByGuid(requests, guid));
+    coordinator->PauseRequests(FilterRequestsByGuid(std::move(requests), guid));
   else
     LOG(WARNING) << "PauseRequestsContinuation has no valid coordinator.";
 }
 
-void ResumeRequestsContinuation(content::BrowserContext* browser_context,
-                                const std::string& guid,
-                                const std::vector<SavePageRequest>& requests) {
+void ResumeRequestsContinuation(
+    content::BrowserContext* browser_context,
+    const std::string& guid,
+    std::vector<std::unique_ptr<SavePageRequest>> requests) {
   RequestCoordinator* coordinator =
       RequestCoordinatorFactory::GetForBrowserContext(browser_context);
   if (coordinator)
-    coordinator->ResumeRequests(FilterRequestsByGuid(requests, guid));
+    coordinator->ResumeRequests(
+        FilterRequestsByGuid(std::move(requests), guid));
   else
     LOG(WARNING) << "ResumeRequestsContinuation has no valid coordinator.";
 }

chrome/browser/android/offline_pages/offline_page_bridge.cc

@@ -138,7 +138,7 @@ void SingleOfflinePageItemCallback(
 
 ScopedJavaLocalRef<jobjectArray> CreateJavaSavePageRequests(
     JNIEnv* env,
-    const std::vector<SavePageRequest>& requests) {
+    std::vector<std::unique_ptr<SavePageRequest>> requests) {
   ScopedJavaLocalRef<jclass> save_page_request_clazz = base::android::GetClass(
       env, "org/chromium/chrome/browser/offlinepages/SavePageRequest");
   jobjectArray joa = env->NewObjectArray(
@@ -146,7 +146,7 @@ ScopedJavaLocalRef<jobjectArray> CreateJavaSavePageRequests(
   base::android::CheckException(env);
 
   for (size_t i = 0; i < requests.size(); ++i) {
-    SavePageRequest request = requests[i];
+    SavePageRequest request = *(requests[i]);
     ScopedJavaLocalRef<jstring> name_space =
         ConvertUTF8ToJavaString(env, request.client_id().name_space);
     ScopedJavaLocalRef<jstring> id =
@@ -163,12 +163,13 @@ ScopedJavaLocalRef<jobjectArray> CreateJavaSavePageRequests(
   return ScopedJavaLocalRef<jobjectArray>(env, joa);
 }
 
-void OnGetAllRequestsDone(const ScopedJavaGlobalRef<jobject>& j_callback_obj,
-                          const std::vector<SavePageRequest>& all_requests) {
+void OnGetAllRequestsDone(
+    const ScopedJavaGlobalRef<jobject>& j_callback_obj,
+    std::vector<std::unique_ptr<SavePageRequest>> all_requests) {
   JNIEnv* env = base::android::AttachCurrentThread();
 
   ScopedJavaLocalRef<jobjectArray> j_result_obj =
-      CreateJavaSavePageRequests(env, all_requests);
+      CreateJavaSavePageRequests(env, std::move(all_requests));
   base::android::RunCallbackAndroid(j_callback_obj, j_result_obj);
 }
 

chrome/browser/ui/webui/offline/offline_internals_ui_message_handler.cc

@@ -118,20 +118,21 @@ void OfflineInternalsUIMessageHandler::HandleStoredPagesCallback(
 void OfflineInternalsUIMessageHandler::HandleRequestQueueCallback(
     std::string callback_id,
     offline_pages::RequestQueue::GetRequestsResult result,
-    const std::vector<offline_pages::SavePageRequest>& requests) {
+    std::vector<std::unique_ptr<offline_pages::SavePageRequest>> requests) {
   base::ListValue save_page_requests;
   if (result == offline_pages::RequestQueue::GetRequestsResult::SUCCESS) {
     for (const auto& request : requests) {
       base::DictionaryValue* save_page_request = new base::DictionaryValue();
       save_page_requests.Append(save_page_request);
-      save_page_request->SetString("onlineUrl", request.url().spec());
+      save_page_request->SetString("onlineUrl", request->url().spec());
       save_page_request->SetDouble("creationTime",
-                                   request.creation_time().ToJsTime());
+                                   request->creation_time().ToJsTime());
       save_page_request->SetString("status", GetStringFromSavePageStatus());
-      save_page_request->SetString("namespace", request.client_id().name_space);
+      save_page_request->SetString("namespace",
+                                   request->client_id().name_space);
       save_page_request->SetDouble("lastAttempt",
-                                   request.last_attempt_time().ToJsTime());
-      save_page_request->SetString("id", std::to_string(request.request_id()));
+                                   request->last_attempt_time().ToJsTime());
+      save_page_request->SetString("id", std::to_string(request->request_id()));
     }
   }
   ResolveJavascriptCallback(base::StringValue(callback_id), save_page_requests);

chrome/browser/ui/webui/offline/offline_internals_ui_message_handler.h

@@ -63,7 +63,7 @@ class OfflineInternalsUIMessageHandler : public content::WebUIMessageHandler {
   void HandleRequestQueueCallback(
       std::string callback_id,
       offline_pages::RequestQueue::GetRequestsResult result,
-      const std::vector<offline_pages::SavePageRequest>& requests);
+      std::vector<std::unique_ptr<offline_pages::SavePageRequest>> requests);
 
   // Callback for DeletePage/ClearAll calls.
   void HandleDeletedPagesCallback(std::string callback_id,

components/offline_pages/background/request_coordinator.cc

@@ -131,8 +131,8 @@ void RequestCoordinator::GetAllRequests(const GetRequestsCallback& callback) {
 void RequestCoordinator::GetQueuedRequestsCallback(
     const GetRequestsCallback& callback,
     RequestQueue::GetRequestsResult result,
-    const std::vector<SavePageRequest>& requests) {
-  callback.Run(requests);
+    std::vector<std::unique_ptr<SavePageRequest>> requests) {
+  callback.Run(std::move(requests));
 }
 
 void RequestCoordinator::StopPrerendering() {
@@ -157,14 +157,14 @@ void RequestCoordinator::StopPrerendering() {
 
 void RequestCoordinator::GetRequestsForSchedulingCallback(
     RequestQueue::GetRequestsResult result,
-    const std::vector<SavePageRequest>& requests) {
+    std::vector<std::unique_ptr<SavePageRequest>> requests) {
   bool user_requested = false;
 
   // Examine all requests, if we find a user requested one, we will use the less
   // restrictive conditions for user_requested requests.  Otherwise we will use
   // the more restrictive non-user-requested conditions.
-  for (const SavePageRequest& request : requests) {
-    if (request.user_requested()) {
+  for (const auto& request : requests) {
+    if (request->user_requested()) {
       user_requested = true;
       break;
     }
@@ -285,18 +285,18 @@ void RequestCoordinator::UpdateRequestCallback(
 // Called in response to updating multiple requests in the request queue.
 void RequestCoordinator::UpdateMultipleRequestsCallback(
     const RequestQueue::UpdateMultipleRequestResults& results,
-    const std::vector<SavePageRequest>& requests) {
+    std::vector<std::unique_ptr<SavePageRequest>> requests) {
   bool available_user_request = false;
-  for (SavePageRequest request : requests) {
-    NotifyChanged(request);
-    if (!available_user_request && request.user_requested() &&
-        request.request_state() == SavePageRequest::RequestState::AVAILABLE) {
+  for (const auto& request : requests) {
+    NotifyChanged(*(request));
+    if (!available_user_request && request->user_requested() &&
+        request->request_state() == SavePageRequest::RequestState::AVAILABLE) {
       // TODO(dougarnett): Consider avoiding prospect of N^2 in case
       // size of bulk requests can get large (perhaps with easier to consume
       // callback interface).
       for (std::pair<int64_t, RequestQueue::UpdateRequestResult> pair :
            results) {
-        if (pair.first == request.request_id() &&
+        if (pair.first == request->request_id() &&
             pair.second == RequestQueue::UpdateRequestResult::SUCCESS) {
           // We have a successfully updated, available, user request.
           available_user_request = true;
@@ -313,17 +313,17 @@ void RequestCoordinator::HandleRemovedRequestsAndCallback(
     const RemoveRequestsCallback& callback,
     BackgroundSavePageResult status,
     const RequestQueue::UpdateMultipleRequestResults& results,
-    const std::vector<SavePageRequest>& requests) {
+    std::vector<std::unique_ptr<SavePageRequest>> requests) {
   callback.Run(results);
-  HandleRemovedRequests(status, results, requests);
+  HandleRemovedRequests(status, results, std::move(requests));
 }
 
 void RequestCoordinator::HandleRemovedRequests(
     BackgroundSavePageResult status,
     const RequestQueue::UpdateMultipleRequestResults& results,
-    const std::vector<SavePageRequest>& requests) {
-  for (SavePageRequest request : requests)
-    NotifyCompleted(request, status);
+    std::vector<std::unique_ptr<SavePageRequest>> requests) {
+  for (const auto& request : requests)
+    NotifyCompleted(*request, status);
 }
 
 void RequestCoordinator::ScheduleAsNeeded() {

components/offline_pages/background/request_coordinator.h

@@ -89,10 +89,8 @@ class RequestCoordinator : public KeyedService,
   // Resume a list of previously paused requests, making them available.
   void ResumeRequests(const std::vector<int64_t>& request_ids);
 
-  // Callback that receives the response for GetAllRequests.  Client must
-  // copy the result right away, it goes out of scope at the end of the
-  // callback.
-  typedef base::Callback<void(const std::vector<SavePageRequest>&)>
+  // Callback that receives the response for GetAllRequests.
+  typedef base::Callback<void(std::vector<std::unique_ptr<SavePageRequest>>)>
       GetRequestsCallback;
 
   // Get all save page request items in the callback.
@@ -167,15 +165,16 @@ class RequestCoordinator : public KeyedService,
  private:
   // Receives the results of a get from the request queue, and turns that into
   // SavePageRequest objects for the caller of GetQueuedRequests.
-  void GetQueuedRequestsCallback(const GetRequestsCallback& callback,
-                                 RequestQueue::GetRequestsResult result,
-                                 const std::vector<SavePageRequest>& requests);
+  void GetQueuedRequestsCallback(
+      const GetRequestsCallback& callback,
+      RequestQueue::GetRequestsResult result,
+      std::vector<std::unique_ptr<SavePageRequest>> requests);
 
   // Receives the results of a get from the request queue, and turns that into
   // SavePageRequest objects for the caller of GetQueuedRequests.
   void GetRequestsForSchedulingCallback(
       RequestQueue::GetRequestsResult result,
-      const std::vector<SavePageRequest>& requests);
+      std::vector<std::unique_ptr<SavePageRequest>> requests);
 
   // Receives the result of add requests to the request queue.
   void AddRequestResultCallback(RequestQueue::AddRequestResult result,
@@ -187,18 +186,18 @@ class RequestCoordinator : public KeyedService,
 
   void UpdateMultipleRequestsCallback(
       const RequestQueue::UpdateMultipleRequestResults& result,
-      const std::vector<SavePageRequest>& requests);
+      std::vector<std::unique_ptr<SavePageRequest>> requests);
 
   void HandleRemovedRequestsAndCallback(
       const RemoveRequestsCallback& callback,
       BackgroundSavePageResult status,
       const RequestQueue::UpdateMultipleRequestResults& results,
-      const std::vector<SavePageRequest>& requests);
+      std::vector<std::unique_ptr<SavePageRequest>> requests);
 
   void HandleRemovedRequests(
       BackgroundSavePageResult status,
       const RequestQueue::UpdateMultipleRequestResults& results,
-      const std::vector<SavePageRequest>& requests);
+      std::vector<std::unique_ptr<SavePageRequest>> requests);
 
   // Start processing now if connected (but with conservative assumption
   // as to other device conditions).

components/offline_pages/background/request_coordinator_unittest.cc

@@ -210,14 +210,15 @@ class RequestCoordinatorTest
 
   // Callback for getting requests.
   void GetRequestsDone(RequestQueue::GetRequestsResult result,
-                       const std::vector<SavePageRequest>& requests);
+                       std::vector<std::unique_ptr<SavePageRequest>> requests);
 
   // Callback for removing requests.
   void RemoveRequestsDone(
       const RequestQueue::UpdateMultipleRequestResults& results);
 
   // Callback for getting request statuses.
-  void GetQueuedRequestsDone(const std::vector<SavePageRequest>& requests);
+  void GetQueuedRequestsDone(
+      std::vector<std::unique_ptr<SavePageRequest>> requests);
 
   void SendOfflinerDoneCallback(const SavePageRequest& request,
                                 Offliner::RequestStatus status);
@@ -226,7 +227,7 @@ class RequestCoordinatorTest
     return last_get_requests_result_;
   }
 
-  const std::vector<SavePageRequest>& last_requests() const {
+  const std::vector<std::unique_ptr<SavePageRequest>>& last_requests() const {
     return last_requests_;
   }
 
@@ -276,8 +277,8 @@ class RequestCoordinatorTest
 
  private:
   RequestQueue::GetRequestsResult last_get_requests_result_;
-  std::vector<SavePageRequest> last_requests_;
   RequestQueue::UpdateMultipleRequestResults last_remove_results_;
+  std::vector<std::unique_ptr<SavePageRequest>> last_requests_;
   scoped_refptr<base::TestMockTimeTaskRunner> task_runner_;
   base::ThreadTaskRunnerHandle task_runner_handle_;
   std::unique_ptr<RequestCoordinator> coordinator_;
@@ -318,9 +319,9 @@ void RequestCoordinatorTest::PumpLoop() {
 
 void RequestCoordinatorTest::GetRequestsDone(
     RequestQueue::GetRequestsResult result,
-    const std::vector<SavePageRequest>& requests) {
+    std::vector<std::unique_ptr<SavePageRequest>> requests) {
   last_get_requests_result_ = result;
-  last_requests_ = requests;
+  last_requests_ = std::move(requests);
 }
 
 void RequestCoordinatorTest::RemoveRequestsDone(
@@ -330,8 +331,8 @@ void RequestCoordinatorTest::RemoveRequestsDone(
 }
 
 void RequestCoordinatorTest::GetQueuedRequestsDone(
-    const std::vector<SavePageRequest>& requests) {
-  last_requests_ = requests;
+    std::vector<std::unique_ptr<SavePageRequest>> requests) {
+  last_requests_ = std::move(requests);
   waiter_.Signal();
 }
 
@@ -394,15 +395,15 @@ TEST_F(RequestCoordinatorTest, SavePageLater) {
 
   // Check the request queue is as expected.
   EXPECT_EQ(1UL, last_requests().size());
-  EXPECT_EQ(kUrl1, last_requests()[0].url());
-  EXPECT_EQ(kClientId1, last_requests()[0].client_id());
+  EXPECT_EQ(kUrl1, last_requests().at(0)->url());
+  EXPECT_EQ(kClientId1, last_requests().at(0)->client_id());
 
   // Expect that the scheduler got notified.
   SchedulerStub* scheduler_stub = reinterpret_cast<SchedulerStub*>(
       coordinator()->scheduler());
   EXPECT_TRUE(scheduler_stub->schedule_called());
   EXPECT_EQ(coordinator()
-                ->GetTriggerConditions(last_requests()[0].user_requested())
+                ->GetTriggerConditions(last_requests()[0]->user_requested())
                 .minimum_battery_percentage,
             scheduler_stub->conditions()->minimum_battery_percentage);
 
@@ -550,8 +551,7 @@ TEST_F(RequestCoordinatorTest, OfflinerDoneForegroundCancel) {
   // Request no longer in the queue (for single attempt policy).
   EXPECT_EQ(1UL, last_requests().size());
   // Verify foreground cancel not counted as an attempt after all.
-  const SavePageRequest& found_request = last_requests().front();
-  EXPECT_EQ(0L, found_request.completed_attempt_count());
+  EXPECT_EQ(0L, last_requests().at(0)->completed_attempt_count());
 }
 
 TEST_F(RequestCoordinatorTest, OfflinerDonePrerenderingCancel) {
@@ -589,8 +589,9 @@ TEST_F(RequestCoordinatorTest, OfflinerDonePrerenderingCancel) {
   // Request still in the queue.
   EXPECT_EQ(1UL, last_requests().size());
   // Verify prerendering cancel not counted as an attempt after all.
-  const SavePageRequest& found_request = last_requests().front();
-  EXPECT_EQ(0L, found_request.completed_attempt_count());
+  const std::unique_ptr<SavePageRequest>& found_request =
+      last_requests().front();
+  EXPECT_EQ(0L, found_request->completed_attempt_count());
 }
 
 // If one item completes, and there are no more user requeted items left,
@@ -898,8 +899,8 @@ TEST_F(RequestCoordinatorTest, GetAllRequests) {
 
   // Check that the statuses found in the callback match what we expect.
   EXPECT_EQ(2UL, last_requests().size());
-  EXPECT_EQ(kRequestId1, last_requests().at(0).request_id());
-  EXPECT_EQ(kRequestId2, last_requests().at(1).request_id());
+  EXPECT_EQ(kRequestId1, last_requests().at(0)->request_id());
+  EXPECT_EQ(kRequestId2, last_requests().at(1)->request_id());
 }
 
 TEST_F(RequestCoordinatorTest, PauseAndResumeObserver) {

components/offline_pages/background/request_picker.cc

@@ -50,7 +50,7 @@ void RequestPicker::ChooseNextRequest(
 // request to operate on (if any).
 void RequestPicker::GetRequestResultCallback(
     RequestQueue::GetRequestsResult,
-    const std::vector<SavePageRequest>& requests) {
+    std::vector<std::unique_ptr<SavePageRequest>> requests) {
   // If there is nothing to do, return right away.
   if (requests.size() == 0) {
     not_picked_callback_.Run(false);
@@ -59,12 +59,12 @@ void RequestPicker::GetRequestResultCallback(
 
   // Get the expired requests to be removed from the queue, and the valid ones
   // from which to pick the next request.
-  std::vector<SavePageRequest> valid_requests;
-  std::vector<SavePageRequest> expired_requests;
-  SplitRequests(requests, valid_requests, expired_requests);
+  std::vector<std::unique_ptr<SavePageRequest>> valid_requests;
+  std::vector<std::unique_ptr<SavePageRequest>> expired_requests;
+  SplitRequests(std::move(requests), &valid_requests, &expired_requests);
   std::vector<int64_t> expired_request_ids;
-  for (auto request : expired_requests)
-    expired_request_ids.push_back(request.request_id());
+  for (const auto& request : expired_requests)
+    expired_request_ids.push_back(request->request_id());
 
   queue_->RemoveRequests(expired_request_ids,
                          base::Bind(&RequestPicker::OnRequestExpired,
@@ -84,12 +84,12 @@ void RequestPicker::GetRequestResultCallback(
   // Iterate once through the requests, keeping track of best candidate.
   bool non_user_requested_tasks_remaining = false;
   for (unsigned i = 0; i < valid_requests.size(); ++i) {
-    if (!valid_requests[i].user_requested())
+    if (!valid_requests[i]->user_requested())
       non_user_requested_tasks_remaining = true;
-    if (!RequestConditionsSatisfied(valid_requests[i]))
+    if (!RequestConditionsSatisfied(valid_requests[i].get()))
       continue;
-    if (IsNewRequestBetter(picked_request, &(valid_requests[i]), comparator))
-      picked_request = &(valid_requests[i]);
+    if (IsNewRequestBetter(picked_request, valid_requests[i].get(), comparator))
+      picked_request = valid_requests[i].get();
   }
 
   // If we have a best request to try next, get the request coodinator to
@@ -104,43 +104,43 @@ void RequestPicker::GetRequestResultCallback(
 // Filter out requests that don't meet the current conditions.  For instance, if
 // this is a predictive request, and we are not on WiFi, it should be ignored
 // this round.
-bool RequestPicker::RequestConditionsSatisfied(const SavePageRequest& request) {
+bool RequestPicker::RequestConditionsSatisfied(const SavePageRequest* request) {
   // If the user did not request the page directly, make sure we are connected
   // to power and have WiFi and sufficient battery remaining before we take this
   // request.
 
   if (!current_conditions_->IsPowerConnected() &&
-      policy_->PowerRequired(request.user_requested())) {
+      policy_->PowerRequired(request->user_requested())) {
     return false;
   }
 
   if (current_conditions_->GetNetConnectionType() !=
           net::NetworkChangeNotifier::ConnectionType::CONNECTION_WIFI &&
-      policy_->UnmeteredNetworkRequired(request.user_requested())) {
+      policy_->UnmeteredNetworkRequired(request->user_requested())) {
     return false;
   }
 
   if (current_conditions_->GetBatteryPercentage() <
-      policy_->BatteryPercentageRequired(request.user_requested())) {
+      policy_->BatteryPercentageRequired(request->user_requested())) {
     return false;
   }
 
   // If we have already started this page the max number of times, it is not
   // eligible to try again.
-  if (request.started_attempt_count() >= policy_->GetMaxStartedTries())
+  if (request->started_attempt_count() >= policy_->GetMaxStartedTries())
     return false;
 
   // If we have already completed trying this page the max number of times, it
   // is not eligible to try again.
-  if (request.completed_attempt_count() >= policy_->GetMaxCompletedTries())
+  if (request->completed_attempt_count() >= policy_->GetMaxCompletedTries())
     return false;
 
   // If the request is paused, do not consider it.
-  if (request.request_state() == SavePageRequest::RequestState::PAUSED)
+  if (request->request_state() == SavePageRequest::RequestState::PAUSED)
     return false;
 
   // If the request is expired, do not consider it.
-  base::TimeDelta requestAge = base::Time::Now() - request.creation_time();
+  base::TimeDelta requestAge = base::Time::Now() - request->creation_time();
   if (requestAge >
       base::TimeDelta::FromSeconds(
           policy_->GetRequestExpirationTimeInSeconds()))
@@ -150,7 +150,7 @@ bool RequestPicker::RequestConditionsSatisfied(const SavePageRequest& request) {
   // TODO(petewil): If the only reason we return nothing to do is that we have
   // inactive requests, we still want to try again later after their activation
   // time elapses, we shouldn't take ourselves completely off the scheduler.
-  if (request.activation_time() > base::Time::Now())
+  if (request->activation_time() > base::Time::Now())
     return false;
 
   return true;
@@ -236,17 +236,17 @@ int RequestPicker::CompareCreationTime(
   return result;
 }
 
-// Split all requests into expired ones and still valid ones.
 void RequestPicker::SplitRequests(
-    const std::vector<SavePageRequest>& requests,
-    std::vector<SavePageRequest>& valid_requests,
-    std::vector<SavePageRequest>& expired_requests) {
-  for (SavePageRequest request : requests) {
-    if (base::Time::Now() - request.creation_time() >=
+    std::vector<std::unique_ptr<SavePageRequest>> requests,
+    std::vector<std::unique_ptr<SavePageRequest>>* valid_requests,
+    std::vector<std::unique_ptr<SavePageRequest>>* expired_requests) {
+  std::vector<std::unique_ptr<SavePageRequest>>::iterator request;
+  for (auto& request : requests) {
+    if (base::Time::Now() - request->creation_time() >=
         base::TimeDelta::FromSeconds(kRequestExpirationTimeInSeconds)) {
-      expired_requests.push_back(request);
+      expired_requests->push_back(std::move(request));
     } else {
-      valid_requests.push_back(request);
+      valid_requests->push_back(std::move(request));
     }
   }
 }
@@ -255,10 +255,12 @@ void RequestPicker::SplitRequests(
 // the coordinator.
 void RequestPicker::OnRequestExpired(
     const RequestQueue::UpdateMultipleRequestResults& results,
-    const std::vector<SavePageRequest>& requests) {
-  for (auto request : requests)
+    const std::vector<std::unique_ptr<SavePageRequest>> requests) {
+  std::vector<std::unique_ptr<SavePageRequest>>::const_iterator request;
+  for (request = requests.begin(); request != requests.end(); ++request)
     notifier_->NotifyCompleted(
-        request, RequestCoordinator::BackgroundSavePageResult::EXPIRED);
+        *(request->get()),
+        RequestCoordinator::BackgroundSavePageResult::EXPIRED);
 }
 
 }  // namespace offline_pages

components/offline_pages/background/request_picker.h

@@ -37,13 +37,14 @@ class RequestPicker {
 
  private:
   // Callback for the GetRequest results to be delivered.
-  void GetRequestResultCallback(RequestQueue::GetRequestsResult result,
-                                const std::vector<SavePageRequest>& results);
+  void GetRequestResultCallback(
+      RequestQueue::GetRequestsResult result,
+      std::vector<std::unique_ptr<SavePageRequest>> results);
 
   // Filter out requests that don't meet the current conditions.  For instance,
   // if this is a predictive request, and we are not on WiFi, it should be
   // ignored this round.
-  bool RequestConditionsSatisfied(const SavePageRequest& request);
+  bool RequestConditionsSatisfied(const SavePageRequest* request);
 
   // Using policies, decide if the new request is preferable to the best we have
   // so far.
@@ -67,15 +68,17 @@ class RequestPicker {
   int CompareCreationTime(const SavePageRequest* left,
                           const SavePageRequest* right);
 
-  // Split all requests into expired ones and still valid ones.
-  void SplitRequests(const std::vector<SavePageRequest>& requests,
-                     std::vector<SavePageRequest>& valid_requests,
-                     std::vector<SavePageRequest>& expired_requests);
+  // Split all requests into expired ones and still valid ones.  Takes ownership
+  // of the requests, and moves them into either valid or expired requests.
+  void SplitRequests(
+      std::vector<std::unique_ptr<SavePageRequest>> requests,
+      std::vector<std::unique_ptr<SavePageRequest>>* valid_requests,
+      std::vector<std::unique_ptr<SavePageRequest>>* expired_requests);
 
   // Callback used after requests get expired.
   void OnRequestExpired(
       const RequestQueue::UpdateMultipleRequestResults& results,
-      const std::vector<SavePageRequest>& requests);
+      const std::vector<std::unique_ptr<SavePageRequest>> requests);
 
   // Unowned pointer to the request queue.
   RequestQueue* queue_;

components/offline_pages/background/request_queue.cc

@@ -16,7 +16,7 @@ namespace {
 // Completes the get requests call.
 void GetRequestsDone(const RequestQueue::GetRequestsCallback& callback,
                      bool success,
-                     const std::vector<SavePageRequest>& requests) {
+                     std::vector<std::unique_ptr<SavePageRequest>> requests) {
   RequestQueue::GetRequestsResult result =
       success ? RequestQueue::GetRequestsResult::SUCCESS
               : RequestQueue::GetRequestsResult::STORE_FAILURE;
@@ -24,7 +24,7 @@ void GetRequestsDone(const RequestQueue::GetRequestsCallback& callback,
   // This may trigger the purging if necessary.
   // Also this may be turned into a method on the request queue or add a policy
   // parameter in the process.
-  callback.Run(result, requests);
+  callback.Run(result, std::move(requests));
 }
 
 // Completes the add request call.
@@ -53,16 +53,16 @@ void UpdateRequestDone(const RequestQueue::UpdateRequestCallback& callback,
 void UpdateMultipleRequestsDone(
     const RequestQueue::UpdateMultipleRequestsCallback& callback,
     const RequestQueue::UpdateMultipleRequestResults& results,
-    const std::vector<SavePageRequest>& requests) {
-  callback.Run(results, requests);
+    std::vector<std::unique_ptr<SavePageRequest>> requests) {
+  callback.Run(results, std::move(requests));
 }
 
 // Completes the remove request call.
 void RemoveRequestsDone(
     const RequestQueue::RemoveRequestsCallback& callback,
     const RequestQueue::UpdateMultipleRequestResults& results,
-    const std::vector<SavePageRequest>& requests) {
-  callback.Run(results, requests);
+    std::vector<std::unique_ptr<SavePageRequest>> requests) {
+  callback.Run(results, std::move(requests));
 }
 
 }  // namespace
@@ -108,7 +108,7 @@ void RequestQueue::GetForUpdateDone(
     const UpdateRequestCallback& update_callback,
     const SavePageRequest& update_request,
     bool success,
-    const std::vector<SavePageRequest>& found_requests) {
+    std::vector<std::unique_ptr<SavePageRequest>> found_requests) {
   // If the result was not found, return now.
   if (!success) {
     update_callback.Run(
@@ -118,9 +118,9 @@ void RequestQueue::GetForUpdateDone(
   // If the found result does not contain the request we are looking for, return
   // now.
   bool found = false;
-  std::vector<SavePageRequest>::const_iterator iter;
+  std::vector<std::unique_ptr<SavePageRequest>>::const_iterator iter;
   for (iter = found_requests.begin(); iter != found_requests.end(); ++iter) {
-    if (iter->request_id() == update_request.request_id())
+    if ((*iter)->request_id() == update_request.request_id())
       found = true;
   }
   if (!found) {

components/offline_pages/background/request_queue.h

@@ -50,7 +50,7 @@ class RequestQueue {
 
   // Callback used for |GetRequests|.
   typedef base::Callback<void(GetRequestsResult,
-                              const std::vector<SavePageRequest>&)>
+                              std::vector<std::unique_ptr<SavePageRequest>>)>
       GetRequestsCallback;
 
   // Callback used for |AddRequest|.
@@ -61,13 +61,15 @@ class RequestQueue {
   typedef base::Callback<void(UpdateRequestResult)> UpdateRequestCallback;
 
   // Callback used by |ChangeState| for more than one update at a time.
-  typedef base::Callback<void(const UpdateMultipleRequestResults& results,
-                              const std::vector<SavePageRequest>& requests)>
+  typedef base::Callback<void(
+      const UpdateMultipleRequestResults& results,
+      std::vector<std::unique_ptr<SavePageRequest>> requests)>
       UpdateMultipleRequestsCallback;
 
   // Callback used by |RemoveRequests|.
-  typedef base::Callback<void(const UpdateMultipleRequestResults& results,
-                              const std::vector<SavePageRequest>& requests)>
+  typedef base::Callback<void(
+      const UpdateMultipleRequestResults& results,
+      std::vector<std::unique_ptr<SavePageRequest>> requests)>
       RemoveRequestsCallback;
 
   explicit RequestQueue(std::unique_ptr<RequestQueueStore> store);
@@ -108,7 +110,7 @@ class RequestQueue {
       const RequestQueue::UpdateRequestCallback& update_callback,
       const SavePageRequest& update_request,
       bool success,
-      const std::vector<SavePageRequest>& requests);
+      std::vector<std::unique_ptr<SavePageRequest>> requests);
 
  private:
   // Callback used by |PurgeRequests|.

components/offline_pages/background/request_queue_in_memory_store.cc

@@ -17,11 +17,15 @@ RequestQueueInMemoryStore::~RequestQueueInMemoryStore() {}
 
 void RequestQueueInMemoryStore::GetRequests(
     const GetRequestsCallback& callback) {
-  std::vector<SavePageRequest> result_requests;
-  for (const auto& id_request_pair : requests_)
-    result_requests.push_back(id_request_pair.second);
+  std::vector<std::unique_ptr<SavePageRequest>> result_requests;
+  for (const auto& id_request_pair : requests_) {
+    std::unique_ptr<SavePageRequest> request(
+        new SavePageRequest(id_request_pair.second));
+    result_requests.push_back(std::move(request));
+  }
   base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE, base::Bind(callback, true, result_requests));
+      FROM_HERE,
+      base::Bind(callback, true, base::Passed(std::move(result_requests))));
 }
 
 void RequestQueueInMemoryStore::AddOrUpdateRequest(
@@ -40,7 +44,7 @@ void RequestQueueInMemoryStore::RemoveRequests(
     const RemoveCallback& callback) {
   RequestQueue::UpdateMultipleRequestResults results;
   RequestQueue::UpdateRequestResult result;
-  std::vector<SavePageRequest> requests;
+  std::vector<std::unique_ptr<SavePageRequest>> requests;
   RequestsMap::iterator iter;
 
   // If we find a request, mark it as succeeded, and put it in the request list.
@@ -48,10 +52,11 @@ void RequestQueueInMemoryStore::RemoveRequests(
   for (auto request_id : request_ids) {
     iter = requests_.find(request_id);
     if (iter != requests_.end()) {
-      SavePageRequest request = iter->second;
+      std::unique_ptr<SavePageRequest> request(
+          new SavePageRequest(iter->second));
       requests_.erase(iter);
       result = RequestQueue::UpdateRequestResult::SUCCESS;
-      requests.push_back(request);
+      requests.push_back(std::move(request));
     } else {
       result = RequestQueue::UpdateRequestResult::REQUEST_DOES_NOT_EXIST;
     }
@@ -59,7 +64,8 @@ void RequestQueueInMemoryStore::RemoveRequests(
   }
 
   base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE, base::Bind(callback, results, requests));
+      FROM_HERE,
+      base::Bind(callback, results, base::Passed(std::move(requests))));
 }
 
 void RequestQueueInMemoryStore::ChangeRequestsState(
@@ -67,7 +73,7 @@ void RequestQueueInMemoryStore::ChangeRequestsState(
     const SavePageRequest::RequestState new_state,
     const UpdateMultipleRequestsCallback& callback) {
   RequestQueue::UpdateMultipleRequestResults results;
-  std::vector<SavePageRequest> requests;
+  std::vector<std::unique_ptr<SavePageRequest>> requests;
   RequestQueue::UpdateRequestResult result;
   for (int64_t request_id : request_ids) {
     auto pair = requests_.find(request_id);
@@ -75,7 +81,9 @@ void RequestQueueInMemoryStore::ChangeRequestsState(
     // the request list.
     if (pair != requests_.end()) {
       pair->second.set_request_state(new_state);
-      requests.push_back(pair->second);
+      std::unique_ptr<SavePageRequest> request(
+          new SavePageRequest(pair->second));
+      requests.push_back(std::move(request));
       result = RequestQueue::UpdateRequestResult::SUCCESS;
     } else {
       result = RequestQueue::UpdateRequestResult::REQUEST_DOES_NOT_EXIST;;
@@ -85,7 +93,8 @@ void RequestQueueInMemoryStore::ChangeRequestsState(
   }
 
   base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE, base::Bind(callback, results, requests));
+      FROM_HERE,
+      base::Bind(callback, results, base::Passed(std::move(requests))));
 }
 
 void RequestQueueInMemoryStore::Reset(const ResetCallback& callback) {

components/offline_pages/background/request_queue_store.h

@@ -26,16 +26,16 @@ class RequestQueueStore {
 
   typedef base::Callback<void(
       bool /* success */,
-      const std::vector<SavePageRequest>& /* requests */)>
+      std::vector<std::unique_ptr<SavePageRequest>> /* requests */)>
       GetRequestsCallback;
   typedef base::Callback<void(UpdateStatus)> UpdateCallback;
   typedef base::Callback<void(
       const RequestQueue::UpdateMultipleRequestResults& /* statuses*/,
-      const std::vector<SavePageRequest>& /* requests */)>
+      std::vector<std::unique_ptr<SavePageRequest>> /* requests */)>
       UpdateMultipleRequestsCallback;
   typedef base::Callback<void(
       const RequestQueue::UpdateMultipleRequestResults& /* statuses */,
-      const std::vector<SavePageRequest>& /* requests */)>
+      std::vector<std::unique_ptr<SavePageRequest>> /* requests */)>
       RemoveCallback;
   typedef base::Callback<void(bool /* success */)> ResetCallback;
 

components/offline_pages/background/request_queue_store_sql.cc

@@ -61,7 +61,8 @@ bool CreateSchema(sql::Connection* db) {
 // Create a save page request from a SQL result.  Expects complete rows with
 // all columns present.  Columns are in order they are defined in select query
 // in |RequestQueueStore::RequestSync| method.
-SavePageRequest MakeSavePageRequest(const sql::Statement& statement) {
+std::unique_ptr<SavePageRequest> MakeSavePageRequest(
+    const sql::Statement& statement) {
   const int64_t id = statement.ColumnInt64(0);
   const base::Time creation_time =
       base::Time::FromInternalValue(statement.ColumnInt64(1));
@@ -82,17 +83,18 @@ SavePageRequest MakeSavePageRequest(const sql::Statement& statement) {
            << " creation time " << creation_time << " user requested "
            << kUserRequested;
 
-  SavePageRequest request(id, url, client_id, creation_time, activation_time,
-                          kUserRequested);
-  request.set_last_attempt_time(last_attempt_time);
-  request.set_started_attempt_count(started_attempt_count);
-  request.set_completed_attempt_count(completed_attempt_count);
-  request.set_request_state(state);
+  std::unique_ptr<SavePageRequest> request(new SavePageRequest(
+      id, url, client_id, creation_time, activation_time, kUserRequested));
+  request->set_last_attempt_time(last_attempt_time);
+  request->set_started_attempt_count(started_attempt_count);
+  request->set_completed_attempt_count(completed_attempt_count);
+  request->set_request_state(state);
   return request;
 }
 
 // Get a request for a specific id.
-SavePageRequest GetOneRequest(sql::Connection* db, const int64_t request_id) {
+std::unique_ptr<SavePageRequest> GetOneRequest(sql::Connection* db,
+                                               const int64_t request_id) {
   const char kSql[] =
       "SELECT request_id, creation_time, activation_time,"
       " last_attempt_time, started_attempt_count, completed_attempt_count,"
@@ -109,9 +111,10 @@ SavePageRequest GetOneRequest(sql::Connection* db, const int64_t request_id) {
 void BuildFailedResultList(const std::vector<int64_t>& request_ids,
                            RequestQueue::UpdateMultipleRequestResults results) {
   results.clear();
-  for (int64_t request_id : request_ids)
+  for (int64_t request_id : request_ids) {
     results.push_back(std::make_pair(
         request_id, RequestQueue::UpdateRequestResult::STORE_FAILURE));
+  }
 }
 
 RequestQueue::UpdateRequestResult DeleteRequestById(sql::Connection* db,
@@ -140,10 +143,13 @@ bool ChangeRequestState(sql::Connection* db,
   return statement.Run();
 }
 
-bool DeleteRequestsByIds(sql::Connection* db,
-                         const std::vector<int64_t>& request_ids,
-                         RequestQueue::UpdateMultipleRequestResults& results,
-                         std::vector<SavePageRequest>& requests) {
+// Helper function to delete requests corresponding to passed in requestIds,
+// and fill an outparam with the removed requests.
+bool DeleteRequestsByIds(
+    sql::Connection* db,
+    const std::vector<int64_t>& request_ids,
+    RequestQueue::UpdateMultipleRequestResults& results,
+    std::vector<std::unique_ptr<SavePageRequest>>* requests) {
   // If you create a transaction but don't Commit() it is automatically
   // rolled back by its destructor when it falls out of scope.
   sql::Transaction transaction(db);
@@ -155,16 +161,16 @@ bool DeleteRequestsByIds(sql::Connection* db,
   // Read the request before we delete it, and if the delete worked, put it on
   // the queue of requests that got deleted.
   for (int64_t request_id : request_ids) {
-    SavePageRequest request = GetOneRequest(db, request_id);
+    std::unique_ptr<SavePageRequest> request = GetOneRequest(db, request_id);
     RequestQueue::UpdateRequestResult result =
         DeleteRequestById(db, request_id);
     results.push_back(std::make_pair(request_id, result));
     if (result == RequestQueue::UpdateRequestResult::SUCCESS)
-      requests.push_back(request);
+      requests->push_back(std::move(request));
   }
 
   if (!transaction.Commit()) {
-    requests.clear();
+    requests->clear();
     BuildFailedResultList(request_ids, results);
     return false;
   }
@@ -172,11 +178,12 @@ bool DeleteRequestsByIds(sql::Connection* db,
   return true;
 }
 
-bool ChangeRequestsState(sql::Connection* db,
-                         const std::vector<int64_t>& request_ids,
-                         SavePageRequest::RequestState new_state,
-                         RequestQueue::UpdateMultipleRequestResults& results,
-                         std::vector<SavePageRequest>& requests) {
+bool ChangeRequestsState(
+    sql::Connection* db,
+    const std::vector<int64_t>& request_ids,
+    SavePageRequest::RequestState new_state,
+    RequestQueue::UpdateMultipleRequestResults& results,
+    std::vector<std::unique_ptr<SavePageRequest>>& requests) {
   // If you create a transaction but don't Commit() it is automatically
   // rolled back by its destructor when it falls out of scope.
   sql::Transaction transaction(db);
@@ -294,12 +301,12 @@ void RequestQueueStoreSQL::GetRequestsSync(
 
   sql::Statement statement(db->GetCachedStatement(SQL_FROM_HERE, kSql));
 
-  std::vector<SavePageRequest> result;
+  std::vector<std::unique_ptr<SavePageRequest>> requests;
   while (statement.Step())
-    result.push_back(MakeSavePageRequest(statement));
+    requests.push_back(MakeSavePageRequest(statement));
 
-  runner->PostTask(FROM_HERE,
-                   base::Bind(callback, statement.Succeeded(), result));
+  runner->PostTask(FROM_HERE, base::Bind(callback, statement.Succeeded(),
+                                         base::Passed(&requests)));
 }
 
 // static
@@ -320,10 +327,11 @@ void RequestQueueStoreSQL::RemoveRequestsSync(
     const std::vector<int64_t>& request_ids,
     const RemoveCallback& callback) {
   RequestQueue::UpdateMultipleRequestResults results;
-  std::vector<SavePageRequest> requests;
+  std::vector<std::unique_ptr<SavePageRequest>> requests;
   // TODO(fgorski): add UMA metrics here.
-  DeleteRequestsByIds(db, request_ids, results, requests);
-  runner->PostTask(FROM_HERE, base::Bind(callback, results, requests));
+  DeleteRequestsByIds(db, request_ids, results, &requests);
+  runner->PostTask(FROM_HERE,
+                   base::Bind(callback, results, base::Passed(&requests)));
 }
 
 // static
@@ -334,11 +342,12 @@ void RequestQueueStoreSQL::ChangeRequestsStateSync(
     const SavePageRequest::RequestState new_state,
     const UpdateMultipleRequestsCallback& callback) {
   RequestQueue::UpdateMultipleRequestResults results;
-  std::vector<SavePageRequest> requests;
+  std::vector<std::unique_ptr<SavePageRequest>> requests;
   // TODO(fgorski): add UMA metrics here.
   offline_pages::ChangeRequestsState(db, request_ids, new_state, results,
                                      requests);
-  runner->PostTask(FROM_HERE, base::Bind(callback, results, requests));
+  runner->PostTask(FROM_HERE,
+                   base::Bind(callback, results, base::Passed(&requests)));
 }
 
 // static
@@ -369,7 +378,8 @@ bool RequestQueueStoreSQL::CheckDb(const base::Closure& callback) {
 
 void RequestQueueStoreSQL::GetRequests(const GetRequestsCallback& callback) {
   DCHECK(db_.get());
-  if (!CheckDb(base::Bind(callback, false, std::vector<SavePageRequest>())))
+  std::vector<std::unique_ptr<SavePageRequest>> requests;
+  if (!CheckDb(base::Bind(callback, false, base::Passed(&requests))))
     return;
 
   background_task_runner_->PostTask(
@@ -395,13 +405,16 @@ void RequestQueueStoreSQL::RemoveRequests(
     const RemoveCallback& callback) {
   // Set up a failed set of results in case we fail the DB check.
   RequestQueue::UpdateMultipleRequestResults results;
-  std::vector<SavePageRequest> requests;
-  for (int64_t request_id : request_ids)
+  for (int64_t request_id : request_ids) {
     results.push_back(std::make_pair(
         request_id, RequestQueue::UpdateRequestResult::STORE_FAILURE));
+  }
 
-  if (!CheckDb(base::Bind(callback, results, requests)))
+  if (!CheckDb(base::Bind(
+          callback, results,
+          base::Passed(std::vector<std::unique_ptr<SavePageRequest>>())))) {
     return;
+  }
 
   background_task_runner_->PostTask(
       FROM_HERE,
@@ -414,9 +427,10 @@ void RequestQueueStoreSQL::ChangeRequestsState(
     const SavePageRequest::RequestState new_state,
     const UpdateMultipleRequestsCallback& callback) {
   RequestQueue::UpdateMultipleRequestResults results;
-  std::vector<SavePageRequest> requests;
-  if (!CheckDb(base::Bind(callback, results, requests)))
+  std::vector<std::unique_ptr<SavePageRequest>> requests;
+  if (!CheckDb(base::Bind(callback, results, base::Passed(&requests)))) {
     return;
+  }
 
   background_task_runner_->PostTask(
       FROM_HERE, base::Bind(&RequestQueueStoreSQL::ChangeRequestsStateSync,

components/offline_pages/background/request_queue_store_unittest.cc

@@ -64,15 +64,15 @@ class RequestQueueStoreTestBase : public testing::Test {
 
   // Callback used for get requests.
   void GetRequestsDone(bool result,
-                       const std::vector<SavePageRequest>& requests);
+                       std::vector<std::unique_ptr<SavePageRequest>> requests);
   // Callback used for add/update request.
   void AddOrUpdateDone(UpdateStatus result);
   void UpdateMultipleRequestsDone(
       const RequestQueue::UpdateMultipleRequestResults& results,
-      const std::vector<SavePageRequest>& requests);
+      std::vector<std::unique_ptr<SavePageRequest>> requests);
   // Callback used for remove requests.
   void RemoveDone(const RequestQueue::UpdateMultipleRequestResults& results,
-                  const std::vector<SavePageRequest>& requests);
+                  std::vector<std::unique_ptr<SavePageRequest>> requests);
   // Callback used for reset.
   void ResetDone(bool result);
 
@@ -86,7 +86,7 @@ class RequestQueueStoreTestBase : public testing::Test {
       const {
     return last_remove_results_;
   }
-  const std::vector<SavePageRequest>& last_requests() const {
+  const std::vector<std::unique_ptr<SavePageRequest>>& last_requests() const {
     return last_requests_;
   }
 
@@ -98,7 +98,7 @@ class RequestQueueStoreTestBase : public testing::Test {
   UpdateStatus last_update_status_;
   RequestQueue::UpdateMultipleRequestResults last_multiple_update_results_;
   RequestQueue::UpdateMultipleRequestResults last_remove_results_;
-  std::vector<SavePageRequest> last_requests_;
+  std::vector<std::unique_ptr<SavePageRequest>> last_requests_;
 
   scoped_refptr<base::TestSimpleTaskRunner> task_runner_;
   base::ThreadTaskRunnerHandle task_runner_handle_;
@@ -130,9 +130,9 @@ void RequestQueueStoreTestBase::ClearResults() {
 
 void RequestQueueStoreTestBase::GetRequestsDone(
     bool result,
-    const std::vector<SavePageRequest>& requests) {
+    std::vector<std::unique_ptr<SavePageRequest>> requests) {
   last_result_ = result ? LastResult::kTrue : LastResult::kFalse;
-  last_requests_ = requests;
+  last_requests_ = std::move(requests);
 }
 
 void RequestQueueStoreTestBase::AddOrUpdateDone(UpdateStatus status) {
@@ -141,16 +141,16 @@ void RequestQueueStoreTestBase::AddOrUpdateDone(UpdateStatus status) {
 
 void RequestQueueStoreTestBase::UpdateMultipleRequestsDone(
     const RequestQueue::UpdateMultipleRequestResults& results,
-    const std::vector<SavePageRequest>& requests) {
+    std::vector<std::unique_ptr<SavePageRequest>> requests) {
   last_multiple_update_results_ = results;
-  last_requests_ = requests;
+  last_requests_ = std::move(requests);
 }
 
 void RequestQueueStoreTestBase::RemoveDone(
     const RequestQueue::UpdateMultipleRequestResults& results,
-    const std::vector<SavePageRequest>& requests) {
+    std::vector<std::unique_ptr<SavePageRequest>> requests) {
   last_remove_results_ = results;
-  last_requests_ = requests;
+  last_requests_ = std::move(requests);
 }
 
 void RequestQueueStoreTestBase::ResetDone(bool result) {
@@ -240,7 +240,7 @@ TYPED_TEST(RequestQueueStoreTest, AddRequest) {
   this->PumpLoop();
   ASSERT_EQ(LastResult::kTrue, this->last_result());
   ASSERT_EQ(1ul, this->last_requests().size());
-  ASSERT_TRUE(request == this->last_requests()[0]);
+  ASSERT_TRUE(request == *(this->last_requests()[0]));
 }
 
 TYPED_TEST(RequestQueueStoreTest, UpdateRequest) {
@@ -275,7 +275,7 @@ TYPED_TEST(RequestQueueStoreTest, UpdateRequest) {
   this->PumpLoop();
   ASSERT_EQ(LastResult::kTrue, this->last_result());
   ASSERT_EQ(1ul, this->last_requests().size());
-  ASSERT_TRUE(updated_request == this->last_requests()[0]);
+  ASSERT_TRUE(updated_request == *(this->last_requests()[0].get()));
 }
 
 TYPED_TEST(RequestQueueStoreTest, RemoveRequests) {
@@ -306,7 +306,7 @@ TYPED_TEST(RequestQueueStoreTest, RemoveRequests) {
   ASSERT_EQ(RequestQueue::UpdateRequestResult::SUCCESS,
             this->last_remove_results().at(1).second);
   ASSERT_EQ(2UL, this->last_requests().size());
-  ASSERT_EQ(kRequestId, this->last_requests().at(0).request_id());
+  ASSERT_EQ(kRequestId, this->last_requests().at(0)->request_id());
   this->ClearResults();
 
   store->GetRequests(base::Bind(&RequestQueueStoreTestBase::GetRequestsDone,
@@ -358,7 +358,7 @@ TYPED_TEST(RequestQueueStoreTest, PauseAndResumeRequest) {
   ASSERT_EQ(1ul, this->last_multiple_update_results().size());
   ASSERT_EQ(RequestQueue::UpdateRequestResult::SUCCESS,
             this->last_multiple_update_results().at(0).second);
-  ASSERT_EQ(kRequestId, this->last_requests().at(0).request_id());
+  ASSERT_EQ(kRequestId, this->last_requests().at(0)->request_id());
   this->ClearResults();
 
   // Get the request from the queue to check it out
@@ -370,7 +370,7 @@ TYPED_TEST(RequestQueueStoreTest, PauseAndResumeRequest) {
   ASSERT_EQ(1UL, this->last_requests().size());
   // Request 1 should be paused.
   ASSERT_EQ(SavePageRequest::RequestState::PAUSED,
-            this->last_requests().at(0).request_state());
+            this->last_requests().at(0)->request_state());
   this->ClearResults();
 
   // Now resume the same request we paused.
@@ -385,7 +385,7 @@ TYPED_TEST(RequestQueueStoreTest, PauseAndResumeRequest) {
   ASSERT_EQ(1ul, this->last_multiple_update_results().size());
   ASSERT_EQ(RequestQueue::UpdateRequestResult::SUCCESS,
             this->last_multiple_update_results().at(0).second);
-  ASSERT_EQ(kRequestId, this->last_requests().at(0).request_id());
+  ASSERT_EQ(kRequestId, this->last_requests().at(0)->request_id());
   this->ClearResults();
 
   // Get the request from the queue to check it out
@@ -397,7 +397,7 @@ TYPED_TEST(RequestQueueStoreTest, PauseAndResumeRequest) {
   ASSERT_EQ(1UL, this->last_requests().size());
   // Request 1 should be paused.
   ASSERT_EQ(SavePageRequest::RequestState::AVAILABLE,
-            this->last_requests().at(0).request_state());
+            this->last_requests().at(0)->request_state());
   this->ClearResults();
 }
 
@@ -452,7 +452,7 @@ TEST_F(RequestQueueStoreSQLTest, SaveCloseReopenRead) {
   this->PumpLoop();
   ASSERT_EQ(LastResult::kTrue, this->last_result());
   ASSERT_EQ(1ul, this->last_requests().size());
-  ASSERT_TRUE(original_request == this->last_requests()[0]);
+  ASSERT_TRUE(original_request == *(this->last_requests().at(0).get()));
 }
 
 }  // offline_pages

components/offline_pages/background/request_queue_unittest.cc

@@ -47,15 +47,15 @@ class RequestQueueTest : public testing::Test {
   void AddRequestDone(AddRequestResult result, const SavePageRequest& request);
   // Callback for getting requests.
   void GetRequestsDone(GetRequestsResult result,
-                       const std::vector<SavePageRequest>& requests);
+                       std::vector<std::unique_ptr<SavePageRequest>> requests);
   // Callback for removing request.
   void RemoveRequestsDone(
       const RequestQueue::UpdateMultipleRequestResults& results,
-      const std::vector<SavePageRequest>& requests);
+      std::vector<std::unique_ptr<SavePageRequest>> requests);
 
   void UpdateMultipleRequestsDone(
       const RequestQueue::UpdateMultipleRequestResults& results,
-      const std::vector<SavePageRequest>& requests);
+      std::vector<std::unique_ptr<SavePageRequest>> requests);
 
   void UpdateRequestDone(UpdateRequestResult result);
 
@@ -81,7 +81,7 @@ class RequestQueueTest : public testing::Test {
   GetRequestsResult last_get_requests_result() const {
     return last_get_requests_result_;
   }
-  const std::vector<SavePageRequest>& last_requests() const {
+  const std::vector<std::unique_ptr<SavePageRequest>>& last_requests() const {
     return last_requests_;
   }
 
@@ -93,7 +93,7 @@ class RequestQueueTest : public testing::Test {
   UpdateRequestResult last_update_result_;
 
   GetRequestsResult last_get_requests_result_;
-  std::vector<SavePageRequest> last_requests_;
+  std::vector<std::unique_ptr<SavePageRequest>> last_requests_;
 
   std::unique_ptr<RequestQueue> queue_;
   scoped_refptr<base::TestSimpleTaskRunner> task_runner_;
@@ -127,23 +127,23 @@ void RequestQueueTest::AddRequestDone(AddRequestResult result,
 
 void RequestQueueTest::GetRequestsDone(
     GetRequestsResult result,
-    const std::vector<SavePageRequest>& requests) {
+    std::vector<std::unique_ptr<SavePageRequest>> requests) {
   last_get_requests_result_ = result;
-  last_requests_ = requests;
+  last_requests_ = std::move(requests);
 }
 
 void RequestQueueTest::RemoveRequestsDone(
     const RequestQueue::UpdateMultipleRequestResults& results,
-    const std::vector<SavePageRequest>& requests) {
+    std::vector<std::unique_ptr<SavePageRequest>> requests) {
   last_remove_results_ = results;
-  last_requests_ = requests;
+  last_requests_ = std::move(requests);
 }
 
 void RequestQueueTest::UpdateMultipleRequestsDone(
     const RequestQueue::UpdateMultipleRequestResults& results,
-    const std::vector<SavePageRequest>& requests) {
+    std::vector<std::unique_ptr<SavePageRequest>> requests) {
   last_multiple_update_results_ = results;
-  last_requests_ = requests;
+  last_requests_ = std::move(requests);
 }
 
 void RequestQueueTest::UpdateRequestDone(UpdateRequestResult result) {
@@ -275,7 +275,7 @@ TEST_F(RequestQueueTest, PauseAndResume) {
   ASSERT_EQ(GetRequestsResult::SUCCESS, last_get_requests_result());
   ASSERT_EQ(1ul, last_requests().size());
   ASSERT_EQ(SavePageRequest::RequestState::PAUSED,
-            last_requests().front().request_state());
+            last_requests().at(0)->request_state());
 
   // Resume the request.
   queue()->ChangeRequestsState(
@@ -295,7 +295,7 @@ TEST_F(RequestQueueTest, PauseAndResume) {
   ASSERT_EQ(GetRequestsResult::SUCCESS, last_get_requests_result());
   ASSERT_EQ(1ul, last_requests().size());
   ASSERT_EQ(SavePageRequest::RequestState::AVAILABLE,
-            last_requests().front().request_state());
+            last_requests().at(0)->request_state());
 }
 
 // A longer test populating the request queue with more than one item, properly
@@ -336,7 +336,7 @@ TEST_F(RequestQueueTest, MultipleRequestsAddGetRemove) {
   PumpLoop();
   ASSERT_EQ(GetRequestsResult::SUCCESS, last_get_requests_result());
   ASSERT_EQ(1ul, last_requests().size());
-  ASSERT_EQ(request2.request_id(), last_requests()[0].request_id());
+  ASSERT_EQ(request2.request_id(), last_requests().at(0)->request_id());
 }
 
 TEST_F(RequestQueueTest, UpdateRequest) {
@@ -362,7 +362,7 @@ TEST_F(RequestQueueTest, UpdateRequest) {
   PumpLoop();
   ASSERT_EQ(GetRequestsResult::SUCCESS, last_get_requests_result());
   ASSERT_EQ(1ul, last_requests().size());
-  ASSERT_EQ(kRetryCount, last_requests().front().completed_attempt_count());
+  ASSERT_EQ(kRetryCount, last_requests().at(0)->completed_attempt_count());
 }
 
 TEST_F(RequestQueueTest, UpdateRequestNotPresent) {