Adjust scheduling for non-user requested items

components/offline_pages/background/request_coordinator.h

 // 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_COORDINATOR_H_
 #define COMPONENTS_OFFLINE_PAGES_BACKGROUND_REQUEST_COORDINATOR_H_
 
 #include <memory>
 
 #include "base/callback.h"
@@ skipping 30 matching lines @@
   // ignore all events before the return from |GetAllRequests|, and consume
   // events after the return callback from |GetAllRequests|.
   class Observer {
    public:
     virtual void OnAdded(const SavePageRequest& request) = 0;
     virtual void OnCompleted(const SavePageRequest& request,
                              RequestNotifier::SavePageStatus status) = 0;
     virtual void OnChanged(const SavePageRequest& request) = 0;
   };
 
   // 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,
                      std::unique_ptr<RequestQueue> queue,
                      std::unique_ptr<Scheduler> scheduler);
 
   ~RequestCoordinator() override;
 
   // Queues |request| to later load and save when system conditions allow.
   // Returns true if the page could be queued successfully.
@@ skipping 33 matching lines @@
   // a callback. If processing was already active, returns false.
   bool StartProcessing(const DeviceConditions& device_conditions,
                        const base::Callback<void(bool)>& callback);
 
   // Stops the current request processing if active. This is a way for
   // caller to abort processing; otherwise, processing will complete on
   // its own. In either case, the callback will be called when processing
   // 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) {
     scheduler_callback_ = callback;
   }
 
   // Observers implementing the RequestCoordinator::Observer interface can
   // register here to get notifications of changes to request state.  This
   // pointer is not owned, and it is the callers responsibility to remove the
   // observer before the observer is deleted.
   void AddObserver(RequestCoordinator::Observer* observer);
 
   void RemoveObserver(RequestCoordinator::Observer* observer);
 
   // Implement RequestNotifier
   void NotifyAdded(const SavePageRequest& request) override;
   void NotifyCompleted(const SavePageRequest& request,
                        RequestNotifier::SavePageStatus status) override;
   void NotifyChanged(const SavePageRequest& request) override;
 
   // Returns the request queue used for requests.  Coordinator keeps ownership.
   RequestQueue* queue() { return queue_.get(); }
 
   // 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_;
   }
 
   bool is_busy() {
     return is_busy_;
   }
 
   // Tracks whether the last offlining attempt got canceled.  This is reset by
   // the next StartProcessing() call.
   bool is_canceled() {
     return is_stopped_;
   }
 
   OfflineEventLogger* GetLogger() {
     return &event_logger_;
   }
 
  private:
   // Receives the results of a get from the request queue, and turns that into
   // SavePageRequest objects for the caller of GetQueuedRequests.
   void GetQueuedRequestsCallback(const GetRequestsCallback& callback,
                                  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);
 
   // Receives the result of update and delete requests to the request queue.
   void UpdateRequestCallback(const ClientId& client_id,
                              RequestQueue::UpdateRequestResult result);
 
   void UpdateMultipleRequestsCallback(
       const RequestQueue::UpdateMultipleRequestResults& result,
       const std::vector<SavePageRequest>& requests);
 
   void HandleRemovedRequestsAndCallback(
       const RemoveRequestsCallback& callback,
       const RequestQueue::UpdateMultipleRequestResults& results,
       const std::vector<SavePageRequest>& requests);
 
   void HandleRemovedRequests(
       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();
 
   void SendRequestToOffliner(const SavePageRequest& request);
 
   // Called by the offliner when an offlining request is completed. (and by
   // tests).
   void OfflinerDoneCallback(const SavePageRequest& request,
                             Offliner::RequestStatus status);
@@ skipping 54 matching lines @@
   base::TimeDelta offliner_timeout_;
   // Allows us to pass a weak pointer to callbacks.
   base::WeakPtrFactory<RequestCoordinator> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(RequestCoordinator);
 };
 
 }  // namespace offline_pages
 
 #endif  // COMPONENTS_OFFLINE_PAGES_BACKGROUND_REQUEST_COORDINATOR_H_