[Offline pages] Adding in memory request queue store with tests

components/offline_pages/background/request_queue_in_memory_store.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_queue_in_memory_store.h"
+
+#include "base/bind.h"
+#include "base/location.h"
+#include "base/thread_task_runner_handle.h"
+#include "components/offline_pages/background/save_page_request.h"
+
+namespace offline_pages {
+
+RequestQueueInMemoryStore::RequestQueueInMemoryStore() {}
+
+RequestQueueInMemoryStore::~RequestQueueInMemoryStore() {}
+
+void RequestQueueInMemoryStore::GetRequests(

[Pete Williamson, 2016/05/03 20:38:52]

In the future, it might be nice to be able to search requests by some condition
(newest first, oldest first, least retried first).  No action needed now.

[fgorski, 2016/05/03 21:50:21]

Acknowledged.

+    const GetRequestsCallback& callback) {
+  std::vector<SavePageRequest> result;
+  for (const auto& id_request_pair : requests_)
+    result.push_back(id_request_pair.second);
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::Bind(callback, true, result));

[Pete Williamson, 2016/05/03 20:38:52]

Would it make sense to batch these?  This seems like a lot of async calls, it
might be inefficient.

[fgorski, 2016/05/03 21:50:21]

What is there to batch here exactly?
This method makes a single call with a vector containing results.

[Pete Williamson, 2016/05/03 22:00:24]

Ah, never mind, I missed that the result was a vector.  Objections withdrawn.

[dougarnett, 2016/05/03 22:11:54]

perhaps if "result" was named "results", it would have been easier to read?

[fgorski, 2016/05/03 23:01:42]

I see. I updated this to result_requests. I hope it will be more readable now.

+}
+
+void RequestQueueInMemoryStore::AddOrUpdateRequest(
+    const SavePageRequest& request,
+    const UpdateCallback& callback) {
+  RequestsMap::iterator iter = requests_.find(request.request_id());
+  if (iter != requests_.end())
+    requests_.erase(iter);
+  requests_.insert(std::make_pair(request.request_id(), request));
+  base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
+                                                base::Bind(callback, true));
+}
+
+void RequestQueueInMemoryStore::RemoveRequests(
+    const std::vector<int64_t>& request_ids,
+    const UpdateCallback& callback) {
+  // We report true if the request ids is an empty list, or when we deleted at
+  // least one request.

[Pete Williamson, 2016/05/03 20:38:52]

So how will the caller know when the list was empty and when at least one got
deleted?  It might be better to return true only when we delete something.

[fgorski, 2016/05/03 21:50:21]

Done.

+  bool result = request_ids.empty();
+
+  RequestsMap::iterator iter;
+  for (auto request_id : request_ids) {
+    iter = requests_.find(request_id);
+    if (iter != requests_.end()) {
+      requests_.erase(iter);
+      result = true;
+    }
+  }
+
+  base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
+                                                base::Bind(callback, result));
+}
+
+void RequestQueueInMemoryStore::Reset(const ResetCallback& callback) {
+  // TODO(fgorski): use thread task runner handle to post the results back.
+  requests_.clear();
+  base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
+                                                base::Bind(callback, true));
+}
+
+}  // namespace offline_pages