API to provide status of save page reqeusts to API.

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 51 matching lines @@
   // Build a SavePageRequest.
   offline_pages::SavePageRequest request(
       id++, url, client_id, base::Time::Now(), was_user_requested);
 
   // Put the request on the request queue.
   queue_->AddRequest(request,
                      base::Bind(&RequestCoordinator::AddRequestResultCallback,
                                 weak_ptr_factory_.GetWeakPtr()));
   return true;
 }
+void RequestCoordinator::GetRequestStatuses(
+    std::string client_namespace, RequestStatusCallback callback) {
+  // Get all 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::GetRequestStatusesCallback,
+      weak_ptr_factory_.GetWeakPtr(),
+      client_namespace, callback));
+}
+
+// For each request matching the client_namespace, put the ClientID and
+// status into a RequestStatus object.
+void RequestCoordinator::GetRequestStatusesCallback(
+    std::string client_namespace,
+    RequestStatusCallback callback,
+    RequestQueue::GetRequestsResult result,
+    const std::vector<SavePageRequest>& requests) {
+  std::vector<RequestStatus> found_requests;
+
+  // TODO(petewil): Is find_if better?
+  for(auto iter = requests.begin(); iter != requests.end(); ++iter) {

[fgorski, 2016/08/02 04:18:11]

for (const auto& request : requests) is the simplest allowed form I believe,
per:
https://chromium-cpp.appspot.com/  (Range-Based For Loops section)

[Pete Williamson, 2016/08/03 00:24:31]

Right, find_if would take a lambda and the collection, and search for matching
items.  I'm wondering if the lambda version introduces more or less cognitive
loading for the reader (ie, is it more or less readable?)

[fgorski, 2016/08/03 03:13:15]

What I suggested is not a lambda, but a C++11 idiom equivalent to your for loop,
except that it also enforces const.
If you insist on going with iterator, use cbeing() and cend().

[Pete Williamson, 2016/08/03 20:13:44]

I changed this to use a ranged for loop as you suggest.

Just out of curiosity, why cbegin and cend instead of begin and end?

+    if (client_namespace == iter->client_id().name_space) {
+      RequestStatus status(iter->client_id().id,
+                           iter->GetStatus(base::Time::Now()));

[fgorski, 2016/08/02 04:18:12]

consider extracting the time to a variable.
Also, is there a chance to use Timer instead of base::Time::Now()? That would
simplify testing.

[Pete Williamson, 2016/08/03 00:24:31]

Removed this code instead, we don't need a time anymore.

+      found_requests.push_back(status);
+    }
+  }
+
+  callback.Run(found_requests);
+

[fgorski, 2016/08/02 04:18:11]

nit: remove extra line

[Pete Williamson, 2016/08/03 00:24:31]

Done.

+}
 
 void RequestCoordinator::AddRequestResultCallback(
     RequestQueue::AddRequestResult result,
     const SavePageRequest& request) {
 
   // Inform the scheduler that we have an outstanding task..
   scheduler_->Schedule(GetTriggerConditionsForUserRequest());
 }
 
 // Called in response to updating a request in the request queue.
@@ skipping 158 matching lines @@
   return trigger_conditions;
 }
 
 void RequestCoordinator::GetOffliner() {
   if (!offliner_) {
     offliner_ = factory_->GetOffliner(policy_.get());
   }
 }
 
 }  // namespace offline_pages