Adds an observer for the request coordinator.

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 3c6b96e280b6d04356dcd01bdb2ffaf7f401f157..e7f2215594fb1c8a36724fbf9f9896fedf71e899 100644
--- a/components/offline_pages/background/request_coordinator.h
+++ b/components/offline_pages/background/request_coordinator.h
@@ -10,6 +10,7 @@
 #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"
@@ -33,6 +34,21 @@ 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;
@@ -93,6 +109,14 @@ class RequestCoordinator : public KeyedService {
     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(); }
 
@@ -151,6 +175,11 @@ class RequestCoordinator : public KeyedService {
 
   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();
@@ -174,6 +203,8 @@ class RequestCoordinator : public KeyedService {
   // 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