Adds UMA for PrerenderingOffliner request processing result status.

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"
 #include "base/logging.h"
+#include "base/metrics/histogram_macros.h"
 #include "base/time/time.h"
 #include "components/offline_pages/background/offliner_factory.h"
 #include "components/offline_pages/background/offliner_policy.h"
 #include "components/offline_pages/background/request_picker.h"
 #include "components/offline_pages/background/save_page_request.h"
 #include "components/offline_pages/offline_page_item.h"
 
 namespace offline_pages {
 
 namespace {
+
+// Records the final request status UMA for an offlining request. This should
+// only be called once per Offliner::LoadAndSave request.

[fgorski, 2016/07/12 22:17:06]

where is the comment about not reporting loaded or unknown status that we
discussed in comments on patch 1 (next to enum definition)?

Are they going to be reported in the end? Or was it missed?

Perhaps it makes sense to DCHECK as well

[dougarnett, 2016/07/12 23:07:59]

Ok, added comment about the two interim status codes in enum defn and added
DCHECKs in offliner->coordinator callback (as that is more central point for
coordinator not expecting them and also then not reporting them.

+void RecordOfflinerResultUMA(Offliner::RequestStatus request_status) {
+  UMA_HISTOGRAM_ENUMERATION("OfflinePages.Background.OfflinerRequestStatus",
+                            request_status,
+                            Offliner::RequestStatus::STATUS_COUNT);
+}
+
 // TODO(dougarnett/petewil): Move to Policy object. Also consider lower minimum
 // battery percentage once there is some processing time limits in place.
 const Scheduler::TriggerConditions kUserRequestTriggerConditions(
     false /* require_power_connected */,
     50 /* minimum_battery_percentage */,
     false /* require_unmetered_network */);
 
 // Timeout is 2.5 minutes based on the size of Marshmallow doze mode
 // maintenance window (3 minutes)
 // TODO(petewil): Find the optimal timeout based on data for 2G connections and
@@ skipping 53 matching lines @@
 
 // Called in response to updating a request in the request queue.
 void RequestCoordinator::UpdateRequestCallback(
     RequestQueue::UpdateRequestResult result) {}
 
 void RequestCoordinator::StopProcessing() {
   is_canceled_ = true;
   if (offliner_ && is_busy_)
     offliner_->Cancel();
 
+  // Stopping offliner means it will not call callback.
+  last_offlining_status_ =
+      Offliner::RequestStatus::REQUEST_COORDINATOR_CANCELED;
+  RecordOfflinerResultUMA(last_offlining_status_);
+
   // Let the scheduler know we are done processing.
   scheduler_callback_.Run(true);
 }
 
 // Returns true if the caller should expect a callback, false otherwise. For
 // instance, this would return false if a request is already in progress.
 bool RequestCoordinator::StartProcessing(
     const DeviceConditions& device_conditions,
     const base::Callback<void(bool)>& callback) {
   if (is_busy_) return false;
@@ skipping 60 matching lines @@
 void RequestCoordinator::OfflinerDoneCallback(const SavePageRequest& request,
                                               Offliner::RequestStatus status) {
   DVLOG(2) << "offliner finished, saved: "
            << (status == Offliner::RequestStatus::SAVED) << ", status: "
            << (int) status << ", " << __FUNCTION__;
   event_logger_.RecordSavePageRequestUpdated(
       request.client_id().name_space,
       "Saved",
       request.request_id());
   last_offlining_status_ = status;
+  RecordOfflinerResultUMA(last_offlining_status_);
   watchdog_timer_.Stop();
 
   is_busy_ = false;
 
   // If the request succeeded, remove it from the Queue and maybe schedule
   // another one.
   if (status == Offliner::RequestStatus::SAVED) {
     queue_->RemoveRequest(request.request_id(),
                           base::Bind(&RequestCoordinator::UpdateRequestCallback,
                                      weak_ptr_factory_.GetWeakPtr()));
@@ skipping 10 matching lines @@
   return kUserRequestTriggerConditions;
 }
 
 void RequestCoordinator::GetOffliner() {
   if (!offliner_) {
     offliner_ = factory_->GetOffliner(policy_.get());
   }
 }
 
 }  // namespace offline_pages