[Offline Pages] Remove expired requests from the queue.

components/offline_pages/background/request_picker.cc

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 84b73056f9d88eec7a65fd207fb7c113f22b83da..39a266e401410c992c2128aeca1d86ce3bc7bd51 100644
--- a/components/offline_pages/background/request_picker.cc
+++ b/components/offline_pages/background/request_picker.cc
@@ -19,10 +19,12 @@ int signum(T t) {
 
 namespace offline_pages {
 
-RequestPicker::RequestPicker(
-    RequestQueue* requestQueue, OfflinerPolicy* policy)
+RequestPicker::RequestPicker(RequestQueue* requestQueue,
+                             OfflinerPolicy* policy,
+                             RequestCoordinator* coordinator)
     : queue_(requestQueue),
       policy_(policy),
+      coordinator_(coordinator),
       fewer_retries_better_(false),
       earlier_requests_better_(false),
       weak_ptr_factory_(this) {}
@@ -55,6 +57,19 @@ void RequestPicker::GetRequestResultCallback(
     return;
   }
 
+  // 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<int64_t> expired_request_ids;
+  for (auto request : expired_requests)
+    expired_request_ids.push_back(request.request_id());
+
+  queue_->RemoveRequests(expired_request_ids,
+                         base::Bind(&RequestPicker::OnRequestExpired,
+                                    weak_ptr_factory_.GetWeakPtr()));
+
   // Pick the most deserving request for our conditions.
   const SavePageRequest* picked_request = nullptr;
 
@@ -67,11 +82,11 @@ void RequestPicker::GetRequestResultCallback(
     comparator = &RequestPicker::RecencyFirstCompareFunction;
 
   // Iterate once through the requests, keeping track of best candidate.
-  for (unsigned i = 0; i < requests.size(); ++i) {
-    if (!RequestConditionsSatisfied(requests[i]))
+  for (unsigned i = 0; i < valid_requests.size(); ++i) {
+    if (!RequestConditionsSatisfied(valid_requests[i]))
       continue;
-    if (IsNewRequestBetter(picked_request, &(requests[i]), comparator))
-      picked_request = &(requests[i]);
+    if (IsNewRequestBetter(picked_request, &(valid_requests[i]), comparator))
+      picked_request = &(valid_requests[i]);
   }
 
   // If we have a best request to try next, get the request coodinator to
@@ -223,4 +238,29 @@ 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() >=
+        base::TimeDelta::FromSeconds(kRequestExpirationTimeInSeconds)) {
+      expired_requests.push_back(request);
+    } else {
+      valid_requests.push_back(request);
+    }
+  }
+}
+
+// Callback used after expired requests are deleted from the queue and notifies
+// the coordinator.
+void RequestPicker::OnRequestExpired(
+    const RequestQueue::UpdateMultipleRequestResults& results,
+    const std::vector<SavePageRequest>& requests) {
+  for (auto request : requests)
+    coordinator_->NotifyFailed(request,

[Pete Williamson, 2016/08/16 23:40:46]

This will change to coordinator_->NotifiyCompleted() when my patch in the CQ
lands...

[romax, 2016/08/17 20:07:59]

Done.

+                               RequestCoordinator::SavePageStatus::EXPIRED);
+}
+
 }  // namespace offline_pages