[Offline Pages] Adds event logs for requests dropped due to number of start or complete attempts.

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 cddeedbf81b7d3e98bbc6d8881ff761295847556..05d9e1d1355f907286bc99e99b55ae1bbebd9cc1 100644
--- a/components/offline_pages/background/request_coordinator.cc
+++ b/components/offline_pages/background/request_coordinator.cc
@@ -196,6 +196,9 @@ void RequestCoordinator::AbortRequestAttempt(SavePageRequest* request) {
   if (request->started_attempt_count() >= policy_->GetMaxStartedTries()) {
     RemoveAttemptedRequest(*request,
                            BackgroundSavePageResult::START_COUNT_EXCEEDED);
+    event_logger_.RecordDroppedSavePageRequest(

[Pete Williamson, 2016/09/09 17:16:19]

This is logging to the offline-internals page, not UMA, if I understand
correctly.  Did we want UMA instead?

[Pete Williamson, 2016/09/09 22:08:31]

Per our conversation, this is OK as logging, not UMA, but let's open a bug to
add UMA for this.

[dougarnett, 2016/09/09 23:58:21]

Opened 645679 for UMA. The point of this event logging is for advanced user to
see why hir requests are dropped per policy settings. This can make it easier to
do manual evals with standard build.

+        request->client_id().name_space,
+        BackgroundSavePageResult::START_COUNT_EXCEEDED, request->request_id());
   } else {
     queue_->UpdateRequest(
         *request,
@@ -205,13 +208,14 @@ void RequestCoordinator::AbortRequestAttempt(SavePageRequest* request) {
 }
 
 void RequestCoordinator::RemoveAttemptedRequest(
-    const SavePageRequest& request, BackgroundSavePageResult status) {
+    const SavePageRequest& request,
+    BackgroundSavePageResult result) {
   std::vector<int64_t> remove_requests;
   remove_requests.push_back(request.request_id());
   queue_->RemoveRequests(remove_requests,
                          base::Bind(&RequestCoordinator::HandleRemovedRequests,
-                                    weak_ptr_factory_.GetWeakPtr(), status));
-  RecordAttemptCount(request, status);
+                                    weak_ptr_factory_.GetWeakPtr(), result));
+  RecordAttemptCount(request, result);
 }
 
 void RequestCoordinator::RemoveRequests(
@@ -476,8 +480,8 @@ void RequestCoordinator::OfflinerDoneCallback(const SavePageRequest& request,
            << ", status: " << static_cast<int>(status) << ", " << __func__;
   DCHECK_NE(status, Offliner::RequestStatus::UNKNOWN);
   DCHECK_NE(status, Offliner::RequestStatus::LOADED);
-  event_logger_.RecordSavePageRequestUpdated(request.client_id().name_space,
-                                             status, request.request_id());
+  event_logger_.RecordOfflinerResult(request.client_id().name_space, status,
+                                     request.request_id());
   last_offlining_status_ = status;
   RecordOfflinerResultUMA(request.client_id(), last_offlining_status_);
   watchdog_timer_.Stop();
@@ -504,6 +508,9 @@ void RequestCoordinator::OfflinerDoneCallback(const SavePageRequest& request,
     // has not yet been updated when we are checking the if condition.
     RemoveAttemptedRequest(request,
                            BackgroundSavePageResult::RETRY_COUNT_EXCEEDED);
+    event_logger_.RecordDroppedSavePageRequest(
+        request.client_id().name_space,
+        BackgroundSavePageResult::RETRY_COUNT_EXCEEDED, request.request_id());
   } else {
     // If we failed, but are not over the limit, update the request in the
     // queue.