Adds an observer for the request coordinator.

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"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
+#include "base/observer_list.h"
 #include "base/time/time.h"
 #include "base/timer/timer.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "components/offline_pages/background/device_conditions.h"
 #include "components/offline_pages/background/offliner.h"
 #include "components/offline_pages/background/request_coordinator_event_logger.h"
 #include "components/offline_pages/background/request_queue.h"
 #include "components/offline_pages/background/scheduler.h"
 #include "url/gurl.h"
 
 namespace offline_pages {
 
 struct ClientId;
 class OfflinerPolicy;
 class OfflinerFactory;
 class Offliner;
 class RequestPicker;
 class SavePageRequest;
 class Scheduler;
 
 // Coordinates queueing and processing save page later requests.
 class RequestCoordinator : public KeyedService {
  public:
+
+  // Data struct for nested observer.  Add more items as needed.
+  struct RequestInfo {
+    ClientId client_id;
+  };
+
+  // Nested observer class
+  class Observer {
+   public:
+    virtual void OnAvailable(RequestInfo info) = 0;

[Dmitry Titov, 2016/08/09 20:14:59]

Since this is an internal, most generic interface to the collection of
SavePageRequests, it should be simply defined in terms of items it serves,
SavePageRequests. As in:

Gets the whole request because it'll almost inevitably trigger a query for that
request right away:

OnAvailable(const SavePageRequest& request);


Also, once Succeeded or Failed, there is additional data to pass, like
newly-created offline page or failure code.

OnSucceeded(int64_t request_id, int64_t offline_id);

I assume this one includes all other reasons the request may disappear,
including things like expiration or removal from internal page?

OnFailed(int64_t request_id, SomeFailureEnum failure);

There should be onUnpause probably...

OnPaused(int64_t request_id);

Another variant (probably better) would be to have 
OnAdded (this is fired first time we add request)
OnChanged (this is fired every time it's tried, paused, unpaused etc - when any
field of it changes)
OnSucceeded (fired when created offlien page and was deleted)
OnFailed (fired when did not result in offline page and was deleted)

This basically clamps all states 'during processing' into OnChanged, which is
probably better because as we invent more internal states ('paused because of
lower priority' etc) we don't need to expand the observer interface.

Open for discussion: we may pass int64_t request_id or a reference to the whole
request. I think many observers will want to know more about request right away,
so I can see why we may pass a reference.

[Pete Williamson, 2016/08/10 01:36:23]

OK, changed to 
OnAdded
OnSucceeded(item, offline_id)
OnFailed(request, failure_code)
OnChanged(request) -> This one gets called for Pause and Unpause

+    virtual void OnSucceeded(RequestInfo info) = 0;
+    virtual void OnFailed(RequestInfo info) = 0;
+    virtual void OnPaused(RequestInfo info) = 0;
+  };
+
   // Callback to report when the processing of a triggered task is complete.
   typedef base::Callback<void(const SavePageRequest& request)>
       RequestPickedCallback;
   typedef base::Callback<void()> RequestQueueEmptyCallback;
 
   RequestCoordinator(std::unique_ptr<OfflinerPolicy> policy,
                      std::unique_ptr<OfflinerFactory> factory,
                      std::unique_ptr<RequestQueue> queue,
                      std::unique_ptr<Scheduler> scheduler);
 
@@ skipping 40 matching lines @@
   // is stopped or complete.
   void StopProcessing();
 
   const Scheduler::TriggerConditions GetTriggerConditionsForUserRequest();
 
   // 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 unowned, and it is the callers responsibility to remove the
+  // observer before the observer is deleted.

[Dmitry Titov, 2016/08/09 20:14:59]

This comment should also mention what happens when the observer is added wrt the
existing collection. For example, lets say I want to have UI that shows all
requests in their current state. If I GetQueuedRequests(), I get a list
asynchronously. When should I start observing to keep my UI (which is
essentially a local cache of request information) up-to-date reliably? If I
start listening too soon (before querying the requests), I may get updates for
items I don't yet have or that do not exist anymore, but if I start listening
too late (after obtaining the result from the query), I may miss some updates.

[Pete Williamson, 2016/08/10 01:36:23]

Done.

+  void AddObserver(RequestCoordinator::Observer* observer);
+
+  void RemoveObserver(RequestCoordinator::Observer* observer);
+
   // 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(); }
 
   // Returns the status of the most recent offlining.
   Offliner::RequestStatus last_offlining_status() {
     return last_offlining_status_;
   }
@@ skipping 38 matching lines @@
 
   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);
 
   void TryNextRequest();
 
+  void NotifyAvailable(ClientId client_id);
+  void NotifySucceeded(ClientId client_id);
+  void NotifyFailed(ClientId client_id);
+  void NotifyPaused(ClientId client_id);
+
   // Returns the appropriate offliner to use, getting a new one from the factory
   // if needed.
   void GetOffliner();
 
   void SetOfflinerTimeoutForTest(const base::TimeDelta& timeout) {
     offliner_timeout_ = timeout;
   }
 
   void SetDeviceConditionsForTest(DeviceConditions& current_conditions) {
     current_conditions_.reset(new DeviceConditions(current_conditions));
   }
 
   friend class RequestCoordinatorTest;
 
   // The offliner can only handle one request at a time - if the offliner is
   // busy, prevent other requests.  This flag marks whether the offliner is in
   // use.
   bool is_busy_;
   // True if the current request has been canceled.
   bool is_canceled_;
   // Unowned pointer to the current offliner, if any.
   Offliner* offliner_;
   base::Time operation_start_time_;
+  // The observers.
+  base::ObserverList<Observer> observers_;
   // Last known conditions for network, battery
   std::unique_ptr<DeviceConditions> current_conditions_;
   // RequestCoordinator takes over ownership of the policy
   std::unique_ptr<OfflinerPolicy> policy_;
   // OfflinerFactory.  Used to create offline pages. Owned.
   std::unique_ptr<OfflinerFactory> factory_;
   // RequestQueue.  Used to store incoming requests. Owned.
   std::unique_ptr<RequestQueue> queue_;
   // Scheduler. Used to request a callback when network is available.  Owned.
   std::unique_ptr<Scheduler> scheduler_;
@@ skipping 13 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_