StackSamplingProfiler clean up

base/profiler/native_stack_sampler.h

+// Copyright 2015 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 BASE_PROFILER_NATIVE_STACK_SAMPLER_H_
+#define BASE_PROFILER_NATIVE_STACK_SAMPLER_H_
+
+#include "base/memory/scoped_ptr.h"
+#include "base/profiler/stack_sampling_profiler.h"
+#include "base/threading/platform_thread.h"
+
+namespace base {
+
+// NativeStackSampler is an implementation detail of StackSamplingProfiler. It
+// abstracts the native implementation required to record a stack sample for a
+// given thread.
+class StackSamplingProfiler::NativeStackSampler {
+ public:
+  virtual ~NativeStackSampler();
+
+  // Creates a stack sampler that records samples for |thread_handle|. Returns
+  // null if this platform does not support stack sampling.
+  static scoped_ptr<NativeStackSampler> Create(PlatformThreadId thread_id);
+
+  // Notifies the sampler that we're starting to record a new profile. This
+  // function is called on the thread used for sampling.

[Peter Kasting, 2015/03/30 23:07:34]

Nit: Perhaps put this above all these functions, then each can have a very short
descriptive comment:

// The following functions are all called by the StackSamplingProfiler on the
// thread used to create the sampler (not the thread being sampled).

This also clarifies who calls these functions, which is a bit vague currently.

[Mike Wittman, 2015/03/31 01:06:36]

Done, although revised the proposed wording to be strictly correct.

+  virtual void ProfileRecordingStarting(Profile* profile) = 0;

[Peter Kasting, 2015/03/30 23:07:34]

Incidentally, I'm worried that the use of "Profile" as a classname and a concept
in these files is confusing what with the pervasive use of "Profile" to mean
something completely different throughout the rest of the codebase.

It's beyond the scope of this review to tell you you need to change the name,
but give it some consideration -- it's certainly been continually confusing to
me.

[Mike Wittman, 2015/03/31 01:06:36]

That's reasonable. I can change the name to CallStackProfile to clearly
distinguish the types.

+
+  // Records a stack sample. This function is called on the thread used for
+  // sampling.
+  virtual void RecordStackSample(Sample* sample) = 0;
+
+  // Notifies the sampler that we've stopped recording the current
+  // profile. This function is called on the thread used for sampling.
+  virtual void ProfileRecordingStopped() = 0;
+
+ protected:
+  NativeStackSampler();
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(NativeStackSampler);
+};
+
+}  // namespace base
+
+#endif  // BASE_PROFILER_NATIVE_STACK_SAMPLER_H_
+