[Offline Pages] Remove expired requests from the queue.

components/offline_pages/background/request_picker.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_picker.h"
 
 #include "base/bind.h"
 #include "base/logging.h"
 #include "components/offline_pages/background/save_page_request.h"
 
 namespace {
 template <typename T>
 int signum(T t) {
   return (T(0) < t) - (t < T(0));
 }
 
 #define CALL_MEMBER_FUNCTION(object,ptrToMember)  ((object)->*(ptrToMember))
 }  // namespace
 
 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) {}
 
 RequestPicker::~RequestPicker() {}
 
 // Entry point for the async operation to choose the next request.
 void RequestPicker::ChooseNextRequest(
     RequestCoordinator::RequestPickedCallback picked_callback,
     RequestCoordinator::RequestQueueEmptyCallback empty_callback,
@@ skipping 12 matching lines @@
 // request to operate on (if any).
 void RequestPicker::GetRequestResultCallback(
     RequestQueue::GetRequestsResult,
     const std::vector<SavePageRequest>& requests) {
   // If there is nothing to do, return right away.
   if (requests.size() == 0) {
     empty_callback_.Run();
     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;
 
   RequestCompareFunction comparator = nullptr;
 
   // Choose which comparison function to use based on policy.
   if (policy_->RetryCountIsMoreImportantThanRecency())
     comparator = &RequestPicker::RetryCountFirstCompareFunction;
   else
     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
   // start it.  Otherwise return that we have no candidates.
   if (picked_request != nullptr) {
     picked_callback_.Run(*picked_request);
   } else {
     empty_callback_.Run();
   }
 }
@@ skipping 127 matching lines @@
   base::TimeDelta difference = left->creation_time() - right->creation_time();
   int result = signum(difference.InMilliseconds());
 
   // Flip the direction of comparison if policy prefers fewer retries.
   if (earlier_requests_better_)
     result *= -1;
 
   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