StackSamplingProfiler clean up

Refine interfaces, implementation, formatting and comments according to the C++
style guide and Chrome coding standards. Behavior is unaltered except for a few
edge cases and Win32 API error handling.

This change is done as part of the readability review process. Original review:
https://crrev.com/1016563004.

Review URL: https://codereview.chromium.org/1030923002

Cr-Commit-Position: refs/heads/master@{#324107}

base/BUILD.gn

@@ -379,6 +379,8 @@ component("base") {
     "power_monitor/power_observer.h",
     "profiler/alternate_timer.cc",
     "profiler/alternate_timer.h",
+    "profiler/native_stack_sampler.cc",
+    "profiler/native_stack_sampler.h",
     "profiler/scoped_profile.cc",
     "profiler/scoped_profile.h",
     "profiler/scoped_tracker.cc",

base/base.gypi

@@ -487,6 +487,8 @@
           'process/process_win.cc',
           'profiler/alternate_timer.cc',
           'profiler/alternate_timer.h',
+          'profiler/native_stack_sampler.cc',
+          'profiler/native_stack_sampler.h',
           'profiler/scoped_profile.cc',
           'profiler/scoped_profile.h',
           'profiler/scoped_tracker.cc',

base/profiler/native_stack_sampler.cc

+// 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.
+
+#include "base/profiler/native_stack_sampler.h"
+
+namespace base {
+
+NativeStackSampler::NativeStackSampler() {}
+
+NativeStackSampler::~NativeStackSampler() {}
+
+}  // namespace base

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 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);
+
+  // The following functions are all called on the SamplingThread (not the
+  // thread being sampled).
+
+  // Notifies the sampler that we're starting to record a new profile. Modules
+  // shared across samples in the profile should be recorded in |modules|.
+  virtual void ProfileRecordingStarting(
+      std::vector<StackSamplingProfiler::Module>* modules) = 0;
+
+  // Records a stack sample to |sample|.
+  virtual void RecordStackSample(StackSamplingProfiler::Sample* sample) = 0;
+
+  // Notifies the sampler that we've stopped recording the current
+  // profile.
+  virtual void ProfileRecordingStopped() = 0;
+
+ protected:
+  NativeStackSampler();
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(NativeStackSampler);
+};
+
+}  // namespace base
+
+#endif  // BASE_PROFILER_NATIVE_STACK_SAMPLER_H_
+

base/profiler/stack_sampling_profiler_posix.cc

@@ -2,12 +2,12 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "base/profiler/stack_sampling_profiler.h"
+#include "base/profiler/native_stack_sampler.h"
 
 namespace base {
 
-scoped_ptr<StackSamplingProfiler::NativeStackSampler>
-StackSamplingProfiler::NativeStackSampler::Create(PlatformThreadId thread_id) {
+scoped_ptr<NativeStackSampler> NativeStackSampler::Create(
+    PlatformThreadId thread_id) {
   return scoped_ptr<NativeStackSampler>();
 }
 

components/metrics/call_stack_profile_metrics_provider.cc

@@ -42,7 +42,7 @@ void CopySampleToProto(
     // A frame may not have a valid module. If so, we can't compute the
     // instruction pointer offset, and we don't want to send bare pointers, so
     // leave call_stack_entry empty.
-    if (frame.module_index < 0)
+    if (frame.module_index == StackSamplingProfiler::Frame::kUnknownModuleIndex)
       continue;
     int64 module_offset =
         reinterpret_cast<const char*>(frame.instruction_pointer) -
@@ -55,7 +55,7 @@ void CopySampleToProto(
 
 // Transcode |profile| into |proto_profile|.
 void CopyProfileToProto(
-    const StackSamplingProfiler::Profile& profile,
+    const StackSamplingProfiler::CallStackProfile& profile,
     CallStackProfile* proto_profile) {
   if (profile.samples.empty())
     return;
@@ -112,13 +112,13 @@ CallStackProfileMetricsProvider::~CallStackProfileMetricsProvider() {}
 
 void CallStackProfileMetricsProvider::ProvideGeneralMetrics(
     ChromeUserMetricsExtension* uma_proto) {
-  std::vector<StackSamplingProfiler::Profile> profiles;
+  std::vector<StackSamplingProfiler::CallStackProfile> profiles;
   if (!source_profiles_for_test_.empty())
     profiles.swap(source_profiles_for_test_);
   else
     StackSamplingProfiler::GetPendingProfiles(&profiles);
 
-  for (const StackSamplingProfiler::Profile& profile : profiles) {
+  for (const StackSamplingProfiler::CallStackProfile& profile : profiles) {
     CallStackProfile* call_stack_profile =
         uma_proto->add_sampled_profile()->mutable_call_stack_profile();
     CopyProfileToProto(profile, call_stack_profile);
@@ -126,7 +126,7 @@ void CallStackProfileMetricsProvider::ProvideGeneralMetrics(
 }
 
 void CallStackProfileMetricsProvider::SetSourceProfilesForTesting(
-    const std::vector<StackSamplingProfiler::Profile>& profiles) {
+    const std::vector<StackSamplingProfiler::CallStackProfile>& profiles) {
   source_profiles_for_test_ = profiles;
 }
 

components/metrics/call_stack_profile_metrics_provider.h

@@ -26,10 +26,12 @@ class CallStackProfileMetricsProvider : public MetricsProvider {
   // ProvideGeneralMetrics, rather than sourcing them from the
   // StackSamplingProfiler.
   void SetSourceProfilesForTesting(
-      const std::vector<base::StackSamplingProfiler::Profile>& profiles);
+      const std::vector<base::StackSamplingProfiler::CallStackProfile>&
+          profiles);
 
  private:
-  std::vector<base::StackSamplingProfiler::Profile> source_profiles_for_test_;
+  std::vector<base::StackSamplingProfiler::CallStackProfile>
+      source_profiles_for_test_;
 
   DISALLOW_COPY_AND_ASSIGN(CallStackProfileMetricsProvider);
 };

components/metrics/call_stack_profile_metrics_provider_unittest.cc

@@ -12,7 +12,7 @@
 using base::StackSamplingProfiler;
 using Frame = StackSamplingProfiler::Frame;
 using Module = StackSamplingProfiler::Module;
-using Profile = StackSamplingProfiler::Profile;
+using Profile = StackSamplingProfiler::CallStackProfile;
 using Sample = StackSamplingProfiler::Sample;
 
 namespace metrics {
@@ -201,7 +201,7 @@ TEST(CallStackProfileMetricsProviderTest, MultipleProfiles) {
                                       module_base_address), entry.address());
         ASSERT_TRUE(entry.has_module_id_index());
         EXPECT_EQ(profile_sample_frames[i][j][k].module_index,
-                  entry.module_id_index());
+                  static_cast<size_t>(entry.module_id_index()));
       }
     }
 
@@ -298,7 +298,8 @@ TEST(CallStackProfileMetricsProviderTest, RepeatedStacksUnordered) {
       EXPECT_EQ(static_cast<uint64>(instruction_pointer - module_base_address),
                 entry.address());
       ASSERT_TRUE(entry.has_module_id_index());
-      EXPECT_EQ(sample_frames[i][j].module_index, entry.module_id_index());
+      EXPECT_EQ(sample_frames[i][j].module_index,
+                static_cast<size_t>(entry.module_id_index()));
     }
   }
 }
@@ -374,7 +375,8 @@ TEST(CallStackProfileMetricsProviderTest, RepeatedStacksOrdered) {
       EXPECT_EQ(static_cast<uint64>(instruction_pointer - module_base_address),
                 entry.address());
       ASSERT_TRUE(entry.has_module_id_index());
-      EXPECT_EQ(sample_frames[i][j].module_index, entry.module_id_index());
+      EXPECT_EQ(sample_frames[i][j].module_index,
+                static_cast<size_t>(entry.module_id_index()));
     }
   }
 }
@@ -382,8 +384,8 @@ TEST(CallStackProfileMetricsProviderTest, RepeatedStacksOrdered) {
 
 // Checks that unknown modules produce an empty Entry.
 TEST(CallStackProfileMetricsProviderTest, UnknownModule) {
-  // -1 indicates an unknown module.
-  const Frame frame(reinterpret_cast<const void*>(0x1000), -1);
+  const Frame frame(reinterpret_cast<const void*>(0x1000),
+                    Frame::kUnknownModuleIndex);
 
   Profile profile;