Request Picker task

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
index 3168ea1d90539654e40e83f4ca3f6a44adade9bf..7667583880c072b00c449c331c059da2c7c4f98f 100644
--- a/components/offline_pages/background/request_queue.h
+++ b/components/offline_pages/background/request_queue.h
@@ -8,12 +8,17 @@
 #include <stdint.h>
 
 #include <memory>
+#include <set>
 #include <string>
 #include <vector>
 
 #include "base/callback.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
+#include "components/offline_pages/background/device_conditions.h"
+#include "components/offline_pages/background/pick_request_task.h"
+#include "components/offline_pages/background/pick_request_task_builder.h"
+#include "components/offline_pages/background/queue_results.h"
 #include "components/offline_pages/background/save_page_request.h"
 #include "components/offline_pages/core/task_queue.h"
 #include "components/offline_pages/offline_page_item.h"
@@ -22,47 +27,30 @@
 namespace offline_pages {
 
 class RequestQueueStore;
-typedef StoreUpdateResult<SavePageRequest> UpdateRequestsResult;
+class PickRequestTaskBuilder;
 
 // Class responsible for managing save page requests.
 class RequestQueue {
  public:
-  enum class GetRequestsResult {
-    SUCCESS,
-    STORE_FAILURE,
-  };
-
-  enum class AddRequestResult {
-    SUCCESS,
-    STORE_FAILURE,
-    ALREADY_EXISTS,
-    REQUEST_QUOTA_HIT,  // Cannot add a request with this namespace, as it has
-                        // reached a quota of active requests.
-  };
-
-  // GENERATED_JAVA_ENUM_PACKAGE:org.chromium.components.offlinepages.background
-  enum class UpdateRequestResult {
-    SUCCESS,
-    STORE_FAILURE,
-    REQUEST_DOES_NOT_EXIST,  // Failed to delete the request because it does not
-                             // exist.
-  };
 
   // Callback used for |GetRequests|.
-  typedef base::Callback<void(GetRequestsResult,
+  typedef base::Callback<void(QueueResults::GetRequestsResult,
                               std::vector<std::unique_ptr<SavePageRequest>>)>
       GetRequestsCallback;
 
   // Callback used for |AddRequest|.
-  typedef base::Callback<void(AddRequestResult, const SavePageRequest& request)>
+  typedef base::Callback<void(QueueResults::AddRequestResult,
+                              const SavePageRequest& request)>
       AddRequestCallback;
 
   // Callback used by |ChangeRequestsState|.
-  typedef base::Callback<void(std::unique_ptr<UpdateRequestsResult>)>
+  typedef base::Callback<void(
+      std::unique_ptr<QueueResults::UpdateRequestsResult>)>
       UpdateCallback;
 
   // Callback used by |UdpateRequest|.
-  typedef base::Callback<void(UpdateRequestResult)> UpdateRequestCallback;
+  typedef base::Callback<void(QueueResults::UpdateRequestResult)>
+      UpdateRequestCallback;
 
   explicit RequestQueue(std::unique_ptr<RequestQueueStore> store);
   ~RequestQueue();
@@ -103,9 +91,22 @@ class RequestQueue {
   // are returned through |callback|.
   void MarkAttemptCompleted(int64_t request_id, const UpdateCallback& callback);
 
+  // Make a task to pick the next request, and report our choice to the
+  // callbacks.
+  void PickNextRequest(
+      PickRequestTask::RequestPickedCallback picked_callback,
+      PickRequestTask::RequestNotPickedCallback not_picked_callback,
+      DeviceConditions* conditions,
+      std::set<int64_t>& disabled_requests);
+
+  // Takes ownership of the builder.

[fgorski, 2016/11/03 22:00:29]

if that is the case, it should be using a unique ptr.
Also is that done in production code or for testing only?

[Pete Williamson, 2016/11/04 18:53:37]

This is for production code.  The idea is that we set it after the creation of
the request queue so that the request coordinator factory doesn't need to build
it at request queue time, so it can be built later.  The intent is that it is
easier to create a request_queue (which has lots of references all over the
code).  Let me know if you prefer, and I can instead set it in the factory.

Also, changed to use a unique ptr.

+  void SetPickerBuilder(PickRequestTaskBuilder* builder) {
+    picker_builder_.reset(builder);
+  }
+
  private:
   // Callback used by |PurgeRequests|.
-  typedef base::Callback<void(UpdateRequestResult,
+  typedef base::Callback<void(QueueResults::UpdateRequestResult,
                               int /* removed requests count */)>
       PurgeRequestsCallback;
 
@@ -119,6 +120,9 @@ class RequestQueue {
   // Task queue to serialize store access.
   TaskQueue task_queue_;
 
+  // Builds PickRequestTask objects
+  std::unique_ptr<PickRequestTaskBuilder> picker_builder_;
+
   // Allows us to pass a weak pointer to callbacks.
   base::WeakPtrFactory<RequestQueue> weak_ptr_factory_;