StackSamplingProfiler clean up

base/profiler/stack_sampling_profiler_unittest.cc

Index: base/profiler/stack_sampling_profiler_unittest.cc
diff --git a/base/profiler/stack_sampling_profiler_unittest.cc b/base/profiler/stack_sampling_profiler_unittest.cc
index dfccab4e1a893ae7da9c768f0e5c17aef4f9b692..8ce663786b57ad2902d04aa912e2761abfee361b 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);
 
@@ -87,18 +91,19 @@ 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;
@@ -109,7 +114,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();
@@ -125,8 +130,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 =
@@ -134,10 +140,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;
@@ -153,11 +159,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();
@@ -167,24 +171,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
@@ -192,31 +199,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(
@@ -228,17 +232,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
@@ -246,12 +248,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());
@@ -259,8 +261,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
@@ -268,19 +270,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
@@ -288,20 +287,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
@@ -310,15 +308,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