Adds an observer for the request coordinator.

components/offline_pages/background/request_coordinator.cc

Index: components/offline_pages/background/request_coordinator.cc
diff --git a/components/offline_pages/background/request_coordinator.cc b/components/offline_pages/background/request_coordinator.cc
index 5864479b6e940df6f14150d5ec6dfa4dcfdb375e..d7d694a5c39083862bb424d9c1d0026c63285a34 100644
--- a/components/offline_pages/background/request_coordinator.cc
+++ b/components/offline_pages/background/request_coordinator.cc
@@ -88,6 +88,7 @@ bool RequestCoordinator::SavePageLater(
   queue_->AddRequest(request,
                      base::Bind(&RequestCoordinator::AddRequestResultCallback,
                                 weak_ptr_factory_.GetWeakPtr()));
+  NotifyAdded(request);
   return true;
 }
 void RequestCoordinator::GetQueuedRequests(
@@ -130,6 +131,7 @@ void RequestCoordinator::PauseRequests(
       request_ids,
       base::Bind(&RequestCoordinator::UpdateMultipleRequestCallback,
                  weak_ptr_factory_.GetWeakPtr()));
+  SendNotifyChangeForRequestIdList(request_ids);

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

I think that should be happening in the callback.

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

Done.

 }
 
 void RequestCoordinator::ResumeRequests(
@@ -139,12 +141,12 @@ void RequestCoordinator::ResumeRequests(
       base::Bind(&RequestCoordinator::UpdateMultipleRequestCallback,
                  weak_ptr_factory_.GetWeakPtr()));
   // TODO: Should we also schedule a task, in case there is not one scheduled?
+  SendNotifyChangeForRequestIdList(request_ids);

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

same

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

Done.

 }
 
 void RequestCoordinator::AddRequestResultCallback(
     RequestQueue::AddRequestResult result,
     const SavePageRequest& request) {
-
   // Inform the scheduler that we have an outstanding task..
   scheduler_->Schedule(GetTriggerConditionsForUserRequest());
 }
@@ -173,6 +175,27 @@ void RequestCoordinator::UpdateMultipleRequestCallback(
   }
 }
 
+// Called in response to updating multiple requests in the request queue.
+void RequestCoordinator::NotifyChangedListCallback(
+    const std::vector<int64_t> request_ids,
+    RequestQueue::GetRequestsResult result,
+    const std::vector<SavePageRequest>& requests) {
+  std::vector<ClientId> client_ids;
+
+  // For each item in request_ids, find it in requests and send a changed
+  // notification for that request.
+  for (const SavePageRequest& request : requests) {
+    // If we find a match, send a notification.
+    for (int64_t request_id : request_ids) {

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

given the structure of this loop, wouldn't std::unordered_set be better?

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

Done.

+      if (request_id == request.request_id()) {
+        NotifyChanged(request);
+        break;
+      }
+    }
+  }
+}
+
+
 void RequestCoordinator::StopProcessing() {
   is_canceled_ = true;
   if (offliner_ && is_busy_) {
@@ -318,19 +341,28 @@ void RequestCoordinator::OfflinerDoneCallback(const SavePageRequest& request,
                           base::Bind(&RequestCoordinator::UpdateRequestCallback,
                                      weak_ptr_factory_.GetWeakPtr(),
                                      updated_request.client_id()));
+    NotifyFailed(updated_request, static_cast<int64_t>(status));

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

If we are returning offliner status, let's not pass it as an int64_t.

I was thinking a general cause would be good as an alternative:
EXPIRATION, RETRY_COUNT, ...

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

Done.

 
-  } else if (status == Offliner::RequestStatus::SAVED ||
-             request.completed_attempt_count() + 1 >=
-                 policy_->GetMaxCompletedTries()) {
-    // Remove the request from the queue if it either succeeded or exceeded the
-    // max number of retries.  The +1 represents the request that just
-    // completed. Since we call MarkAttemptCompleted within the if branches,
-    // the completed_attempt_count has not yet been updated when we are checking
-    // the if condition.
+  } else if (status == Offliner::RequestStatus::SAVED) {
+    // Remove the request from the queue if it succeeded.
     queue_->RemoveRequest(
         request.request_id(),
         base::Bind(&RequestCoordinator::UpdateRequestCallback,
                    weak_ptr_factory_.GetWeakPtr(), request.client_id()));
+    // TODO: We don't have an offline ID to return.  Modify the prerenderer
+    // callback to pass it.
+    NotifySucceeded(request, 0);
+  } else if (request.completed_attempt_count() + 1 >=
+             policy_->GetMaxCompletedTries()) {
+    // Remove from the request queue if we exceeeded max retries. The +1
+    // represents the request that just completed. Since we call
+    // MarkAttemptCompleted within the if branches, the completed_attempt_count
+    // has not yet been updated when we are checking the if condition.
+    queue_->RemoveRequest(
+        request.request_id(),
+        base::Bind(&RequestCoordinator::UpdateRequestCallback,
+                   weak_ptr_factory_.GetWeakPtr(), request.client_id()));
+    NotifyFailed(request, static_cast<int64_t>(status));
   } else {
     // If we failed, but are not over the limit, update the request in the
     // queue.
@@ -340,6 +372,7 @@ void RequestCoordinator::OfflinerDoneCallback(const SavePageRequest& request,
                           base::Bind(&RequestCoordinator::UpdateRequestCallback,
                                      weak_ptr_factory_.GetWeakPtr(),
                                      updated_request.client_id()));
+    NotifyChanged(updated_request);
   }
 
   // Determine whether we might try another request in this
@@ -375,6 +408,42 @@ RequestCoordinator::GetTriggerConditionsForUserRequest() {
   return trigger_conditions;
 }
 
+void RequestCoordinator::AddObserver(Observer* observer) {
+  DCHECK(observer);
+  observers_.AddObserver(observer);
+}
+
+void RequestCoordinator::RemoveObserver(Observer* observer) {
+  observers_.RemoveObserver(observer);
+}
+
+void RequestCoordinator::NotifyAdded(const SavePageRequest& request) {
+  FOR_EACH_OBSERVER(Observer, observers_, OnAdded(request));
+}
+
+void RequestCoordinator::NotifySucceeded(
+    const SavePageRequest& request, int64_t offline_id) {
+  FOR_EACH_OBSERVER(Observer, observers_, OnSucceeded(request, offline_id));
+}
+
+void RequestCoordinator::NotifyFailed(
+    const SavePageRequest& request, int64_t failure_code) {
+  FOR_EACH_OBSERVER(Observer, observers_, OnFailed(request, failure_code));
+}
+
+void RequestCoordinator::NotifyChanged(const SavePageRequest& request) {
+  FOR_EACH_OBSERVER(Observer, observers_, OnChanged(request));
+}
+
+void RequestCoordinator::SendNotifyChangeForRequestIdList(
+    const std::vector<int64_t> request_ids) {
+  // Bind the request_ids onto our callback parameter list, the work will be
+  // done in the callback.
+  queue_->GetRequests(base::Bind(&RequestCoordinator::NotifyChangedListCallback,
+                                 weak_ptr_factory_.GetWeakPtr(),
+                                 request_ids));
+}
+
 void RequestCoordinator::GetOffliner() {
   if (!offliner_) {
     offliner_ = factory_->GetOffliner(policy_.get());