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:
+  // Nested observer class
+  class Observer {
+   public:
+    virtual void OnAdded(const SavePageRequest& request) = 0;
+    virtual void OnSucceeded(
+        const SavePageRequest& request, int64_t offline_id) = 0;

[fgorski, 2016/08/10 21:30:44]

We should discuss making offline_id == request_id.

[Pete Williamson, 2016/08/11 00:08:36]

TODO added (in the SavePageLater function)

+    // TODO(petewil): Consider what the type of faliure_code should be.  We
+    // could use a new enum, re-use the offliner status enum, or just an int.
+    virtual void OnFailed(
+        const SavePageRequest& request, int64_t failure_code) = 0;
+    virtual void OnChanged(const SavePageRequest& request) = 0;

[fgorski, 2016/08/10 21:30:44]

Did we consider OnUpdated?

[Pete Williamson, 2016/08/11 00:08:36]

We discussed this in person, and nobody had a strong preference.  Code Search
shows more instances of OnChanged than OnUpdated, so I think it is more common,
sticking with OnChanged for now.

+  };
+
   // 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

[fgorski, 2016/08/10 21:30:44]

nit: "not owned" sounds better here.

[Pete Williamson, 2016/08/11 00:08:36]

Done.

+  // observer before the observer is deleted.  Add events after attaching an
+  // observer will be reported, even if the listener has not called
+  // GetQueuedRequests yet, so before GetQueuedRequests is called, the listener

[fgorski, 2016/08/10 21:30:44]

I think I disagree with Dmitry about having this part of the comment. Let's
discuss it today.

[Pete Williamson, 2016/08/11 00:08:36]

Comment updated per our discussion, and moved to the observer instead of the
AddObserver method.

+  // may need to ignore events.
+  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 23 matching lines @@
   // 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 UpdateMultipleRequestCallback(RequestQueue::UpdateRequestResult result);
 
+  void NotifyChangedListCallback(const std::vector<int64_t> request_ids,

[fgorski, 2016/08/10 21:30:44]

is copying intended here?

[Pete Williamson, 2016/08/11 00:08:36]

Done.

+                                 RequestQueue::GetRequestsResult result,
+                                 const std::vector<SavePageRequest>& requests);
+
   // 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 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 NotifyAdded(const SavePageRequest& request);
+  void NotifySucceeded(const SavePageRequest& request, int64_t offline_id);
+  void NotifyFailed(const SavePageRequest& request, int64_t failure_code);
+  void NotifyChanged(const SavePageRequest& request);
+
+  // Sends a changed notification for each request_id in the list.
+  void SendNotifyChangeForRequestIdList(std::vector<int64_t> request_ids);
+
   // 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_