More detailed implementation of the RequestPicker

components/offline_pages/background/request_picker.h

Index: components/offline_pages/background/request_picker.h
diff --git a/components/offline_pages/background/request_picker.h b/components/offline_pages/background/request_picker.h
index dd2b7ff295ae22a60ead1f0d31d32131641293c5..e64f712875e31f73639d92241395b9909856815d 100644
--- a/components/offline_pages/background/request_picker.h
+++ b/components/offline_pages/background/request_picker.h
@@ -5,14 +5,22 @@
 #ifndef COMPONENTS_OFFLINE_PAGES_BACKGROUND_REQUEST_PICKER_H_
 #define COMPONENTS_OFFLINE_PAGES_BACKGROUND_REQUEST_PICKER_H_
 
+#include <memory>
+
 #include "base/memory/weak_ptr.h"
+#include "components/offline_pages/background/device_conditions.h"
+#include "components/offline_pages/background/offliner_policy.h"
 #include "components/offline_pages/background/request_coordinator.h"
 #include "components/offline_pages/background/request_queue.h"
 
 namespace offline_pages {
+
+typedef bool (RequestPicker::*RequestCompareFunction)(

[fgorski, 2016/07/21 17:01:20]

nit: RequestLessThanFunction perhaps?

[Pete Williamson, 2016/07/21 19:48:46]

I'd prefer to leave it as is.  It isn't really a less than function.

+    const SavePageRequest* left, const SavePageRequest* right);
+
 class RequestPicker {
  public:
-  RequestPicker(RequestQueue* requestQueue);
+  RequestPicker(RequestQueue* requestQueue, OfflinerPolicy* policy);
 
   ~RequestPicker();
 
@@ -20,15 +28,43 @@ class RequestPicker {
   // conditions, and call back to the RequestCoordinator when we have one.
   void ChooseNextRequest(
       RequestCoordinator::RequestPickedCallback picked_callback,
-      RequestCoordinator::RequestQueueEmptyCallback empty_callback);
+      RequestCoordinator::RequestQueueEmptyCallback empty_callback,
+      DeviceConditions* device_conditions);
 
  private:
   // Callback for the GetRequest results to be delivered.
   void GetRequestResultCallback(RequestQueue::GetRequestsResult result,
                                 const std::vector<SavePageRequest>& results);
 
+  // Filter out requests that don't meet the current conditions.  For instance,
+  // if this is a predictive request, and we are not on WiFi, it should be
+  // ignored this round.
+  bool RequestConditionsSatisfied(const SavePageRequest& request);
+
+  // Using policies, decide if the new request is preferable to the best we have
+  // so far.
+  bool IsNewRequestBetter(const SavePageRequest* oldRequest,
+                          const SavePageRequest* newRequest,
+                          RequestCompareFunction comparator);
+
+  // Is the new request preferable from the retry count first standpoint?
+  bool RetryCountFirstCompareFunction(const SavePageRequest* left,
+                                      const SavePageRequest* right);
+
+  // Is the new request better from the recency first standpoint?
+  bool RecencyFirstCompareFunction(const SavePageRequest* left,
+                                   const SavePageRequest* right);
+
   // unowned pointer to the request queue.
   RequestQueue* queue_;
+  // unowned pointer to the policy object.
+  OfflinerPolicy* policy_;
+  // Current conditions on the device
+  std::unique_ptr<DeviceConditions> current_conditions_;
+  // True if we prefer less-tried requests
+  bool fewer_retries_better_;
+  // True if we prefer requests submitted more recently
+  bool earlier_requests_better_;
   // Callback for when we are done picking a request to do next.
   RequestCoordinator::RequestPickedCallback picked_callback_;
   // Callback for when there are no more reqeusts to pick.