Adds request queue removal capability to OfflinePageBridge.

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 88ebb6092d655506a1fd9a701bb813f1639ce229..e449b891296ee97550ed3828001e214a73f90269 100644
--- a/components/offline_pages/background/request_coordinator.cc
+++ b/components/offline_pages/background/request_coordinator.cc
@@ -113,10 +113,12 @@ void RequestCoordinator::GetQueuedRequestsCallback(
 }
 
 void RequestCoordinator::RemoveRequests(
-    const std::vector<int64_t>& request_ids) {
-  queue_->RemoveRequests(request_ids,
-                         base::Bind(&RequestCoordinator::RemoveRequestsCallback,
-                                    weak_ptr_factory_.GetWeakPtr()));
+    const std::vector<int64_t>& request_ids,
+    const RemoveRequestsCallback& callback) {
+  queue_->RemoveRequests(
+      request_ids,
+      base::Bind(&RequestCoordinator::HandleRemovedRequestsAndCallback,
+                 weak_ptr_factory_.GetWeakPtr(), callback));
 }
 
 void RequestCoordinator::PauseRequests(
@@ -164,7 +166,15 @@ void RequestCoordinator::UpdateMultipleRequestsCallback(
     NotifyChanged(request);
 }
 
-void RequestCoordinator::RemoveRequestsCallback(
+void RequestCoordinator::HandleRemovedRequestsAndCallback(
+    const RemoveRequestsCallback& callback,
+    const RequestQueue::UpdateMultipleRequestResults& results,
+    const std::vector<SavePageRequest>& requests) {
+  HandleRemovedRequests(results, requests);

[fgorski, 2016/08/19 21:51:19]

3 things that I am not comfortable here:
1. results is never used (even in HandleRemovedRequests)
2. callback should be invoked first if available, followed by notifications
3. since you can probably pass a dummy callback in, you don't need 2 Handle...
methods.

[dewittj, 2016/08/19 21:56:37]

I agree this is concerning; I don't think it's a thing I should solve in this
cl.
Can you explain that rationale?
Since there are multiple HandleRemovedRequests calls, having a dummy callback
would be a bit of noise, is having the second method that bad?

[dewittj, 2016/08/20 03:22:35]

I solved this for my API; However the request coordinator itself doesn't use the
results from the remove operation.
I generally think your explanation makes sense, switched the order.

+  callback.Run(requests);
+}
+
+void RequestCoordinator::HandleRemovedRequests(
     const RequestQueue::UpdateMultipleRequestResults& results,
     const std::vector<SavePageRequest>& requests) {
   for (SavePageRequest request : requests)
@@ -323,7 +333,7 @@ void RequestCoordinator::OfflinerDoneCallback(const SavePageRequest& request,
     std::vector<int64_t> remove_requests;
     remove_requests.push_back(request.request_id());
     queue_->RemoveRequests(
-        remove_requests, base::Bind(&RequestCoordinator::RemoveRequestsCallback,
+        remove_requests, base::Bind(&RequestCoordinator::HandleRemovedRequests,
                                     weak_ptr_factory_.GetWeakPtr()));
     NotifyCompleted(request, SavePageStatus::SUCCESS);
   } else if (request.completed_attempt_count() + 1 >=
@@ -335,7 +345,7 @@ void RequestCoordinator::OfflinerDoneCallback(const SavePageRequest& request,
     std::vector<int64_t> remove_requests;
     remove_requests.push_back(request.request_id());
     queue_->RemoveRequests(
-        remove_requests, base::Bind(&RequestCoordinator::RemoveRequestsCallback,
+        remove_requests, base::Bind(&RequestCoordinator::HandleRemovedRequests,
                                     weak_ptr_factory_.GetWeakPtr()));
     NotifyCompleted(request, SavePageStatus::RETRY_COUNT_EXCEEDED);
   } else {