Provide API in RequestCoordinator to remove results by client ID.

components/offline_pages/background/request_queue_in_memory_store.cc

Index: components/offline_pages/background/request_queue_in_memory_store.cc
diff --git a/components/offline_pages/background/request_queue_in_memory_store.cc b/components/offline_pages/background/request_queue_in_memory_store.cc
index defb87e311ffed9c84328e47b31764f9da3d00d9..380696e84f07e957b0bb4a98b7bfd4e26089242b 100644
--- a/components/offline_pages/background/request_queue_in_memory_store.cc
+++ b/components/offline_pages/background/request_queue_in_memory_store.cc
@@ -38,8 +38,6 @@ void RequestQueueInMemoryStore::AddOrUpdateRequest(
 void RequestQueueInMemoryStore::RemoveRequests(
     const std::vector<int64_t>& request_ids,
     const RemoveCallback& callback) {
-  // In case the |request_ids| is empty, the result will be true, but the count
-  // of deleted pages will be empty.
   int count = 0;
   RequestsMap::iterator iter;
   for (auto request_id : request_ids) {
@@ -54,6 +52,32 @@ void RequestQueueInMemoryStore::RemoveRequests(
       FROM_HERE, base::Bind(callback, true, count));
 }
 
+void RequestQueueInMemoryStore::RemoveRequestsByClientId(
+    const std::vector<ClientId>& client_ids,
+    const RemoveCallback& callback) {
+  int count = 0;
+
+  // Once for each client id in our list
+  for (auto client_id : client_ids) {

[fgorski, 2016/08/02 05:01:22]

const auto&

[Pete Williamson, 2016/08/02 23:27:17]

Done.

+    // Check it against each request in the store.
+    for (auto iter = requests_.begin();
+         iter != requests_.end();
+         ++iter) {

[fgorski, 2016/08/02 05:01:22]

(I am aware this is only used for testing, but consider the following)

There are at least 2 problems with this code:
* It does not work for non-unique ClientID
* It is O(m*n)

How about:
1. putting client IDs into a <set>
2. iterate over requests_ with a proper pre- and post- increment?

for (auto iter = requests_.begin(); iter != requests_.end(); ) {
  if (client_id_set.find(iter->second.client_id()) != client_id_set.end()) {
    requests_.erase(iter++);
    ++count;
  } else {
    ++iter;
  }
}

[Pete Williamson, 2016/08/02 23:27:17]

OK, but I had to add operator< to ClientID to make it work in a set.

[fgorski, 2016/08/03 03:29:41]

That is because it is backed by BST. Have you tried unordered_set?

[Pete Williamson, 2016/08/03 19:51:08]

I think it might be faster as an ordered set - using the ordering, it might be
able to search faster, we could (in theory, depending on implementation) end up
as O(M*log(N)) instead of O(M*N)

+      // Check against each incoming client ID, and delete if we have a match.
+      if ((*iter).second.client_id() == client_id) {
+        requests_.erase(iter);
+        // This delete will invalidate the iterator, so move on to the next
+        // entry.
+        ++count;
+        break;
+      }
+    }
+  }
+
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::Bind(callback, true, count));
+}
+
 void RequestQueueInMemoryStore::Reset(const ResetCallback& callback) {
   requests_.clear();
   base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,