Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(347)

Unified Diff: components/offline_pages/background/request_picker.cc

Issue 2262423002: Use a vector of smart pointers for callback return type. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Logical merge fixes Created 4 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: components/offline_pages/background/request_picker.cc
diff --git a/components/offline_pages/background/request_picker.cc b/components/offline_pages/background/request_picker.cc
index c8e597b772db75e2e751828db01150743ccaf450..81ab6b597d59533514ab291ee5d760064c3a3557 100644
--- a/components/offline_pages/background/request_picker.cc
+++ b/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) {
empty_callback_.Run();
@@ -59,12 +59,14 @@ 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;
+ std::vector<std::unique_ptr<SavePageRequest>> valid_requests;
+ std::vector<std::unique_ptr<SavePageRequest>> expired_requests;
SplitRequests(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());
+ std::vector<std::unique_ptr<SavePageRequest>>::iterator request;
+ for (request = expired_requests.begin(); request != expired_requests.end();
+ ++request)
+ expired_request_ids.push_back(request->get()->request_id());
queue_->RemoveRequests(expired_request_ids,
base::Bind(&RequestPicker::OnRequestExpired,
@@ -83,10 +85,10 @@ void RequestPicker::GetRequestResultCallback(
// Iterate once through the requests, keeping track of best candidate.
for (unsigned i = 0; i < valid_requests.size(); ++i) {
- 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
@@ -101,13 +103,13 @@ 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.
// TODO(petewil): We may later want to configure whether to allow 2G for non
// user_requested items, add that to policy.
- if (!request.user_requested()) {
+ if (!request->user_requested()) {
if (!current_conditions_->IsPowerConnected())
return false;
@@ -126,23 +128,23 @@ bool RequestPicker::RequestConditionsSatisfied(const SavePageRequest& request) {
// eligible to try again.
// TODO(petewil): We should have code to remove the page from the
// queue after the last retry.
- 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.
// TODO(petewil): We should have code to remove the page from the
// queue after the last retry.
- 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.
// TODO(petewil): We need to remove this from the queue.
- 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()))
@@ -152,7 +154,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;
@@ -238,17 +240,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 (request = requests.begin(); request != requests.end(); ++request) {
+ if (base::Time::Now() - request->get()->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));
}
}
}
@@ -257,9 +259,10 @@ void RequestPicker::SplitRequests(
// the coordinator.
void RequestPicker::OnRequestExpired(
const RequestQueue::UpdateMultipleRequestResults& results,
- const std::vector<SavePageRequest>& requests) {
- for (auto request : requests)
- notifier_->NotifyCompleted(request,
+ 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->get()),
RequestCoordinator::SavePageStatus::EXPIRED);
}

Powered by Google App Engine
This is Rietveld 408576698