Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(7413)

Unified Diff: components/offline_pages/background/request_coordinator.h

Issue 2219393004: Adds an observer for the request coordinator. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@resumeAPI
Patch Set: First round of feedback Created 4 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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 3e52021e3597ca352a12cc6d2fdacad9c25b6db8..0640aab0024e6e5132f4297dbe62931e18499c17 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,19 @@ 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 stro
+ };
+
// Callback to report when the processing of a triggered task is complete.
typedef base::Callback<void(const SavePageRequest& request)>
RequestPickedCallback;
@@ -93,6 +107,17 @@ 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
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 p
Pete Williamson 2016/08/11 00:08:36 Comment updated per our discussion, and moved to t
+ // 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(); }
@@ -136,6 +161,10 @@ class RequestCoordinator : public KeyedService {
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);
@@ -151,6 +180,14 @@ class RequestCoordinator : public KeyedService {
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();
@@ -174,6 +211,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

Powered by Google App Engine
This is Rietveld 408576698