PerfProvider: Restructure collection parameters into a struct

chrome/browser/metrics/perf/perf_provider_chromeos.h

Index: chrome/browser/metrics/perf/perf_provider_chromeos.h
diff --git a/chrome/browser/metrics/perf/perf_provider_chromeos.h b/chrome/browser/metrics/perf/perf_provider_chromeos.h
index 64b28d2330bbfb3f354df5942c47c6652d3f50bb..7de176bf5068ef66c017a9b96a2974d675c60117 100644
--- a/chrome/browser/metrics/perf/perf_provider_chromeos.h
+++ b/chrome/browser/metrics/perf/perf_provider_chromeos.h
@@ -6,6 +6,7 @@
 #define CHROME_BROWSER_METRICS_PERF_PERF_PROVIDER_CHROMEOS_H_
 
 #include <string>
+#include <type_traits>
 #include <vector>
 
 #include "base/basictypes.h"
@@ -35,6 +36,78 @@ class PerfProvider : public base::NonThreadSafe,
   bool GetSampledProfiles(std::vector<SampledProfile>* sampled_profiles);
 
  protected:
+  typedef int64 TimeDeltaInternalType;
+
+  class CollectionParams {
+   public:
+    class TriggerParams {
+     public:
+      TriggerParams(int64 sampling_factor,
+                    base::TimeDelta max_collection_delay);
+
+      // Limit the number of profiles collected.

[Alexei Svitkine (slow), 2015/10/09 19:24:32]

Nit: Put the comments above the member variables and you can leave the
getters/setters without comments. Same throughout.

[dhsharp, 2015/10/09 21:18:35]

Done.

+      int64 sampling_factor() const { return sampling_factor_; }
+      void set_sampling_factor(int64 factor) { sampling_factor_ = factor; }
+      // Add a random delay before collecting after the trigger.
+      // The delay should be randomly selected between 0 and this value.
+      base::TimeDelta max_collection_delay() const {
+        return base::TimeDelta::FromInternalValue(max_collection_delay_);
+      }
+      void set_max_collection_delay(base::TimeDelta delay) {
+        max_collection_delay_ = delay.ToInternalValue();
+      }

[Alexei Svitkine (slow), 2015/10/09 19:24:32]

Nit: Add an empty line below this.

[dhsharp, 2015/10/09 21:18:34]

Done.

+     private:
+      TriggerParams() = default;  // POD

[Alexei Svitkine (slow), 2015/10/09 19:24:32]

Nit: Add an empty line below this.

[dhsharp, 2015/10/09 21:18:34]

Done.

+      int64 sampling_factor_;

[Alexei Svitkine (slow), 2015/10/09 19:24:32]

Nit: In new code, int64_t is preferred.

[dhsharp, 2015/10/09 21:18:34]

Done.

+      TimeDeltaInternalType max_collection_delay_;
+    };
+
+    CollectionParams(base::TimeDelta collection_duration,
+                     base::TimeDelta periodic_interval,
+                     TriggerParams resume_from_suspend,
+                     TriggerParams restore_session);
+
+    // Time perf is run for.
+    base::TimeDelta collection_duration() const {
+      return base::TimeDelta::FromInternalValue(collection_duration_);
+    }
+    void set_collection_duration(base::TimeDelta duration) {
+      collection_duration_ = duration.ToInternalValue();
+    }
+
+    // For PERIODIC_COLLECTION, partition time since login into successive
+    // intervals of this duration. In each interval, a random time is picked to
+    // collect a profile.
+    base::TimeDelta periodic_interval() const {
+      return base::TimeDelta::FromInternalValue(periodic_interval_);
+    }
+    void set_periodic_interval(base::TimeDelta interval) {
+      periodic_interval_ = interval.ToInternalValue();
+    }
+
+    // Parameters for RESUME_FROM_SUSPEND and RESTORE_SESSION collections:
+    const TriggerParams& resume_from_suspend() const {
+      return resume_from_suspend_;
+    }
+    TriggerParams& mutable_resume_from_suspend() {
+      return resume_from_suspend_;
+    }
+    const TriggerParams& restore_session() const {
+      return restore_session_;
+    }
+    TriggerParams& mutable_restore_session() {

[Alexei Svitkine (slow), 2015/10/09 19:24:32]

Non-const refs are discouraged. This should return a pointer, as protos do.

However, given your current CL is not using these, suggest removing all of these
mutable/setters and re-adding them in the CL that actually integrations with
variations params.

[dhsharp, 2015/10/09 21:18:34]

D'oh, poor memory. You're right I was trying to emulate protos. Done.
That change is pretty substantial already. I think I'm doing you a favor by
leaving this in this change.

$ git show --stat cwp-finch
<snip>
 chrome/browser/metrics/perf/perf_provider_chromeos.cc          | 149
+++++++++++++++++++++++++++++++++++++++++++++++++++-
 chrome/browser/metrics/perf/perf_provider_chromeos.h           |  10 ++++
 chrome/browser/metrics/perf/perf_provider_chromeos_unittest.cc | 253
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 chrome/browser/metrics/perf/random_selector.cc                 |  16 +++++-
 chrome/browser/metrics/perf/random_selector.h                  |  18 +++++--
 chrome/browser/metrics/perf/random_selector_unittest.cc        |  42
++++++++++++++-
 6 files changed, 478 insertions(+), 10 deletions(-)

+      return restore_session_;
+    }
+
+   private:
+    CollectionParams() = default;  // POD
+
+    TimeDeltaInternalType collection_duration_;
+    TimeDeltaInternalType periodic_interval_;
+    TriggerParams resume_from_suspend_;
+    TriggerParams restore_session_;
+  };
+
   // Parses a PerfDataProto from serialized data |perf_data|, if it exists.
   // Parses a PerfStatProto from serialized data |perf_stat|, if it exists.
   // Only one of these may contain data. If both |perf_data| and |perf_stat|
@@ -111,6 +184,9 @@ class PerfProvider : public base::NonThreadSafe,
       const base::TimeDelta& time_after_restore,
       int num_tabs_restored);
 
+  // Parameters controlling how profiles are collected.
+  CollectionParams collection_params_;
+
   // Vector of SampledProfile protobufs containing perf profiles.
   std::vector<SampledProfile> cached_perf_data_;