[Offline pages] Applying TaskQueue to RequestQueue::ChangeRequestsState

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 814263ae519a46da9947527e5237226f06386109..1138909ac2e1abee9de1c9f3e09ce1c09fdb807b 100644
--- a/components/offline_pages/background/request_coordinator.cc
+++ b/components/offline_pages/background/request_coordinator.cc
@@ -294,26 +294,16 @@ void RequestCoordinator::UpdateRequestCallback(
   }
 }
 
-// Called in response to updating multiple requests in the request queue.
 void RequestCoordinator::UpdateMultipleRequestsCallback(
-    const RequestQueue::UpdateMultipleRequestResults& results,
-    std::vector<std::unique_ptr<SavePageRequest>> requests) {
+    std::unique_ptr<UpdateRequestsResult> result) {
+  for (const auto& request : result->updated_items)
+    NotifyChanged(request);
+
   bool available_user_request = false;
-  for (const auto& request : requests) {
-    NotifyChanged(*(request));
-    if (!available_user_request && request->user_requested() &&
-        request->request_state() == SavePageRequest::RequestState::AVAILABLE) {
-      // TODO(dougarnett): Consider avoiding prospect of N^2 in case
-      // size of bulk requests can get large (perhaps with easier to consume
-      // callback interface).
-      for (std::pair<int64_t, RequestQueue::UpdateRequestResult> pair :
-           results) {
-        if (pair.first == request->request_id() &&
-            pair.second == RequestQueue::UpdateRequestResult::SUCCESS) {
-          // We have a successfully updated, available, user request.
-          available_user_request = true;
-        }
-      }
+  for (const auto& request : result->updated_items) {

[dougarnett, 2016/09/29 21:02:49]

Could have just one iteration through updated_items

[fgorski, 2016/09/30 23:14:35]

I know, but I prefer this, because it is the same complexity, but clearer
separation (notify changed can be extracted from all methods and augmented by
event logger) and readability that way.

+    if (!available_user_request && request.user_requested() &&
+        request.request_state() == SavePageRequest::RequestState::AVAILABLE) {
+      available_user_request = true;
     }
   }