[Offline pages] Empty RequestQueue and SavePageRequest

components/offline_pages/background/request_queue.h

Index: components/offline_pages/background/request_queue.h
diff --git a/components/offline_pages/background/request_queue.h b/components/offline_pages/background/request_queue.h
new file mode 100644
index 0000000000000000000000000000000000000000..bc974fa66613a89bd1ab06199d82fd4e7b975b05
--- /dev/null
+++ b/components/offline_pages/background/request_queue.h
@@ -0,0 +1,86 @@
+// 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.
+
+#ifndef COMPONENTS_OFFLINE_PAGES_BACKGROUND_REQUEST_QUEUE_H_
+#define COMPONENTS_OFFLINE_PAGES_BACKGROUND_REQUEST_QUEUE_H_
+
+#include <stdint.h>
+
+#include "base/callback.h"
+
+namespace offline_pages {
+
+class SavePageRequest;
+
+// Class responsible for managing save page requests.
+class RequestQueue {
+ public:
+  enum class GetRequestsResult {
+    kSuccess,
+    kStoreFailure,
+  };
+
+  enum class AddRequestResult {
+    kSuccess,
+    kStoreFailure,
+    kActiveRequestQuotaHit,  // Cannot add a request with this namespace, as
+                             // it has reached a quota of active requests.

[Pete Williamson, 2016/04/28 23:16:07]

Why do we need two quotas? Would one suffice?  I suspect that most requests will
be active (not too many will be "Don't load this until 5AM", for instance, so we
should count that as active, and just have one quota for active requests).

[fgorski, 2016/04/29 05:19:15]

Done.

+    kTotalRequestQuotaHit,   // Cannot add a request with this namespace, as
+                             // it has reached a quota of total requests.
+  };
+
+  enum class UpdateRequestResult {
+    kSuccess,
+    kStoreFailure,
+    kRequestDoesNotExist,  // Failed to delete the request because it does not
+                           // exist.
+  };
+
+  // Callback used for |GetRequests|.

[Pete Williamson, 2016/04/28 23:16:07]

Why make these async?  I'm not averse to async per se, I just want to know what
we get for the extra complexity.

[fgorski, 2016/04/29 05:19:15]

The capability to store and read it from a file.

+  typedef base::Callback <
+      void(GetRequestsResult,
+           const std::vector<SavePageRequest>&) GetRequestsCallback;
+
+  // Callback used for |AddRequest|.
+  typedef base::Callback<void(AddRequestResult, const SavePageRequest& request)>
+      AddRequestCallback;
+
+  // Callback used by |UdpateRequest| and |RemoveRequest|.
+  typedef base::Callback<void(UpdateRequestResult)> UpdateRequestCallback;
+
+  // Gets all of the active requests from the store. Calling this method may
+  // schedule purging of the request queue.

[Pete Williamson, 2016/04/28 23:16:07]

Why not have all the methods schedule a queue purge?

[fgorski, 2016/04/29 05:19:15]

Because other methods don't know if there is anything to clean up, while this
one has a pretty good view off the queue. That way if it notices that there are
some tasks that are long expired/failed, it can simply schedule purging. 

+  void GetRequests(const GetRequestsResult& callback);
+
+  // Adds |request| to the request queue. Result is returned through |callback|.
+  // In case adding the request violates policy, the result will fail with
+  // appropriate result. Callback will also return a copy of a request with all
+  // fields set.
+  void AddRequest(const SavePageRequest& request,
+                  const AddRequestCallback& callback);
+
+  // Updates a request in the request queue if a request with matching ID
+  // exists. Does nothing otherwise. Result is returned through |callback|.
+  void UpdateRequest(const SavePageRequest& request,
+                     const UpdateRequestCallback& callback);
+
+  // Removes the request matching the |request_id|. Result is returned through
+  // |callback|.
+  void RemoveRequest(int64_t request_id, const UpdateRequestCallback& callback);
+
+ private:
+  // Callback used by |PurgeRequests|.
+  typedef base::Callback<void(UpdateRequestResult,
+                              int /* removed requests count */)>
+      PurgeRequestsCallback;
+
+  // Purges the queue, removing the requests that are no longer relevant, e.g.
+  // expired request. Result is returned through |callback| carries the number
+  // of removed requests.
+  void PurgeRequests(const PurgeRequestsCallback& callback);
+};
+
+}  // namespace offline_pages
+
+#endif  // COMPONENTS_OFFLINE_PAGES_BACKGROUND_REQUEST_QUEUE_H_