[Offline Pages] Adds UMA for number of started attempts and for network

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 27 matching lines @@
   // macro adapted to allow for a dynamically suffixed histogram name.
   // Note: The factory creates and owns the histogram.
   base::HistogramBase* histogram = base::LinearHistogram::FactoryGet(
       histogram_name, 1,
       static_cast<int>(Offliner::RequestStatus::STATUS_COUNT),
       static_cast<int>(Offliner::RequestStatus::STATUS_COUNT) + 1,
       base::HistogramBase::kUmaTargetedHistogramFlag);
   histogram->Add(static_cast<int>(request_status));
 }
 
+// Records the number of started attempts for completed requests (whether
+// successful or not).
+void RecordAttemptCount(const SavePageRequest& request,
+                        RequestNotifier::SavePageStatus status) {
+  if (status == RequestNotifier::SavePageStatus::SUCCESS) {
+    // TODO(dougarnett): Also record UMA for completed attempts here.
+    UMA_HISTOGRAM_CUSTOM_COUNTS(
+        "OfflinePages.Background.RequestSuccess.StartedAttemptCount",
+        request.started_attempt_count(), 1, 10, 11);
+  } else {
+    UMA_HISTOGRAM_CUSTOM_COUNTS(
+        "OfflinePages.Background.RequestFailure.StartedAttemptCount",
+        request.started_attempt_count(), 1, 10, 11);
+  }
+}
+
 // This should use the same algorithm as we use for OfflinePageItem, so the IDs
 // are similar.
 int64_t GenerateOfflineId() {
   return base::RandGenerator(std::numeric_limits<int64_t>::max()) + 1;
 }
 
 // In case we start processing from SavePageLater, we need a callback, but there
 // is nothing for it to do.
 void EmptySchedulerCallback(bool started) {}
 
@@ skipping 54 matching lines @@
 
 void RequestCoordinator::GetQueuedRequestsCallback(
     const GetRequestsCallback& callback,
     RequestQueue::GetRequestsResult result,
     const std::vector<SavePageRequest>& requests) {
   callback.Run(requests);
 }
 
 void RequestCoordinator::StopPrerendering() {
   if (offliner_ && is_busy_) {
+    DCHECK(active_request_.get());
     offliner_->Cancel();
-    // Find current request and mark attempt aborted.
-    active_request_->MarkAttemptAborted();
-    queue_->UpdateRequest(*(active_request_.get()),
-                          base::Bind(&RequestCoordinator::UpdateRequestCallback,
-                                     weak_ptr_factory_.GetWeakPtr(),
-                                     active_request_->client_id()));
+    AbortRequestAttempt(active_request_.get());
   }
 
   // Stopping offliner means it will not call callback.
   last_offlining_status_ =
       Offliner::RequestStatus::REQUEST_COORDINATOR_CANCELED;
 
   if (active_request_) {
     RecordOfflinerResultUMA(active_request_->client_id(),
                             last_offlining_status_);
     is_busy_ = false;
@@ skipping 11 matching lines @@
     if (request_ids.end() != std::find(request_ids.begin(), request_ids.end(),
                                        active_request_->request_id())) {
       StopPrerendering();
       return true;
     }
   }
 
   return false;
 }
 
+void RequestCoordinator::AbortRequestAttempt(SavePageRequest* request) {
+  request->MarkAttemptAborted();
+  if (request->started_attempt_count() >= policy_->GetMaxStartedTries()) {
+    RemoveAttemptedRequest(*request, SavePageStatus::START_COUNT_EXCEEDED);
+  } else {
+    queue_->UpdateRequest(
+        *request,
+        base::Bind(&RequestCoordinator::UpdateRequestCallback,
+                   weak_ptr_factory_.GetWeakPtr(), request->client_id()));
+  }
+}
+
+void RequestCoordinator::RemoveAttemptedRequest(const SavePageRequest& request,
+                                                SavePageStatus status) {
+  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);
+}
+
 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,
                  SavePageStatus::REMOVED));
   if (canceled)
@@ skipping 235 matching lines @@
 
   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);
-    updated_request.MarkAttemptAborted();
-    queue_->UpdateRequest(updated_request,
-                          base::Bind(&RequestCoordinator::UpdateRequestCallback,
-                                     weak_ptr_factory_.GetWeakPtr(),
-                                     updated_request.client_id()));
+    AbortRequestAttempt(&updated_request);
     SavePageStatus notify_status =
         (status == Offliner::RequestStatus::FOREGROUND_CANCELED)
             ? SavePageStatus::FOREGROUND_CANCELED
             : SavePageStatus::PRERENDER_CANCELED;
     NotifyCompleted(updated_request, notify_status);
-
   } else if (status == Offliner::RequestStatus::SAVED) {
     // Remove the request from the queue if it succeeded.
-    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(), SavePageStatus::SUCCESS));
+    RemoveAttemptedRequest(request, SavePageStatus::SUCCESS);
   } else if (request.completed_attempt_count() + 1 >=
              policy_->GetMaxCompletedTries()) {
-    // Remove from the request queue if we exceeeded max retries. The +1
+    // 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.
-    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(),
-                                    SavePageStatus::RETRY_COUNT_EXCEEDED));
+    RemoveAttemptedRequest(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);
@@ skipping 54 matching lines @@
   FOR_EACH_OBSERVER(Observer, observers_, OnChanged(request));
 }
 
 void RequestCoordinator::GetOffliner() {
   if (!offliner_) {
     offliner_ = factory_->GetOffliner(policy_.get());
   }
 }
 
 }  // namespace offline_pages