Adjust scheduling for non-user requested items

components/offline_pages/background/request_coordinator.h

Index: components/offline_pages/background/request_coordinator.h
diff --git a/components/offline_pages/background/request_coordinator.h b/components/offline_pages/background/request_coordinator.h
index 885970c48613a59961b4d31060ae96b0b408a254..545e2675d83aa9884e94bc1630cde2afe28be0ce 100644
--- a/components/offline_pages/background/request_coordinator.h
+++ b/components/offline_pages/background/request_coordinator.h
@@ -51,7 +51,7 @@ class RequestCoordinator : public KeyedService, public RequestNotifier {
   // Callback to report when the processing of a triggered task is complete.

[dougarnett, 2016/08/29 15:47:22]

comment doesn't seem to match

[Pete Williamson, 2016/09/01 00:10:31]

Done.

   typedef base::Callback<void(const SavePageRequest& request)>
       RequestPickedCallback;
-  typedef base::Callback<void()> RequestQueueEmptyCallback;
+  typedef base::Callback<void(bool)> RequestNotPickedCallback;
 
   RequestCoordinator(std::unique_ptr<OfflinerPolicy> policy,
                      std::unique_ptr<OfflinerFactory> factory,
@@ -105,7 +105,8 @@ class RequestCoordinator : public KeyedService, public RequestNotifier {
   // is stopped or complete.
   void StopProcessing();
 
-  const Scheduler::TriggerConditions GetTriggerConditionsForUserRequest();
+  const Scheduler::TriggerConditions GetTriggerConditions(
+      const bool user_requested);
 
   // A way for tests to set the callback in use when an operation is over.
   void SetProcessingCallbackForTest(const base::Callback<void(bool)> callback) {
@@ -132,6 +133,8 @@ class RequestCoordinator : public KeyedService, public RequestNotifier {
   // Return an unowned pointer to the Scheduler.
   Scheduler* scheduler() { return scheduler_.get(); }
 
+  OfflinerPolicy* policy() { return policy_.get(); }
+
   // Returns the status of the most recent offlining.
   Offliner::RequestStatus last_offlining_status() {
     return last_offlining_status_;
@@ -158,6 +161,12 @@ class RequestCoordinator : public KeyedService, public RequestNotifier {
                                  RequestQueue::GetRequestsResult result,
                                  const std::vector<SavePageRequest>& requests);
 
+  // Receives the results of a get from the request queue, and turns that into
+  // SavePageRequest objects for the caller of GetQueuedRequests.
+  void GetRequestsForSchedulingCallback(
+      RequestQueue::GetRequestsResult result,
+      const std::vector<SavePageRequest>& requests);
+
   // Receives the result of add requests to the request queue.
   void AddRequestResultCallback(RequestQueue::AddRequestResult result,
                                 const SavePageRequest& request);
@@ -179,11 +188,15 @@ class RequestCoordinator : public KeyedService, public RequestNotifier {
       const RequestQueue::UpdateMultipleRequestResults& results,
       const std::vector<SavePageRequest>& requests);
 
+  // Check the request queue, and schedule a task corresponding
+  // to the least restrictive type of request in the queue.
+  void ScheduleAsNeeded();
+
   // Callback from the request picker when it has chosen our next request.
   void RequestPicked(const SavePageRequest& request);
 
   // Callback from the request picker when no more requests are in the queue.
-  void RequestQueueEmpty();
+  void RequestNotPicked(bool non_user_requested_tasks_remaining);

[dougarnett, 2016/08/29 15:47:22]

I wonder if this should be something like least_restrictive_task or
next_trigger_conditions to allow for other concerns (like activation time or
namespace) to be communicated as well.

[Pete Williamson, 2016/09/01 00:10:31]

While it would be useful to have something like TriggerConditions to be passed
here, I'd prefer to have only request coordinator need to know about device
conditions, so I prefer passing back a boolean for request type instead of
device conditions.  While we could pass back a request, and may someday, which
one?  If there are multiple requests, one might not speak for all, so if we
expand the number of types, I'd prefer to make this boolean into an enum, and
pass back a type here.

[dougarnett, 2016/09/01 18:24:13]

Ok, please comment the point of the arg so a bit easier to understand purpose.
Perhaps something along the lines of: 

// The parameter is a signal for what, if any, conditions to schedule future
processing for.

[Pete Williamson, 2016/09/01 21:56:08]

Done.

 
   // Cancels an in progress pre-rendering, and updates state appropriately.
   void StopPrerendering();