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}

12 files changed, 503 insertions, 410 deletions

diff --git a/base/BUILD.gn b/base/BUILD.gn
index df06d24..8a942ee 100644
--- a/base/BUILD.gn
+++ b/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",
diff --git a/base/base.gypi b/base/base.gypi
index fd63f6d..d95062c 100644
--- a/base/base.gypi
+++ b/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',
diff --git a/base/profiler/native_stack_sampler.cc b/base/profiler/native_stack_sampler.cc
new file mode 100644
index 0000000..8b4731b
--- /dev/null
+++ b/base/profiler/native_stack_sampler.cc
@@ -0,0 +1,13 @@
+// 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
diff --git a/base/profiler/native_stack_sampler.h b/base/profiler/native_stack_sampler.h
new file mode 100644
index 0000000..bc170dc
--- /dev/null
+++ b/base/profiler/native_stack_sampler.h
@@ -0,0 +1,50 @@
+// 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_
+
diff --git a/base/profiler/stack_sampling_profiler.cc b/base/profiler/stack_sampling_profiler.cc
index f760507..317400b 100644
--- a/base/profiler/stack_sampling_profiler.cc
+++ b/base/profiler/stack_sampling_profiler.cc
@@ -9,33 +9,41 @@
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/memory/singleton.h"
+#include "base/profiler/native_stack_sampler.h"
 #include "base/synchronization/lock.h"
-#include "base/synchronization/waitable_event.h"
 #include "base/timer/elapsed_timer.h"
 
-template <typename T> struct DefaultSingletonTraits;
-
 namespace base {
 
+// PendingProfiles ------------------------------------------------------------
+
 namespace {
 
-// Thread-safe singleton class that stores collected profiles waiting to be
-// processed.
+// Thread-safe singleton class that stores collected call stack profiles waiting
+// to be processed.
 class PendingProfiles {
  public:
-  PendingProfiles();
   ~PendingProfiles();
 
   static PendingProfiles* GetInstance();
 
-  // Appends |profiles|. This function is thread safe.
-  void PutProfiles(const std::vector<StackSamplingProfiler::Profile>& profiles);
-  // Gets the pending profiles into *|profiles|. This function is thread safe.
-  void GetProfiles(std::vector<StackSamplingProfiler::Profile>* profiles);
+  // Appends |profiles| to |profiles_|. This function may be called on any
+  // thread.
+  void AppendProfiles(
+      const std::vector<StackSamplingProfiler::CallStackProfile>& profiles);
+
+  // Copies the pending profiles from |profiles_| into |profiles|, and clears
+  // |profiles_|. This function may be called on any thread.
+  void GetAndClearPendingProfiles(
+      std::vector<StackSamplingProfiler::CallStackProfile>* profiles);
 
  private:
+  friend struct DefaultSingletonTraits<PendingProfiles>;
+
+  PendingProfiles();
+
   Lock profiles_lock_;
-  std::vector<StackSamplingProfiler::Profile> profiles_;
+  std::vector<StackSamplingProfiler::CallStackProfile> profiles_;
 
   DISALLOW_COPY_AND_ASSIGN(PendingProfiles);
 };
@@ -49,21 +57,24 @@ PendingProfiles* PendingProfiles::GetInstance() {
   return Singleton<PendingProfiles>::get();
 }
 
-void PendingProfiles::PutProfiles(
-    const std::vector<StackSamplingProfiler::Profile>& profiles) {
+void PendingProfiles::AppendProfiles(
+    const std::vector<StackSamplingProfiler::CallStackProfile>& profiles) {
   AutoLock scoped_lock(profiles_lock_);
   profiles_.insert(profiles_.end(), profiles.begin(), profiles.end());
 }
 
-void PendingProfiles::GetProfiles(
-    std::vector<StackSamplingProfiler::Profile>* profiles) {
+void PendingProfiles::GetAndClearPendingProfiles(
+    std::vector<StackSamplingProfiler::CallStackProfile>* profiles) {
   profiles->clear();
 
   AutoLock scoped_lock(profiles_lock_);
   profiles_.swap(*profiles);
 }
+
 }  // namespace
 
+// StackSamplingProfiler::Module ----------------------------------------------
+
 StackSamplingProfiler::Module::Module() : base_address(nullptr) {}
 StackSamplingProfiler::Module::Module(const void* base_address,
                                       const std::string& id,
@@ -72,60 +83,28 @@ StackSamplingProfiler::Module::Module(const void* base_address,
 
 StackSamplingProfiler::Module::~Module() {}
 
-StackSamplingProfiler::Frame::Frame()
-    : instruction_pointer(nullptr),
-      module_index(-1) {}
+// StackSamplingProfiler::Frame -----------------------------------------------
 
 StackSamplingProfiler::Frame::Frame(const void* instruction_pointer,
-                                    int module_index)
+                                    size_t module_index)
     : instruction_pointer(instruction_pointer),
       module_index(module_index) {}
 
 StackSamplingProfiler::Frame::~Frame() {}
 
-StackSamplingProfiler::Profile::Profile() : preserve_sample_ordering(false) {}
-
-StackSamplingProfiler::Profile::~Profile() {}
-
-class StackSamplingProfiler::SamplingThread : public PlatformThread::Delegate {
- public:
-  // Samples stacks using |native_sampler|. When complete, invokes
-  // |profiles_callback| with the collected profiles. |profiles_callback| must
-  // be thread-safe and may consume the contents of the vector.
-  SamplingThread(
-      scoped_ptr<NativeStackSampler> native_sampler,
-      const SamplingParams& params,
-      Callback<void(const std::vector<Profile>&)> completed_callback);
-  ~SamplingThread() override;
-
-  // Implementation of PlatformThread::Delegate:
-  void ThreadMain() override;
-
-  void Stop();
-
- private:
-  // Collects a profile from a single burst. Returns true if the profile was
-  // collected, or false if collection was stopped before it completed.
-  bool CollectProfile(Profile* profile, TimeDelta* elapsed_time);
-  // Collects profiles from all bursts, or until the sampling is stopped. If
-  // stopped before complete, |profiles| will contains only full bursts.
-  void CollectProfiles(std::vector<Profile>* profiles);
-
-  scoped_ptr<NativeStackSampler> native_sampler_;
-
-  const SamplingParams params_;
+// StackSamplingProfiler::CallStackProfile ------------------------------------
 
-  WaitableEvent stop_event_;
+StackSamplingProfiler::CallStackProfile::CallStackProfile()
+    : preserve_sample_ordering(false) {}
 
-  Callback<void(const std::vector<Profile>&)> completed_callback_;
+StackSamplingProfiler::CallStackProfile::~CallStackProfile() {}
 
-  DISALLOW_COPY_AND_ASSIGN(SamplingThread);
-};
+// StackSamplingProfiler::SamplingThread --------------------------------------
 
 StackSamplingProfiler::SamplingThread::SamplingThread(
     scoped_ptr<NativeStackSampler> native_sampler,
     const SamplingParams& params,
-    Callback<void(const std::vector<Profile>&)> completed_callback)
+    CompletedCallback completed_callback)
     : native_sampler_(native_sampler.Pass()),
       params_(params),
       stop_event_(false, false),
@@ -137,61 +116,77 @@ StackSamplingProfiler::SamplingThread::~SamplingThread() {}
 void StackSamplingProfiler::SamplingThread::ThreadMain() {
   PlatformThread::SetName("Chrome_SamplingProfilerThread");
 
-  std::vector<Profile> profiles;
+  CallStackProfiles profiles;
   CollectProfiles(&profiles);
   completed_callback_.Run(profiles);
 }
 
+// Depending on how long the sampling takes and the length of the sampling
+// interval, a burst of samples could take arbitrarily longer than
+// samples_per_burst * sampling_interval. In this case, we (somewhat
+// arbitrarily) honor the number of samples requested rather than strictly
+// adhering to the sampling intervals. Once we have established users for the
+// StackSamplingProfiler and the collected data to judge, we may go the other
+// way or make this behavior configurable.
 bool StackSamplingProfiler::SamplingThread::CollectProfile(
-    Profile* profile,
+    CallStackProfile* profile,
     TimeDelta* elapsed_time) {
   ElapsedTimer profile_timer;
-  Profile current_profile;
-  native_sampler_->ProfileRecordingStarting(&current_profile);
+  CallStackProfile current_profile;
+  native_sampler_->ProfileRecordingStarting(&current_profile.modules);
   current_profile.sampling_period = params_.sampling_interval;
-  bool stopped_early = false;
+  bool burst_completed = true;
+  TimeDelta previous_elapsed_sample_time;
   for (int i = 0; i < params_.samples_per_burst; ++i) {
-    ElapsedTimer sample_timer;
-    current_profile.samples.push_back(Sample());
-    native_sampler_->RecordStackSample(&current_profile.samples.back());
-    TimeDelta elapsed_sample_time = sample_timer.Elapsed();
-    if (i != params_.samples_per_burst - 1) {
+    if (i != 0) {
+      // Always wait, even if for 0 seconds, so we can observe a signal on
+      // stop_event_.
       if (stop_event_.TimedWait(
-              std::max(params_.sampling_interval - elapsed_sample_time,
+              std::max(params_.sampling_interval - previous_elapsed_sample_time,
                        TimeDelta()))) {
-        stopped_early = true;
+        burst_completed = false;
         break;
       }
     }
+    ElapsedTimer sample_timer;
+    current_profile.samples.push_back(Sample());
+    native_sampler_->RecordStackSample(&current_profile.samples.back());
+    previous_elapsed_sample_time = sample_timer.Elapsed();
   }
 
   *elapsed_time = profile_timer.Elapsed();
   current_profile.profile_duration = *elapsed_time;
   native_sampler_->ProfileRecordingStopped();
 
-  if (!stopped_early)
+  if (burst_completed)
     *profile = current_profile;
 
-  return !stopped_early;
+  return burst_completed;
 }
 
+// In an analogous manner to CollectProfile() and samples exceeding the expected
+// total sampling time, bursts may also exceed the burst_interval. We adopt the
+// same wait-and-see approach here.
 void StackSamplingProfiler::SamplingThread::CollectProfiles(
-    std::vector<Profile>* profiles) {
+    CallStackProfiles* profiles) {
   if (stop_event_.TimedWait(params_.initial_delay))
     return;
 
+  TimeDelta previous_elapsed_profile_time;
   for (int i = 0; i < params_.bursts; ++i) {
-    Profile profile;
-    TimeDelta elapsed_profile_time;
-    if (CollectProfile(&profile, &elapsed_profile_time))
-      profiles->push_back(profile);
-    else
-      return;
+    if (i != 0) {
+      // Always wait, even if for 0 seconds, so we can observe a signal on
+      // stop_event_.
+      if (stop_event_.TimedWait(
+              std::max(params_.burst_interval - previous_elapsed_profile_time,
+                       TimeDelta())))
+        return;
+    }
 
-    if (stop_event_.TimedWait(
-            std::max(params_.burst_interval - elapsed_profile_time,
-                     TimeDelta())))
+    CallStackProfile profile;
+    if (!CollectProfile(&profile, &previous_elapsed_profile_time))
       return;
+    profiles->push_back(profile);
   }
 }
 
@@ -199,14 +194,7 @@ void StackSamplingProfiler::SamplingThread::Stop() {
   stop_event_.Signal();
 }
 
-void StackSamplingProfiler::SamplingThreadDeleter::operator()(
-    SamplingThread* thread) const {
-  delete thread;
-}
-
-StackSamplingProfiler::NativeStackSampler::NativeStackSampler() {}
-
-StackSamplingProfiler::NativeStackSampler::~NativeStackSampler() {}
+// StackSamplingProfiler ------------------------------------------------------
 
 StackSamplingProfiler::SamplingParams::SamplingParams()
     : initial_delay(TimeDelta::FromMilliseconds(0)),
@@ -224,19 +212,20 @@ StackSamplingProfiler::StackSamplingProfiler(PlatformThreadId thread_id,
 StackSamplingProfiler::~StackSamplingProfiler() {}
 
 void StackSamplingProfiler::Start() {
-  native_sampler_ = NativeStackSampler::Create(thread_id_);
-  if (!native_sampler_)
+  scoped_ptr<NativeStackSampler> native_sampler =
+      NativeStackSampler::Create(thread_id_);
+  if (!native_sampler)
     return;
 
   sampling_thread_.reset(
       new SamplingThread(
-          native_sampler_.Pass(), params_,
+          native_sampler.Pass(), params_,
           (custom_completed_callback_.is_null() ?
-           Bind(&PendingProfiles::PutProfiles,
+           Bind(&PendingProfiles::AppendProfiles,
                 Unretained(PendingProfiles::GetInstance())) :
            custom_completed_callback_)));
   if (!PlatformThread::CreateNonJoinable(0, sampling_thread_.get()))
-    LOG(ERROR) << "failed to create thread";
+    sampling_thread_.reset();
 }
 
 void StackSamplingProfiler::Stop() {
@@ -245,14 +234,11 @@ void StackSamplingProfiler::Stop() {
 }
 
 // static
-void StackSamplingProfiler::GetPendingProfiles(std::vector<Profile>* profiles) {
-  PendingProfiles::GetInstance()->GetProfiles(profiles);
+void StackSamplingProfiler::GetPendingProfiles(CallStackProfiles* profiles) {
+  PendingProfiles::GetInstance()->GetAndClearPendingProfiles(profiles);
 }
 
-void StackSamplingProfiler::SetCustomCompletedCallback(
-    Callback<void(const std::vector<Profile>&)> callback) {
-  custom_completed_callback_ = callback;
-}
+// StackSamplingProfiler::Frame global functions ------------------------------
 
 bool operator==(const StackSamplingProfiler::Frame &a,
                 const StackSamplingProfiler::Frame &b) {
diff --git a/base/profiler/stack_sampling_profiler.h b/base/profiler/stack_sampling_profiler.h
index 4438786..bea266d 100644
--- a/base/profiler/stack_sampling_profiler.h
+++ b/base/profiler/stack_sampling_profiler.h
@@ -13,17 +13,20 @@
 #include "base/files/file_path.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/strings/string16.h"
+#include "base/synchronization/waitable_event.h"
 #include "base/threading/platform_thread.h"
 #include "base/time/time.h"
 
 namespace base {
 
+class NativeStackSampler;
+
 // StackSamplingProfiler periodically stops a thread to sample its stack, for
 // the purpose of collecting information about which code paths are
 // executing. This information is used in aggregate by UMA to identify hot
 // and/or janky code paths.
 //
-// Sample StackStackSamplingProfiler usage:
+// Sample StackSamplingProfiler usage:
 //
 //   // Create and customize params as desired.
 //   base::StackStackSamplingProfiler::SamplingParams params;
@@ -31,9 +34,9 @@ namespace base {
 //   base::StackSamplingProfiler profiler(base::PlatformThread::CurrentId()),
 //       params);
 //
-//   // To process the profiles within Chrome rather than via UMA, set a custom
-//   // completed callback:
-//   base::Callback<void(const std::vector<Profile>&)>
+//   // To process the call stack profiles within Chrome rather than via UMA,
+//   // set a custom completed callback:
+//   base::StackStackSamplingProfiler::CompletedCallback
 //       thread_safe_callback = ...;
 //   profiler.SetCustomCompletedCallback(thread_safe_callback);
 //
@@ -41,13 +44,24 @@ namespace base {
 //   // ... work being done on the target thread here ...
 //   profiler.Stop();  // optional, stops collection before complete per params
 //
-// When all profiles are complete or the profiler is stopped, if the custom
-// completed callback was set it will be called from the profiler thread with
-// the completed profiles. If no callback was set, the profiles are stored
-// internally and retrieved for UMA through
-// GetPendingProfiles(). GetPendingProfiles() should never be called by other
-// code; to retrieve profiles for in-process processing, set a completed
-// callback.
+// The default SamplingParams causes stacks to be recorded in a single burst at
+// a 10Hz interval for a total of 30 seconds. All of these parameters may be
+// altered as desired.
+//
+// When all call stack profiles are complete or the profiler is stopped, if the
+// custom completed callback was set it is called from the profiler thread with
+// the completed profiles. A profile is considered complete if all requested
+// samples were recorded for the profile (i.e. it was not stopped
+// prematurely). If no callback was set, the completed profiles are stored
+// internally and retrieved for UMA through GetPendingProfiles().
+// GetPendingProfiles() should never be called by other code; to retrieve
+// profiles for in-process processing, set a completed callback.
+//
+// The results of the profiling are passed to the completed callback and consist
+// of a vector of CallStackProfiles. Each CallStackProfile corresponds to a
+// burst as specified in SamplingParams and contains a set of Samples and
+// Modules. One Sample corresponds to a single recorded stack, and the Modules
+// record those modules associated with the recorded stack frames.
 class BASE_EXPORT StackSamplingProfiler {
  public:
   // Module represents the module (DLL or exe) corresponding to a stack frame.
@@ -59,6 +73,7 @@ class BASE_EXPORT StackSamplingProfiler {
 
     // Points to the base address of the module.
     const void* base_address;
+
     // An opaque binary string that uniquely identifies a particular program
     // version with high probability. This is parsed from headers of the loaded
     // module.
@@ -67,69 +82,50 @@ class BASE_EXPORT StackSamplingProfiler {
     // On Windows:
     //   GUID + AGE in the debug image headers of a module.
     std::string id;
+
     // The filename of the module.
     FilePath filename;
   };
 
   // Frame represents an individual sampled stack frame with module information.
   struct BASE_EXPORT Frame {
-    Frame();
-    Frame(const void* instruction_pointer, int module_index);
+    // Identifies an unknown module.
+    static const size_t kUnknownModuleIndex = static_cast<size_t>(-1);
+
+    Frame(const void* instruction_pointer, size_t module_index);
     ~Frame();
 
     // The sampled instruction pointer within the function.
     const void* instruction_pointer;
-    // Index of the module in the array of modules. We don't represent module
-    // state directly here to save space.
-    int module_index;
+
+    // Index of the module in CallStackProfile::modules. We don't represent
+    // module state directly here to save space.
+    size_t module_index;
   };
 
   // Sample represents a set of stack frames.
   using Sample = std::vector<Frame>;
 
-  // Profile represents a set of samples.
-  struct BASE_EXPORT Profile {
-    Profile();
-    ~Profile();
+  // CallStackProfile represents a set of samples.
+  struct BASE_EXPORT CallStackProfile {
+    CallStackProfile();
+    ~CallStackProfile();
 
     std::vector<Module> modules;
     std::vector<Sample> samples;
+
     // Duration of this profile.
     TimeDelta profile_duration;
+
     // Time between samples.
     TimeDelta sampling_period;
+
     // True if sample ordering is important and should be preserved if and when
     // this profile is compressed and processed.
     bool preserve_sample_ordering;
   };
 
-  // NativeStackSampler abstracts the native implementation required to record a
-  // stack sample for a given thread.
-  class NativeStackSampler {
-   public:
-    virtual ~NativeStackSampler();
-
-    // Create 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);
-
-    // Notify the sampler that we're starting to record a new profile. This
-    // function is called on the SamplingThread.
-    virtual void ProfileRecordingStarting(Profile* profile) = 0;
-
-    // Record a stack sample. This function is called on the SamplingThread.
-    virtual void RecordStackSample(Sample* sample) = 0;
-
-    // Notify the sampler that we've stopped recording the current profile. This
-    // function is called on the SamplingThread.
-    virtual void ProfileRecordingStopped() = 0;
-
-   protected:
-    NativeStackSampler();
-
-   private:
-    DISALLOW_COPY_AND_ASSIGN(NativeStackSampler);
-  };
+  using CallStackProfiles = std::vector<CallStackProfile>;
 
   // Represents parameters that configure the sampling.
   struct BASE_EXPORT SamplingParams {
@@ -137,51 +133,102 @@ class BASE_EXPORT StackSamplingProfiler {
 
     // Time to delay before first samples are taken. Defaults to 0.
     TimeDelta initial_delay;
+
     // Number of sampling bursts to perform. Defaults to 1.
     int bursts;
+
     // Interval between sampling bursts. This is the desired duration from the
     // start of one burst to the start of the next burst. Defaults to 10s.
     TimeDelta burst_interval;
+
     // Number of samples to record per burst. Defaults to 300.
     int samples_per_burst;
+
     // Interval between samples during a sampling burst. This is the desired
-    // duration from the start of one burst to the start of the next
-    // burst. Defaults to 100ms.
+    // duration from the start of one sample to the start of the next
+    // sample. Defaults to 100ms.
     TimeDelta sampling_interval;
+
     // True if sample ordering is important and should be preserved if and when
     // this profile is compressed and processed. Defaults to false.
     bool preserve_sample_ordering;
   };
 
+  // The callback type used to collect completed profiles.
+  //
+  // IMPORTANT NOTE: the callback is invoked on a thread the profiler
+  // constructs, rather than on the thread used to construct the profiler and
+  // set the callback, and thus the callback must be callable on any thread. For
+  // threads with message loops that create StackSamplingProfilers, posting a
+  // task to the message loop with a copy of the profiles is the recommended
+  // thread-safe callback implementation.
+  using CompletedCallback = Callback<void(const CallStackProfiles&)>;
+
   StackSamplingProfiler(PlatformThreadId thread_id,
                         const SamplingParams& params);
   ~StackSamplingProfiler();
 
   // Initializes the profiler and starts sampling.
   void Start();
+
   // Stops the profiler and any ongoing sampling. Calling this function is
-  // optional; if not invoked profiling will terminate when all the profiling
-  // bursts specified in the SamplingParams are completed.
+  // optional; if not invoked profiling terminates when all the profiling bursts
+  // specified in the SamplingParams are completed.
   void Stop();
 
-  // Gets the pending profiles into *|profiles| and clears the internal
-  // storage. This function is thread safe.
+  // Moves all pending call stack profiles from internal storage to
+  // |profiles|. This function is thread safe.
   //
   // ***This is intended for use only by UMA.*** Callers who want to process the
   // collected profiles should use SetCustomCompletedCallback.
-  static void GetPendingProfiles(std::vector<Profile>* profiles);
+  static void GetPendingProfiles(CallStackProfiles* call_stack_profiles);
 
-  // By default, collected profiles are stored internally and can be retrieved
-  // by GetPendingProfiles. If a callback is provided via this function,
-  // however, it will be called with the collected profiles instead. Note that
-  // this call to the callback occurs *on the profiler thread*.
-  void SetCustomCompletedCallback(
-      Callback<void(const std::vector<Profile>&)> callback);
+  // By default, collected call stack profiles are stored internally and can be
+  // retrieved by GetPendingProfiles. If a callback is provided via this
+  // function, however, it is called with the collected profiles instead.
+  void set_custom_completed_callback(CompletedCallback callback) {
+    custom_completed_callback_ = callback;
+  }
 
  private:
-  class SamplingThread;
-  struct SamplingThreadDeleter {
-    void operator() (SamplingThread* thread) const;
+  // SamplingThread is a separate thread used to suspend and sample stacks from
+  // the target thread.
+  class SamplingThread : public PlatformThread::Delegate {
+   public:
+    // Samples stacks using |native_sampler|. When complete, invokes
+    // |completed_callback| with the collected call stack profiles.
+    // |completed_callback| must be callable on any thread.
+    SamplingThread(scoped_ptr<NativeStackSampler> native_sampler,
+                   const SamplingParams& params,
+                   CompletedCallback completed_callback);
+    ~SamplingThread() override;
+
+    // PlatformThread::Delegate:
+    void ThreadMain() override;
+
+    void Stop();
+
+   private:
+    // Collects a call stack profile from a single burst. Returns true if the
+    // profile was collected, or false if collection was stopped before it
+    // completed.
+    bool CollectProfile(CallStackProfile* profile, TimeDelta* elapsed_time);
+
+    // Collects call stack profiles from all bursts, or until the sampling is
+    // stopped. If stopped before complete, |call_stack_profiles| will contain
+    // only full bursts.
+    void CollectProfiles(CallStackProfiles* profiles);
+
+    scoped_ptr<NativeStackSampler> native_sampler_;
+    const SamplingParams params_;
+
+    // If Stop() is called, it signals this event to force the sampling to
+    // terminate before all the samples specified in |params_| are collected.
+    WaitableEvent stop_event_;
+
+    CompletedCallback completed_callback_;
+
+    DISALLOW_COPY_AND_ASSIGN(SamplingThread);
   };
 
   // The thread whose stack will be sampled.
@@ -189,18 +236,17 @@ class BASE_EXPORT StackSamplingProfiler {
 
   const SamplingParams params_;
 
-  scoped_ptr<SamplingThread, SamplingThreadDeleter> sampling_thread_;
-  scoped_ptr<NativeStackSampler> native_sampler_;
+  scoped_ptr<SamplingThread> sampling_thread_;
 
-  Callback<void(const std::vector<Profile>&)> custom_completed_callback_;
+  CompletedCallback custom_completed_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(StackSamplingProfiler);
 };
 
-// Defined to allow equality check of Samples.
+// The metrics provider code wants to put Samples in a map and compare them,
+// which requires us to define a few operators.
 BASE_EXPORT bool operator==(const StackSamplingProfiler::Frame& a,
                             const StackSamplingProfiler::Frame& b);
-// Defined to allow ordering of Samples.
 BASE_EXPORT bool operator<(const StackSamplingProfiler::Frame& a,
                            const StackSamplingProfiler::Frame& b);
 
diff --git a/base/profiler/stack_sampling_profiler_posix.cc b/base/profiler/stack_sampling_profiler_posix.cc
index 6a44d7e..bce37e1 100644
--- a/base/profiler/stack_sampling_profiler_posix.cc
+++ b/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>();
 }
 
diff --git a/base/profiler/stack_sampling_profiler_unittest.cc b/base/profiler/stack_sampling_profiler_unittest.cc
index ad9e926..5bfc2d4 100644
--- a/base/profiler/stack_sampling_profiler_unittest.cc
+++ b/base/profiler/stack_sampling_profiler_unittest.cc
@@ -2,12 +2,11 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include <sstream>
-
 #include "base/bind.h"
 #include "base/compiler_specific.h"
 #include "base/path_service.h"
 #include "base/profiler/stack_sampling_profiler.h"
+#include "base/strings/stringprintf.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/threading/platform_thread.h"
 #include "base/time/time.h"
@@ -18,27 +17,32 @@ namespace base {
 using Frame = StackSamplingProfiler::Frame;
 using Module = StackSamplingProfiler::Module;
 using Sample = StackSamplingProfiler::Sample;
-using Profile = StackSamplingProfiler::Profile;
+using CallStackProfile = StackSamplingProfiler::CallStackProfile;
 
 namespace {
+
 // A thread to target for profiling, whose stack is guaranteed to contain
 // SignalAndWaitUntilSignaled() when coordinated with the main thread.
 class TargetThread : public PlatformThread::Delegate {
  public:
   TargetThread();
 
-  // Implementation of PlatformThread::Delegate:
+  // PlatformThread::Delegate:
   void ThreadMain() override;
 
-  // Wait for the thread to have started and be executing in
+  // Waits for the thread to have started and be executing in
   // SignalAndWaitUntilSignaled().
   void WaitForThreadStart();
-  // Allow the thread to return from SignalAndWaitUntilSignaled() and finish
+
+  // Allows the thread to return from SignalAndWaitUntilSignaled() and finish
   // execution.
   void SignalThreadToFinish();
 
   // This function is guaranteed to be executing between calls to
-  // WaitForThreadStart() and SignalThreadToFinish().
+  // WaitForThreadStart() and SignalThreadToFinish(). This function is static so
+  // that we can get a straightforward address for it in one of the tests below,
+  // rather than dealing with the complexity of a member function pointer
+  // representation.
   static void SignalAndWaitUntilSignaled(WaitableEvent* thread_started_event,
                                          WaitableEvent* finish_event);
 
@@ -84,18 +88,19 @@ NOINLINE void TargetThread::SignalAndWaitUntilSignaled(
 // Called on the profiler thread when complete. Collects profiles produced by
 // the profiler, and signals an event to allow the main thread to know that that
 // the profiler is done.
-void SaveProfilesAndSignalEvent(std::vector<Profile>* profiles,
-                                WaitableEvent* event,
-                                const std::vector<Profile>& pending_profiles) {
+void SaveProfilesAndSignalEvent(
+    std::vector<CallStackProfile>* profiles,
+    WaitableEvent* event,
+    const std::vector<CallStackProfile>& pending_profiles) {
   *profiles = pending_profiles;
   event->Signal();
 }
 
-// Captures profiles as specified by |params| on the TargetThread, and returns
-// them in |profiles|. Waits up to |profiler_wait_time| for the profiler to
-// complete.
+// Captures call stack profiles as specified by |params| on the TargetThread,
+// and returns them in |profiles|. Waits up to |profiler_wait_time| for the
+// profiler to complete.
 void CaptureProfiles(const StackSamplingProfiler::SamplingParams& params,
-                     std::vector<Profile>* profiles,
+                     std::vector<CallStackProfile>* profiles,
                      TimeDelta profiler_wait_time) {
   TargetThread target_thread;
   PlatformThreadHandle target_thread_handle;
@@ -106,7 +111,7 @@ void CaptureProfiles(const StackSamplingProfiler::SamplingParams& params,
   WaitableEvent sampling_thread_completed(true, false);
   profiles->clear();
   StackSamplingProfiler profiler(target_thread.id(), params);
-  profiler.SetCustomCompletedCallback(
+  profiler.set_custom_completed_callback(
       Bind(&SaveProfilesAndSignalEvent, Unretained(profiles),
            Unretained(&sampling_thread_completed)));
   profiler.Start();
@@ -122,8 +127,9 @@ void CaptureProfiles(const StackSamplingProfiler::SamplingParams& params,
 // If this executable was linked with /INCREMENTAL (the default for non-official
 // debug and release builds on Windows), function addresses do not correspond to
 // function code itself, but instead to instructions in the Incremental Link
-// Table that jump to the functions. Check for a jump instruction and if present
-// do a little decompilation to find the function's actual starting address.
+// Table that jump to the functions. Checks for a jump instruction and if
+// present does a little decompilation to find the function's actual starting
+// address.
 const void* MaybeFixupFunctionAddressForILT(const void* function_address) {
 #if defined(_WIN64)
   const unsigned char* opcode =
@@ -131,10 +137,10 @@ const void* MaybeFixupFunctionAddressForILT(const void* function_address) {
   if (*opcode == 0xe9) {
     // This is a relative jump instruction. Assume we're in the ILT and compute
     // the function start address from the instruction offset.
-    const unsigned char* offset = opcode + 1;
-    const unsigned char* next_instruction = opcode + 5;
-    return next_instruction +
-        static_cast<int64>(*reinterpret_cast<const int32*>(offset));
+    const int32* offset = reinterpret_cast<const int32*>(opcode + 1);
+    const unsigned char* next_instruction =
+        reinterpret_cast<const unsigned char*>(offset + 1);
+    return next_instruction + *offset;
   }
 #endif
   return function_address;
@@ -150,11 +156,9 @@ Sample::const_iterator FindFirstFrameWithinFunction(
     int function_size) {
   function_address = MaybeFixupFunctionAddressForILT(function_address);
   for (auto it = sample.begin(); it != sample.end(); ++it) {
-    if ((reinterpret_cast<const unsigned char*>(it->instruction_pointer) >=
-         reinterpret_cast<const unsigned char*>(function_address)) &&
-        (reinterpret_cast<const unsigned char*>(it->instruction_pointer) <
-         (reinterpret_cast<const unsigned char*>(function_address) +
-          function_size)))
+    if ((it->instruction_pointer >= function_address) &&
+        (it->instruction_pointer <
+         (static_cast<const unsigned char*>(function_address) + function_size)))
       return it;
   }
   return sample.end();
@@ -164,24 +168,27 @@ Sample::const_iterator FindFirstFrameWithinFunction(
 std::string FormatSampleForDiagnosticOutput(
     const Sample& sample,
     const std::vector<Module>& modules) {
-  std::ostringstream stream;
+  std::string output;
   for (const Frame& frame: sample) {
-    stream << frame.instruction_pointer << " "
-           << modules[frame.module_index].filename.value() << std::endl;
+    output += StringPrintf(
+        "0x%p %s\n", frame.instruction_pointer,
+        modules[frame.module_index].filename.AsUTF8Unsafe().c_str());
   }
-  return stream.str();
+  return output;
 }
 
 // Returns a duration that is longer than the test timeout. We would use
 // TimeDelta::Max() but https://crbug.com/465948.
 TimeDelta AVeryLongTimeDelta() { return TimeDelta::FromDays(1); }
+
 }  // namespace
 
 
 // The tests below are enabled for Win x64 only, pending implementation of the
 // tested functionality on other platforms/architectures.
 
-// Checks that the basic expected information is present in a sampled profile.
+// Checks that the basic expected information is present in a sampled call stack
+// profile.
 #if defined(_WIN64)
 #define MAYBE_Basic Basic
 #else
@@ -189,31 +196,28 @@ TimeDelta AVeryLongTimeDelta() { return TimeDelta::FromDays(1); }
 #endif
 TEST(StackSamplingProfilerTest, MAYBE_Basic) {
   StackSamplingProfiler::SamplingParams params;
-  params.initial_delay = params.burst_interval = params.sampling_interval =
-      TimeDelta::FromMilliseconds(0);
-  params.bursts = 1;
+  params.sampling_interval = TimeDelta::FromMilliseconds(0);
   params.samples_per_burst = 1;
 
-  std::vector<Profile> profiles;
+  std::vector<CallStackProfile> profiles;
   CaptureProfiles(params, &profiles, AVeryLongTimeDelta());
 
   // Check that the profile and samples sizes are correct, and the module
   // indices are in range.
-
   ASSERT_EQ(1u, profiles.size());
-  const Profile& profile = profiles[0];
+  const CallStackProfile& profile = profiles[0];
   ASSERT_EQ(1u, profile.samples.size());
   EXPECT_EQ(params.sampling_interval, profile.sampling_period);
   const Sample& sample = profile.samples[0];
   for (const auto& frame : sample) {
-    ASSERT_GE(frame.module_index, 0);
-    ASSERT_LT(frame.module_index, static_cast<int>(profile.modules.size()));
+    ASSERT_GE(frame.module_index, 0u);
+    ASSERT_LT(frame.module_index, profile.modules.size());
   }
 
   // Check that the stack contains a frame for
   // TargetThread::SignalAndWaitUntilSignaled() and that the frame has this
   // executable's module.
-
+  //
   // Since we don't have a good way to know the function size, use 100 bytes as
   // a reasonable window to locate the instruction pointer.
   Sample::const_iterator loc = FindFirstFrameWithinFunction(
@@ -225,17 +229,15 @@ TEST(StackSamplingProfilerTest, MAYBE_Basic) {
       << MaybeFixupFunctionAddressForILT(
           reinterpret_cast<const void*>(
               &TargetThread::SignalAndWaitUntilSignaled))
-      << " was not found in stack:" << std::endl
+      << " was not found in stack:\n"
       << FormatSampleForDiagnosticOutput(sample, profile.modules);
-
   FilePath executable_path;
-  bool got_executable_path = PathService::Get(FILE_EXE, &executable_path);
-  EXPECT_TRUE(got_executable_path);
+  EXPECT_TRUE(PathService::Get(FILE_EXE, &executable_path));
   EXPECT_EQ(executable_path, profile.modules[loc->module_index].filename);
 }
 
 // Checks that the expected number of profiles and samples are present in the
-// profiles produced.
+// call stack profiles produced.
 #if defined(_WIN64)
 #define MAYBE_MultipleProfilesAndSamples MultipleProfilesAndSamples
 #else
@@ -243,12 +245,12 @@ TEST(StackSamplingProfilerTest, MAYBE_Basic) {
 #endif
 TEST(StackSamplingProfilerTest, MAYBE_MultipleProfilesAndSamples) {
   StackSamplingProfiler::SamplingParams params;
-  params.initial_delay = params.burst_interval = params.sampling_interval =
+  params.burst_interval = params.sampling_interval =
       TimeDelta::FromMilliseconds(0);
   params.bursts = 2;
   params.samples_per_burst = 3;
 
-  std::vector<Profile> profiles;
+  std::vector<CallStackProfile> profiles;
   CaptureProfiles(params, &profiles, AVeryLongTimeDelta());
 
   ASSERT_EQ(2u, profiles.size());
@@ -256,8 +258,8 @@ TEST(StackSamplingProfilerTest, MAYBE_MultipleProfilesAndSamples) {
   EXPECT_EQ(3u, profiles[1].samples.size());
 }
 
-// Checks that no profiles are captured if the profiling is stopped during the
-// initial delay.
+// Checks that no call stack profiles are captured if the profiling is stopped
+// during the initial delay.
 #if defined(_WIN64)
 #define MAYBE_StopDuringInitialDelay StopDuringInitialDelay
 #else
@@ -265,19 +267,16 @@ TEST(StackSamplingProfilerTest, MAYBE_MultipleProfilesAndSamples) {
 #endif
 TEST(StackSamplingProfilerTest, MAYBE_StopDuringInitialDelay) {
   StackSamplingProfiler::SamplingParams params;
-  params.burst_interval = params.sampling_interval =
-      TimeDelta::FromMilliseconds(0);
   params.initial_delay = TimeDelta::FromSeconds(60);
-  params.bursts = params.samples_per_burst = 1;
 
-  std::vector<Profile> profiles;
+  std::vector<CallStackProfile> profiles;
   CaptureProfiles(params, &profiles, TimeDelta::FromMilliseconds(0));
 
   EXPECT_TRUE(profiles.empty());
 }
 
-// Checks that the single completed profile is captured if the profiling is
-// stopped between bursts.
+// Checks that the single completed call stack profile is captured if the
+// profiling is stopped between bursts.
 #if defined(_WIN64)
 #define MAYBE_StopDuringInterBurstInterval StopDuringInterBurstInterval
 #else
@@ -285,20 +284,19 @@ TEST(StackSamplingProfilerTest, MAYBE_StopDuringInitialDelay) {
 #endif
 TEST(StackSamplingProfilerTest, MAYBE_StopDuringInterBurstInterval) {
   StackSamplingProfiler::SamplingParams params;
-  params.initial_delay = params.sampling_interval =
-      TimeDelta::FromMilliseconds(0);
+  params.sampling_interval = TimeDelta::FromMilliseconds(0);
   params.burst_interval = TimeDelta::FromSeconds(60);
   params.bursts = 2;
   params.samples_per_burst = 1;
 
-  std::vector<Profile> profiles;
+  std::vector<CallStackProfile> profiles;
   CaptureProfiles(params, &profiles, TimeDelta::FromMilliseconds(50));
 
   ASSERT_EQ(1u, profiles.size());
   EXPECT_EQ(1u, profiles[0].samples.size());
 }
 
-// Checks that only completed profiles are captured.
+// Checks that only completed call stack profiles are captured.
 #if defined(_WIN64)
 #define MAYBE_StopDuringInterSampleInterval StopDuringInterSampleInterval
 #else
@@ -307,15 +305,13 @@ TEST(StackSamplingProfilerTest, MAYBE_StopDuringInterBurstInterval) {
 #endif
 TEST(StackSamplingProfilerTest, MAYBE_StopDuringInterSampleInterval) {
   StackSamplingProfiler::SamplingParams params;
-  params.initial_delay = params.burst_interval = TimeDelta::FromMilliseconds(0);
   params.sampling_interval = TimeDelta::FromSeconds(60);
-  params.bursts = 1;
   params.samples_per_burst = 2;
 
-  std::vector<Profile> profiles;
+  std::vector<CallStackProfile> profiles;
   CaptureProfiles(params, &profiles, TimeDelta::FromMilliseconds(50));
 
   EXPECT_TRUE(profiles.empty());
 }
 
-}  // namespace tracked_objects
+}  // namespace base
diff --git a/base/profiler/stack_sampling_profiler_win.cc b/base/profiler/stack_sampling_profiler_win.cc
index ba46cf0..4d0f1d6 100644
--- a/base/profiler/stack_sampling_profiler_win.cc
+++ b/base/profiler/stack_sampling_profiler_win.cc
@@ -2,14 +2,13 @@
 // 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 <windows.h>
 
-#include <dbghelp.h>
 #include <map>
 #include <utility>
-#include <windows.h>
 
 #include "base/logging.h"
+#include "base/profiler/native_stack_sampler.h"
 #include "base/time/time.h"
 #include "base/win/pe_image.h"
 #include "base/win/scoped_handle.h"
@@ -18,38 +17,7 @@ namespace base {
 
 namespace {
 
-class NativeStackSamplerWin : public StackSamplingProfiler::NativeStackSampler {
- public:
-  explicit NativeStackSamplerWin(win::ScopedHandle thread_handle);
-  ~NativeStackSamplerWin() override;
-
-  // StackSamplingProfiler::NativeStackSampler:
-  void ProfileRecordingStarting(
-      StackSamplingProfiler::Profile* profile) override;
-  void RecordStackSample(StackSamplingProfiler::Sample* sample) override;
-  void ProfileRecordingStopped() override;
-
- private:
-  static bool GetModuleInfo(HMODULE module,
-                            StackSamplingProfiler::Module* module_info);
-
-  void CopyToSample(const void* const instruction_pointers[],
-                    const HMODULE modules[],
-                    int stack_depth,
-                    StackSamplingProfiler::Sample* sample,
-                    std::vector<StackSamplingProfiler::Module>* module_infos);
-
-  win::ScopedHandle thread_handle_;
-  // Weak. Points to the profile being recorded between
-  // ProfileRecordingStarting() and ProfileRecordingStopped().
-  StackSamplingProfiler::Profile* current_profile_;
-  // Maps a module to the module's index within current_profile_->modules.
-  std::map<HMODULE, int> profile_module_index_;
-
-  DISALLOW_COPY_AND_ASSIGN(NativeStackSamplerWin);
-};
-
-// Walk the stack represented by |context| from the current frame downwards,
+// Walks the stack represented by |context| from the current frame downwards,
 // recording the instruction pointers for each frame in |instruction_pointers|.
 int RecordStack(CONTEXT* context,
                 int max_stack_size,
@@ -58,66 +26,65 @@ int RecordStack(CONTEXT* context,
 #ifdef _WIN64
   *last_frame_is_unknown_function = false;
 
-  IMAGEHLP_SYMBOL64 sym;
-  sym.SizeOfStruct = sizeof(sym);
-  sym.MaxNameLength = 0;
-
-  for (int i = 0; i < max_stack_size; ++i) {
+  int i = 0;
+  for (; (i < max_stack_size) && context->Rip; ++i) {
     // Try to look up unwind metadata for the current function.
     ULONG64 image_base;
     PRUNTIME_FUNCTION runtime_function =
         RtlLookupFunctionEntry(context->Rip, &image_base, nullptr);
 
-    instruction_pointers[i] = reinterpret_cast<void*>(context->Rip);
+    instruction_pointers[i] = reinterpret_cast<const void*>(context->Rip);
 
     if (runtime_function) {
       KNONVOLATILE_CONTEXT_POINTERS nvcontext = {0};
       void* handler_data;
       ULONG64 establisher_frame;
       RtlVirtualUnwind(0, image_base, context->Rip, runtime_function, context,
-          &handler_data, &establisher_frame, &nvcontext);
+                       &handler_data, &establisher_frame, &nvcontext);
     } else {
-      // If we don't have a RUNTIME_FUNCTION, then we've encountered
-      // a leaf function.  Adjust the stack appropriately.
+      // If we don't have a RUNTIME_FUNCTION, then we've encountered a leaf
+      // function.  Adjust the stack appropriately prior to the next function
+      // lookup.
       context->Rip = *reinterpret_cast<PDWORD64>(context->Rsp);
       context->Rsp += 8;
       *last_frame_is_unknown_function = true;
     }
-
-    if (!context->Rip)
-      return i;
   }
-  return max_stack_size;
+  return i;
 #else
   return 0;
 #endif
 }
 
-// Fills in |modules| corresponding to the pointers to code in |addresses|. The
-// modules are returned with reference counts incremented should be freed with
-// FreeModules.
-void FindModulesForAddresses(const void* const addresses[], HMODULE modules[],
-                             int stack_depth,
+// Fills in |module_handles| corresponding to the pointers to code in
+// |addresses|. The module handles are returned with reference counts
+// incremented and should be freed with FreeModuleHandles. See note in
+// SuspendThreadAndRecordStack for why |addresses| and |module_handles| are
+// arrays.
+void FindModuleHandlesForAddresses(const void* const addresses[],
+                             HMODULE module_handles[], int stack_depth,
                              bool last_frame_is_unknown_function) {
-  const int module_frames = last_frame_is_unknown_function ? stack_depth - 1 :
-      stack_depth;
+  const int module_frames =
+      last_frame_is_unknown_function ? stack_depth - 1 : stack_depth;
   for (int i = 0; i < module_frames; ++i) {
-    HMODULE module = NULL;
+    HMODULE module_handle = NULL;
     if (GetModuleHandleEx(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS,
                           reinterpret_cast<LPCTSTR>(addresses[i]),
-                          &module)) {
-      // HMODULE is the base address of the module.
-      DCHECK_LT(reinterpret_cast<const void*>(module), addresses[i]);
-      modules[i] = module;
+                          &module_handle)) {
+      // HMODULE actually represents the base address of the module, so we can
+      // use it directly as an address.
+      DCHECK_LE(reinterpret_cast<const void*>(module_handle), addresses[i]);
+      module_handles[i] = module_handle;
     }
   }
 }
 
-// Free the modules returned by FindModulesForAddresses.
-void FreeModules(int stack_depth, HMODULE modules[]) {
+// Frees the modules handles returned by FindModuleHandlesForAddresses. See note
+// in SuspendThreadAndRecordStack for why |module_handles| is an array.
+void FreeModuleHandles(int stack_depth, HMODULE module_handles[]) {
   for (int i = 0; i < stack_depth; ++i) {
-    if (modules[i])
-      ::FreeLibrary(modules[i]);
+    if (module_handles[i])
+      ::FreeLibrary(module_handles[i]);
   }
 }
 
@@ -141,185 +108,212 @@ ScopedDisablePriorityBoost::ScopedDisablePriorityBoost(HANDLE thread_handle)
       boost_state_was_disabled_(false) {
   got_previous_boost_state_ =
       ::GetThreadPriorityBoost(thread_handle_, &boost_state_was_disabled_);
-  if (got_previous_boost_state_ && !boost_state_was_disabled_) {
-    // Confusingly, TRUE disables priority boost ...
+  if (got_previous_boost_state_) {
+    // Confusingly, TRUE disables priority boost.
     ::SetThreadPriorityBoost(thread_handle_, TRUE);
   }
 }
 
 ScopedDisablePriorityBoost::~ScopedDisablePriorityBoost() {
-  if (got_previous_boost_state_ && !boost_state_was_disabled_) {
-    // ... and FALSE enables priority boost.
-    ::SetThreadPriorityBoost(thread_handle_, FALSE);
-  }
+  if (got_previous_boost_state_)
+    ::SetThreadPriorityBoost(thread_handle_, boost_state_was_disabled_);
 }
 
 // Suspends the thread with |thread_handle|, records the stack into
 // |instruction_pointers|, then resumes the thread. Returns the size of the
 // stack.
+//
+// IMPORTANT NOTE: No heap allocations may occur between SuspendThread and
+// ResumeThread. Otherwise this code can deadlock on heap locks acquired by the
+// target thread before it was suspended. This is why we pass instruction
+// pointers and module handles as preallocated arrays rather than vectors, since
+// vectors make it too easy to subtly allocate memory.
 int SuspendThreadAndRecordStack(HANDLE thread_handle, int max_stack_size,
                                 const void* instruction_pointers[],
                                 bool* last_frame_is_unknown_function) {
-#if defined(_WIN64)
-  if (RtlVirtualUnwind == nullptr || RtlLookupFunctionEntry == nullptr)
+  if (::SuspendThread(thread_handle) == -1)
     return 0;
-#endif
-
-  if (::SuspendThread(thread_handle) == -1) {
-    LOG(ERROR) << "SuspendThread failed: " << GetLastError();
-    return 0;
-  }
 
+  int stack_depth = 0;
   CONTEXT thread_context = {0};
   thread_context.ContextFlags = CONTEXT_FULL;
-  if (!::GetThreadContext(thread_handle, &thread_context)) {
-    LOG(ERROR) << "GetThreadContext failed: " << GetLastError();
+  if (::GetThreadContext(thread_handle, &thread_context)) {
+    stack_depth = RecordStack(&thread_context, max_stack_size,
+                              instruction_pointers,
+                              last_frame_is_unknown_function);
   }
 
-  int stack_depth = RecordStack(&thread_context, max_stack_size,
-                                instruction_pointers,
-                                last_frame_is_unknown_function);
-
-  {
-    ScopedDisablePriorityBoost disable_priority_boost(thread_handle);
-    if (::ResumeThread(thread_handle) == -1)
-      LOG(ERROR) << "ResumeThread failed: " << GetLastError();
-  }
+  // Disable the priority boost that the thread would otherwise receive on
+  // resume. We do this to avoid artificially altering the dynamics of the
+  // executing application any more than we already are by suspending and
+  // resuming the thread.
+  //
+  // Note that this can racily disable a priority boost that otherwise would
+  // have been given to the thread, if the thread is waiting on other wait
+  // conditions at the time of SuspendThread and those conditions are satisfied
+  // before priority boost is reenabled. The measured length of this window is
+  // ~100us, so this should occur fairly rarely.
+  ScopedDisablePriorityBoost disable_priority_boost(thread_handle);
+  bool resume_thread_succeeded = ::ResumeThread(thread_handle) != -1;
+  CHECK(resume_thread_succeeded) << "ResumeThread failed: " << GetLastError();
 
   return stack_depth;
 }
 
-}  // namespace
+class NativeStackSamplerWin : public NativeStackSampler {
+ public:
+  explicit NativeStackSamplerWin(win::ScopedHandle thread_handle);
+  ~NativeStackSamplerWin() override;
 
-scoped_ptr<StackSamplingProfiler::NativeStackSampler>
-StackSamplingProfiler::NativeStackSampler::Create(PlatformThreadId thread_id) {
-#if _WIN64
-  // Get the thread's handle.
-  HANDLE thread_handle = ::OpenThread(
-      THREAD_GET_CONTEXT | THREAD_SUSPEND_RESUME | THREAD_QUERY_INFORMATION,
-      FALSE,
-      thread_id);
-  DCHECK(thread_handle) << "OpenThread failed";
+  // StackSamplingProfiler::NativeStackSampler:
+  void ProfileRecordingStarting(
+      std::vector<StackSamplingProfiler::Module>* modules) override;
+  void RecordStackSample(StackSamplingProfiler::Sample* sample) override;
+  void ProfileRecordingStopped() override;
 
-  return scoped_ptr<NativeStackSampler>(new NativeStackSamplerWin(
-      win::ScopedHandle(thread_handle)));
-#else
-  return scoped_ptr<NativeStackSampler>();
-#endif
-}
+ private:
+  // Attempts to query the module filename, base address, and id for
+  // |module_handle|, and store them in |module|. Returns true if it succeeded.
+  static bool GetModuleForHandle(HMODULE module_handle,
+                                 StackSamplingProfiler::Module* module);
+
+  // Gets the index for the Module corresponding to |module_handle| in
+  // |modules|, adding it if it's not already present. Returns
+  // StackSamplingProfiler::Frame::kUnknownModuleIndex if no Module can be
+  // determined for |module|.
+  size_t GetModuleIndex(HMODULE module_handle,
+                        std::vector<StackSamplingProfiler::Module>* modules);
+
+  // Copies the stack information represented by |instruction_pointers| into
+  // |sample| and |modules|.
+  void CopyToSample(const void* const instruction_pointers[],
+                    const HMODULE module_handles[],
+                    int stack_depth,
+                    StackSamplingProfiler::Sample* sample,
+                    std::vector<StackSamplingProfiler::Module>* modules);
+
+  win::ScopedHandle thread_handle_;
+  // Weak. Points to the modules associated with the profile being recorded
+  // between ProfileRecordingStarting() and ProfileRecordingStopped().
+  std::vector<StackSamplingProfiler::Module>* current_modules_;
+  // Maps a module handle to the corresponding Module's index within
+  // current_modules_.
+  std::map<HMODULE, size_t> profile_module_index_;
+
+  DISALLOW_COPY_AND_ASSIGN(NativeStackSamplerWin);
+};
 
 NativeStackSamplerWin::NativeStackSamplerWin(win::ScopedHandle thread_handle)
     : thread_handle_(thread_handle.Take()) {
-#ifdef _WIN64
-  if (RtlVirtualUnwind == nullptr && RtlLookupFunctionEntry == nullptr) {
-    const HMODULE nt_dll_handle = ::GetModuleHandle(L"ntdll.dll");
-    // This should always be non-null, but handle just in case.
-    if (nt_dll_handle) {
-      reinterpret_cast<void*&>(RtlVirtualUnwind) =
-          ::GetProcAddress(nt_dll_handle, "RtlVirtualUnwind");
-      reinterpret_cast<void*&>(RtlLookupFunctionEntry) =
-          ::GetProcAddress(nt_dll_handle, "RtlLookupFunctionEntry");
-    }
-  }
-#endif
 }
 
 NativeStackSamplerWin::~NativeStackSamplerWin() {
 }
 
 void NativeStackSamplerWin::ProfileRecordingStarting(
-    StackSamplingProfiler::Profile* profile) {
-  current_profile_ = profile;
+    std::vector<StackSamplingProfiler::Module>* modules) {
+  current_modules_ = modules;
   profile_module_index_.clear();
 }
 
 void NativeStackSamplerWin::RecordStackSample(
     StackSamplingProfiler::Sample* sample) {
-  DCHECK(current_profile_);
+  DCHECK(current_modules_);
 
   const int max_stack_size = 64;
   const void* instruction_pointers[max_stack_size] = {0};
-  HMODULE modules[max_stack_size] = {0};
+  HMODULE module_handles[max_stack_size] = {0};
 
   bool last_frame_is_unknown_function = false;
   int stack_depth = SuspendThreadAndRecordStack(
       thread_handle_.Get(), max_stack_size, instruction_pointers,
       &last_frame_is_unknown_function);
-  FindModulesForAddresses(instruction_pointers, modules, stack_depth,
-                          last_frame_is_unknown_function);
-  CopyToSample(instruction_pointers, modules, stack_depth, sample,
-               &current_profile_->modules);
-  FreeModules(stack_depth, modules);
+  FindModuleHandlesForAddresses(instruction_pointers, module_handles,
+                                stack_depth, last_frame_is_unknown_function);
+  CopyToSample(instruction_pointers, module_handles, stack_depth, sample,
+               current_modules_);
+  FreeModuleHandles(stack_depth, module_handles);
 }
 
 void NativeStackSamplerWin::ProfileRecordingStopped() {
-  current_profile_ = nullptr;
+  current_modules_ = nullptr;
 }
 
 // static
-bool NativeStackSamplerWin::GetModuleInfo(
-    HMODULE module,
-    StackSamplingProfiler::Module* module_info) {
+bool NativeStackSamplerWin::GetModuleForHandle(
+    HMODULE module_handle,
+    StackSamplingProfiler::Module* module) {
   wchar_t module_name[MAX_PATH];
   DWORD result_length =
-      GetModuleFileName(module, module_name, arraysize(module_name));
+      GetModuleFileName(module_handle, module_name, arraysize(module_name));
   if (result_length == 0)
     return false;
 
-  module_info->filename = base::FilePath(module_name);
+  module->filename = base::FilePath(module_name);
 
-  module_info->base_address = reinterpret_cast<const void*>(module);
+  module->base_address = reinterpret_cast<const void*>(module_handle);
 
   GUID guid;
   DWORD age;
-  win::PEImage(module).GetDebugId(&guid, &age);
-  module_info->id.insert(module_info->id.end(),
-                         reinterpret_cast<char*>(&guid),
-                         reinterpret_cast<char*>(&guid + 1));
-  module_info->id.insert(module_info->id.end(),
-                         reinterpret_cast<char*>(&age),
-                         reinterpret_cast<char*>(&age + 1));
+  win::PEImage(module_handle).GetDebugId(&guid, &age);
+  module->id.assign(reinterpret_cast<char*>(&guid), sizeof(guid));
+  module->id.append(reinterpret_cast<char*>(&age), sizeof(age));
 
   return true;
 }
 
+size_t NativeStackSamplerWin::GetModuleIndex(
+    HMODULE module_handle,
+    std::vector<StackSamplingProfiler::Module>* modules) {
+  if (!module_handle)
+    return StackSamplingProfiler::Frame::kUnknownModuleIndex;
+
+  auto loc = profile_module_index_.find(module_handle);
+  if (loc == profile_module_index_.end()) {
+    StackSamplingProfiler::Module module;
+    if (!GetModuleForHandle(module_handle, &module))
+      return StackSamplingProfiler::Frame::kUnknownModuleIndex;
+    modules->push_back(module);
+    loc = profile_module_index_.insert(std::make_pair(
+        module_handle, modules->size() - 1)).first;
+  }
+
+  return loc->second;
+}
+
 void NativeStackSamplerWin::CopyToSample(
     const void* const instruction_pointers[],
-    const HMODULE modules[],
+    const HMODULE module_handles[],
     int stack_depth,
     StackSamplingProfiler::Sample* sample,
-    std::vector<StackSamplingProfiler::Module>* module_infos) {
+    std::vector<StackSamplingProfiler::Module>* module) {
   sample->clear();
   sample->reserve(stack_depth);
 
   for (int i = 0; i < stack_depth; ++i) {
-    sample->push_back(StackSamplingProfiler::Frame());
-    StackSamplingProfiler::Frame& frame = sample->back();
-
-    frame.instruction_pointer = instruction_pointers[i];
+    sample->push_back(StackSamplingProfiler::Frame(
+        instruction_pointers[i],
+        GetModuleIndex(module_handles[i], module)));
+  }
+}
 
-    // Record an invalid module index if we don't have a valid module.
-    if (!modules[i]) {
-      frame.module_index = -1;
-      continue;
-    }
+}  // namespace
 
-    auto loc = profile_module_index_.find(modules[i]);
-    if (loc == profile_module_index_.end()) {
-      StackSamplingProfiler::Module module_info;
-      // Record an invalid module index if we have a module but can't find
-      // information on it.
-      if (!GetModuleInfo(modules[i], &module_info)) {
-        frame.module_index = -1;
-        continue;
-      }
-      module_infos->push_back(module_info);
-      loc = profile_module_index_.insert(std::make_pair(
-          modules[i], static_cast<int>(module_infos->size() - 1))).first;
-    }
+scoped_ptr<NativeStackSampler> NativeStackSampler::Create(
+    PlatformThreadId thread_id) {
+#if _WIN64
+  // Get the thread's handle.
+  HANDLE thread_handle = ::OpenThread(
+      THREAD_GET_CONTEXT | THREAD_SUSPEND_RESUME | THREAD_QUERY_INFORMATION,
+      FALSE,
+      thread_id);
 
-    frame.module_index = loc->second;
+  if (thread_handle) {
+    return scoped_ptr<NativeStackSampler>(new NativeStackSamplerWin(
+        win::ScopedHandle(thread_handle)));
   }
+#endif
+  return scoped_ptr<NativeStackSampler>();
 }
 
 }  // namespace base
diff --git a/components/metrics/call_stack_profile_metrics_provider.cc b/components/metrics/call_stack_profile_metrics_provider.cc
index 7ed60f5..f96923b 100644
--- a/components/metrics/call_stack_profile_metrics_provider.cc
+++ b/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;
 }
 
diff --git a/components/metrics/call_stack_profile_metrics_provider.h b/components/metrics/call_stack_profile_metrics_provider.h
index a2bb0f8..6d5ff8c 100644
--- a/components/metrics/call_stack_profile_metrics_provider.h
+++ b/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);
 };
diff --git a/components/metrics/call_stack_profile_metrics_provider_unittest.cc b/components/metrics/call_stack_profile_metrics_provider_unittest.cc
index 20e53d6..a4e3efb 100644
--- a/components/metrics/call_stack_profile_metrics_provider_unittest.cc
+++ b/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;