Use a vector of smart pointers for callback return type.

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 <utility>
 
 #include "base/bind.h"
 #include "base/callback.h"
@@ skipping 112 matching lines @@
   // Get all matching requests from the request queue, send them to our
   // callback.  We bind the namespace and callback to the front of the callback
   // param set.
   queue_->GetRequests(base::Bind(&RequestCoordinator::GetQueuedRequestsCallback,
                                  weak_ptr_factory_.GetWeakPtr(), 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() {
   if (offliner_ && is_busy_) {
     DCHECK(active_request_.get());
     offliner_->Cancel();
     AbortRequestAttempt(active_request_.get());
   }
 
   // Stopping offliner means it will not call callback.
   last_offlining_status_ =
       Offliner::RequestStatus::REQUEST_COORDINATOR_CANCELED;
 
   if (active_request_) {
     RecordOfflinerResultUMA(active_request_->client_id(),
                             last_offlining_status_);
     is_busy_ = false;
     active_request_.reset();
   }
 
 }
 
 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;
     }
   }
 
   // In the get callback, determine the least restrictive, and call
   // GetTriggerConditions based on that.
   scheduler_->Schedule(GetTriggerConditions(user_requested));
 }
 
@@ skipping 100 matching lines @@
   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);
   }
 }
 
 // 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;
         }
       }
     }
   }
 
   if (available_user_request)
     StartProcessingIfConnected();
 }
 
 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) {
+  std::vector<std::unique_ptr<SavePageRequest>>::iterator request;
+  for (request = requests.begin(); request != requests.end(); ++request)

[Bernhard Bauer, 2016/09/07 10:13:41]

Why this style of loop?

[Pete Williamson, 2016/09/07 20:21:01]

I was having problems getting the type right for a for_each style loop. Fixed
now.

+    NotifyCompleted(*(request->get()), status);
 }
 
 void RequestCoordinator::ScheduleAsNeeded() {
   // Get all requests from queue (there is no filtering mechanism).
   queue_->GetRequests(
       base::Bind(&RequestCoordinator::GetRequestsForSchedulingCallback,
                  weak_ptr_factory_.GetWeakPtr()));
 }
 
 void RequestCoordinator::StopProcessing() {
@@ skipping 226 matching lines @@
   FOR_EACH_OBSERVER(Observer, observers_, OnChanged(request));
 }
 
 void RequestCoordinator::GetOffliner() {
   if (!offliner_) {
     offliner_ = factory_->GetOffliner(policy_.get());
   }
 }
 
 }  // namespace offline_pages