API to provide status of save page reqeusts to API.

components/offline_pages/background/request_coordinator.cc

Index: components/offline_pages/background/request_coordinator.cc
diff --git a/components/offline_pages/background/request_coordinator.cc b/components/offline_pages/background/request_coordinator.cc
index 5876d8386d79b7c3834f6cf9b7d72bd22f8c38cd..7e7862f2e6d3155c151ceed7fd503902f3a38c4d 100644
--- a/components/offline_pages/background/request_coordinator.cc
+++ b/components/offline_pages/background/request_coordinator.cc
@@ -69,6 +69,37 @@ bool RequestCoordinator::SavePageLater(
                                 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,