[Offline Pages] Revises Background OfflinerRequestStatus histogram to have namespace suffix.

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"
 #include "components/offline_pages/offline_page_model.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.
-void RecordOfflinerResultUMA(Offliner::RequestStatus request_status) {
-  UMA_HISTOGRAM_ENUMERATION("OfflinePages.Background.OfflinerRequestStatus",
-                            request_status,
-                            Offliner::RequestStatus::STATUS_COUNT);
+void RecordOfflinerResultUMA(const ClientId& client_id,
+                             Offliner::RequestStatus request_status) {
+  // TODO(dougarnett): Consider exposing AddHistogramSuffix from
+  // offline_page_model_impl.cc as visible utility method.
+  std::string histogram_name("OfflinePages.Background.OfflinerRequestStatus");
+  if (!client_id.name_space.empty()) {
+    histogram_name += "." + client_id.name_space;
+  }
+
+  // The histogram below is an expansion of the UMA_HISTOGRAM_ENUMERATION
+  // 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));
 }
 
 }  // namespace
 
 RequestCoordinator::RequestCoordinator(std::unique_ptr<OfflinerPolicy> policy,
                                        std::unique_ptr<OfflinerFactory> factory,
                                        std::unique_ptr<RequestQueue> queue,
                                        std::unique_ptr<Scheduler> scheduler)
     : is_busy_(false),
       is_canceled_(false),
       offliner_(nullptr),
       policy_(std::move(policy)),
       factory_(std::move(factory)),
       queue_(std::move(queue)),
       scheduler_(std::move(scheduler)),
+      active_request_(nullptr),
       last_offlining_status_(Offliner::RequestStatus::UNKNOWN),
       offliner_timeout_(base::TimeDelta::FromSeconds(
           policy_->GetSinglePageTimeLimitInSeconds())),
       weak_ptr_factory_(this) {
   DCHECK(policy_ != nullptr);
   picker_.reset(new RequestPicker(queue_.get(), policy_.get()));
 }
 
 RequestCoordinator::~RequestCoordinator() {}
 
@@ skipping 88 matching lines @@
 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;
-  RecordOfflinerResultUMA(last_offlining_status_);
+
+  if (active_request_) {
+    RecordOfflinerResultUMA(active_request_->client_id(),
+                            last_offlining_status_);
+    active_request_.reset();
+  }
 
   // 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) {
@@ skipping 57 matching lines @@
 
   GetOffliner();
   if (!offliner_) {
     DVLOG(0) << "Unable to create Offliner. "
              << "Cannot background offline page.";
     return;
   }
 
   DCHECK(!is_busy_);
   is_busy_ = true;
+  active_request_.reset(new SavePageRequest(request));
 
   // Prepare an updated request to attempt.
   SavePageRequest updated_request(request);
   updated_request.MarkAttemptStarted(base::Time::Now());
 
   // Start the load and save process in the offliner (Async).
   if (offliner_->LoadAndSave(
           updated_request, base::Bind(&RequestCoordinator::OfflinerDoneCallback,
                                       weak_ptr_factory_.GetWeakPtr()))) {
     // Offliner accepted request so update it in the queue.
@@ skipping 15 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());
   last_offlining_status_ = status;
-  RecordOfflinerResultUMA(last_offlining_status_);
+  RecordOfflinerResultUMA(request.client_id(), last_offlining_status_);
   watchdog_timer_.Stop();
 
   is_busy_ = false;
+  active_request_.reset(nullptr);
 
   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(),
@@ skipping 51 matching lines @@
   return trigger_conditions;
 }
 
 void RequestCoordinator::GetOffliner() {
   if (!offliner_) {
     offliner_ = factory_->GetOffliner(policy_.get());
   }
 }
 
 }  // namespace offline_pages