Allow PolicyLoadStatusSample to override reporting method

components/policy/core/common/policy_load_status.h

 // Copyright (c) 2013 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.
 
 #ifndef COMPONENTS_POLICY_CORE_COMMON_POLICY_LOAD_STATUS_H_
 #define COMPONENTS_POLICY_CORE_COMMON_POLICY_LOAD_STATUS_H_
 
 #include <bitset>
 
+#include "base/callback.h"
 #include "base/macros.h"
 #include "components/policy/policy_export.h"
 
-namespace base {
-class HistogramBase;
-}
-
 namespace policy {
 
 // UMA histogram enum for policy load status. Don't change existing constants,
 // append additional constants to the end if needed.
 enum PolicyLoadStatus {
   // Policy load attempt started. This gets logged for each policy load attempt
   // to get a baseline on the number of requests, and an arbitrary number of
   // the below status codes may get added in addition.
   POLICY_LOAD_STATUS_STARTED = 0,
   // System failed to determine whether there's policy.
@@ skipping 15 matching lines @@
 
   // This must stay last.
   POLICY_LOAD_STATUS_SIZE
 };
 
 // A helper for generating policy load status UMA statistics that'll collect
 // histogram samples for a policy load operation and records histogram samples
 // for the status codes that were seen on destruction.
 class POLICY_EXPORT PolicyLoadStatusSample {
  public:
-  PolicyLoadStatusSample();
+  using ReportCallback = base::Callback<void(PolicyLoadStatus status)>;
+
+  // If |report_callback| is set, status is sent to this callback. Otherwise,

[emaxx, 2017/05/04 13:00:34]

To me this looks like stretching a single class for two quite different usages.
Also it's a bit worrying to have a generic callback that is being called from
the destructor (it's nontrivial to track down the execution flow; also thinking
about potential reentrancy issues).

Can this be separated into two different classes instead? One just for UMA, as
before, and another would be for your purposes. For instance, if you don't
really need the "automatically call callback on destruction" behavior, then this
can be boiled down to an interface with just Add() and GetAllStatuses().

[ljusten (tachyonic), 2017/05/05 08:16:27]

Thanks, I like that better.

+  // status is sent to a base::LinearHistogram with name |kHistogramName|. Set
+  // the callback in situations where base::LinearHistogram cannot be used, e.g.
+  // when called from Chromium OS code.
+  PolicyLoadStatusSample(
+      const ReportCallback& report_callback = ReportCallback());
   ~PolicyLoadStatusSample();
 
   // Adds a status code.
   void Add(PolicyLoadStatus status);
 
  private:
   std::bitset<POLICY_LOAD_STATUS_SIZE> status_bits_;
-  base::HistogramBase* histogram_;
+  ReportCallback report_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(PolicyLoadStatusSample);
 };
 
 }  // namespace policy
 
 #endif  // COMPONENTS_POLICY_CORE_COMMON_POLICY_LOAD_STATUS_H_