[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 47bff566b5f7b6764e6d08b460c21bd1f16eaba2..adba2844d5bf2504a4f17710ea445d7f415168f4 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,20 @@ 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_->RemoveRequestsById(
+      expired_request_ids,
+      base::Bind(&RequestPicker::OnRequestExpired,
+                 weak_ptr_factory_.GetWeakPtr(), expired_requests));
+
   // Pick the most deserving request for our conditions.
   const SavePageRequest* picked_request = nullptr;
 
@@ -67,11 +83,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
@@ -219,4 +235,27 @@ 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 std::vector<SavePageRequest>& expired_requests,
+    RequestQueue::UpdateRequestResult result) {
+  coordinator_->NotifyRequestsExpired(expired_requests, result);

[Pete Williamson, 2016/08/10 17:15:11]

When the observer changelist lands, this should be changed to call NotifyFailed
for each request.

[romax, 2016/08/16 23:20:56]

Done.

+}
+
 }  // namespace offline_pages