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

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 178 matching lines @@
   }
 
   return false;
 }
 
 void RequestCoordinator::AbortRequestAttempt(SavePageRequest* request) {
   request->MarkAttemptAborted();
   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,
         base::Bind(&RequestCoordinator::UpdateRequestCallback,
                    weak_ptr_factory_.GetWeakPtr(), request->client_id()));
   }
 }
 
 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(
     const std::vector<int64_t>& request_ids,
     const RemoveRequestsCallback& callback) {
   bool canceled = CancelActiveRequestIfItMatches(request_ids);
   queue_->RemoveRequests(
       request_ids,
       base::Bind(&RequestCoordinator::HandleRemovedRequestsAndCallback,
                  weak_ptr_factory_.GetWeakPtr(), callback,
@@ skipping 244 matching lines @@
   }
 }
 
 void RequestCoordinator::OfflinerDoneCallback(const SavePageRequest& request,
                                               Offliner::RequestStatus status) {
   DVLOG(2) << "offliner finished, saved: "
            << (status == Offliner::RequestStatus::SAVED)
            << ", 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();
 
   is_busy_ = false;
   active_request_.reset(nullptr);
 
   if (status == Offliner::RequestStatus::FOREGROUND_CANCELED ||
       status == Offliner::RequestStatus::PRERENDERING_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);
     AbortRequestAttempt(&updated_request);
     NotifyChanged(updated_request);
   } else if (status == Offliner::RequestStatus::SAVED) {
     // Remove the request from the queue if it succeeded.
     RemoveAttemptedRequest(request, BackgroundSavePageResult::SUCCESS);
   } else if (request.completed_attempt_count() + 1 >=
              policy_->GetMaxCompletedTries()) {
     // Remove from the request queue if we exceeded 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.
     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.
     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);
@@ skipping 53 matching lines @@
   FOR_EACH_OBSERVER(Observer, observers_, OnChanged(request));
 }
 
 void RequestCoordinator::GetOffliner() {
   if (!offliner_) {
     offliner_ = factory_->GetOffliner(policy_.get());
   }
 }
 
 }  // namespace offline_pages