Adds an observer for the request coordinator.

components/offline_pages/background/request_coordinator.cc

 // 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.
 
 #include "components/offline_pages/background/request_coordinator.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/callback.h"
@@ skipping 59 matching lines @@
 
 bool RequestCoordinator::SavePageLater(
     const GURL& url, const ClientId& client_id, bool user_requested) {
   DVLOG(2) << "URL is " << url << " " << __func__;
 
   if (!OfflinePageModel::CanSaveURL(url)) {
     DVLOG(1) << "Not able to save page for requested url: " << url;
     return false;
   }
 
-  // TODO(petewil): We need a robust scheme for allocating new IDs.
+  // TODO(petewil): Use an int64_t random number for this, pass it to the
+  // offliner, and eventually use it as the offline_id - that way the client
+  // uses offline_id everywhere.
   static int64_t id = 0;
 
   // Build a SavePageRequest.
   offline_pages::SavePageRequest request(
       id++, url, client_id, base::Time::Now(), user_requested);
 
   // Put the request on the request queue.
   queue_->AddRequest(request,
                      base::Bind(&RequestCoordinator::AddRequestResultCallback,
                                 weak_ptr_factory_.GetWeakPtr()));
+  NotifyAdded(request);
   return true;
 }
 void RequestCoordinator::GetQueuedRequests(
     const std::string& client_namespace,
     const QueuedRequestCallback& callback) {
   // Get all matching requests from the request queue, send them to our
   // callback.  We bind the namespace and callback to the front of the callback
   // param set.
   queue_->GetRequests(base::Bind(&RequestCoordinator::GetQueuedRequestsCallback,
                                  weak_ptr_factory_.GetWeakPtr(),
@@ skipping 11 matching lines @@
   for (const auto& request : requests) {
     if (client_namespace == request.client_id().name_space)
       client_ids.push_back(request.client_id());
   }
 
   callback.Run(client_ids);
 }
 
 void RequestCoordinator::RemoveRequests(
     const std::vector<ClientId>& client_ids) {
+  std::vector<int64_t> placeholder;
   queue_->RemoveRequestsByClientId(
       client_ids, base::Bind(&RequestCoordinator::UpdateMultipleRequestCallback,
-                             weak_ptr_factory_.GetWeakPtr()));
+                             weak_ptr_factory_.GetWeakPtr(), placeholder));
 }
 
 void RequestCoordinator::PauseRequests(
     const std::vector<int64_t>& request_ids) {
   queue_->ChangeRequestsState(
       request_ids, SavePageRequest::RequestState::PAUSED,
       base::Bind(&RequestCoordinator::UpdateMultipleRequestCallback,
-                 weak_ptr_factory_.GetWeakPtr()));
+                 weak_ptr_factory_.GetWeakPtr(), request_ids));
 }
 
 void RequestCoordinator::ResumeRequests(
     const std::vector<int64_t>& request_ids) {
   queue_->ChangeRequestsState(
       request_ids, SavePageRequest::RequestState::AVAILABLE,
       base::Bind(&RequestCoordinator::UpdateMultipleRequestCallback,
-                 weak_ptr_factory_.GetWeakPtr()));
+                 weak_ptr_factory_.GetWeakPtr(), request_ids));
   // TODO: Should we also schedule a task, in case there is not one scheduled?
 }
 
 void RequestCoordinator::AddRequestResultCallback(
     RequestQueue::AddRequestResult result,
     const SavePageRequest& request) {
-
   // Inform the scheduler that we have an outstanding task..
   scheduler_->Schedule(GetTriggerConditionsForUserRequest());
 }
 
 // Called in response to updating a request in the request queue.
 void RequestCoordinator::UpdateRequestCallback(
     const ClientId& client_id,
     RequestQueue::UpdateRequestResult result) {
   // If the request succeeded, nothing to do.  If it failed, we can't really do
   // much, so just log it.
   if (result != RequestQueue::UpdateRequestResult::SUCCESS) {
     DVLOG(1) << "Failed to update request attempt details. "
              << static_cast<int>(result);
     event_logger_.RecordUpdateRequestFailed(client_id.name_space, result);
   }
 }
 
 // Called in response to updating multiple requests in the request queue.
 void RequestCoordinator::UpdateMultipleRequestCallback(

[Dmitry Titov, 2016/08/11 03:41:55]

UpdateMultipleRequestCallback and SendNotifyChangeForRequestIdList can be merged
into a single method.

[Pete Williamson, 2016/08/13 03:20:34]

Done.

+    const std::vector<int64_t>& request_ids,
     RequestQueue::UpdateRequestResult result) {
   // If the request succeeded, nothing to do.  If it failed, we can't really do

[Dmitry Titov, 2016/08/11 03:41:55]

This comment seems needs updating, because "nothing to do" is not valid anymore.

[Pete Williamson, 2016/08/13 03:20:34]

Done.

   // much, so just log it.
   if (result != RequestQueue::UpdateRequestResult::SUCCESS) {
     DVLOG(1) << "Failed to update request attempt details. "
              << static_cast<int>(result);
+  } else {
+    SendNotifyChangeForRequestIdList(request_ids);
   }
 }
 
+// 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.
+  std::unordered_set<int64_t> changed_ids(request_ids.begin(),
+                                          request_ids.end());
+  for (const SavePageRequest& request : requests) {
+    // If we find a match, send a notification.
+    if (changed_ids.count(request.request_id()) > 0)
+      NotifyChanged(request);
+  }
+}
+
 void RequestCoordinator::StopProcessing() {
   is_canceled_ = true;
   if (offliner_ && is_busy_) {
     // TODO(dougarnett): Find current request and mark attempt aborted.
     offliner_->Cancel();
   }
 
   // Stopping offliner means it will not call callback.
   last_offlining_status_ =
       Offliner::RequestStatus::REQUEST_COORDINATOR_CANCELED;
@@ skipping 125 matching lines @@
   if (status == Offliner::RequestStatus::FOREGROUND_CANCELED) {
     // Update the request for the canceled attempt.
     // TODO(dougarnett): See if we can conclusively identify other attempt
     // aborted cases to treat this way (eg, for Render Process Killed).
     SavePageRequest updated_request(request);
     updated_request.MarkAttemptAborted();
     queue_->UpdateRequest(updated_request,
                           base::Bind(&RequestCoordinator::UpdateRequestCallback,
                                      weak_ptr_factory_.GetWeakPtr(),
                                      updated_request.client_id()));
+    NotifyFailed(updated_request, SavePageStatus::FOREGROUND_CANCELED);
 
-  } 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, SavePageStatus::RETRY_COUNT_EXCEEDED);
   } else {
     // If we failed, but are not over the limit, update the request in the
     // queue.
     SavePageRequest updated_request(request);
     updated_request.MarkAttemptCompleted();
     queue_->UpdateRequest(updated_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
   // processing window based on how the previous request completed.
   //
   // TODO(dougarnett): Need to split PRERENDERING_FAILED into separate
   // codes as to whether we should try another request or not.
   switch (status) {
     case Offliner::RequestStatus::SAVED:
     case Offliner::RequestStatus::SAVE_FAILED:
@@ skipping 15 matching lines @@
 
 const Scheduler::TriggerConditions
 RequestCoordinator::GetTriggerConditionsForUserRequest() {
   Scheduler::TriggerConditions trigger_conditions(
       policy_->PowerRequiredForUserRequestedPage(),
       policy_->BatteryPercentageRequiredForUserRequestedPage(),
       policy_->UnmeteredNetworkRequiredForUserRequestedPage());
   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,
+                                      SavePageStatus status) {
+  FOR_EACH_OBSERVER(Observer, observers_, OnFailed(request, status));
+}
+
+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,

[Dmitry Titov, 2016/08/11 03:41:55]

It is unfortunate that we need to go do the SQL query against a database just to
send notifications. In fact, there can be some requests there that are already
deleted from the database by the time we are fetching them. For example, in case
of RemoveRequests - are they first removed fmor DB, and then we try to fetch
them again to notify? Will it even work?

It would be better to keep the list of requests that were actually affected
passed through chain of callbacks and then notify based on that.

[Pete Williamson, 2016/08/13 03:20:34]

Done.

+                                 weak_ptr_factory_.GetWeakPtr(), request_ids));
+}
+
 void RequestCoordinator::GetOffliner() {
   if (!offliner_) {
     offliner_ = factory_->GetOffliner(policy_.get());
   }
 }
 
 }  // namespace offline_pages