Support parallel captures from the StackSamplingProfiler.

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 0b300d575d73c76f65a6afd7e53a372020a844a3..277c519a1d20830e5441265dc0818cf3dc756511 100644
--- a/base/profiler/stack_sampling_profiler_unittest.cc
+++ b/base/profiler/stack_sampling_profiler_unittest.cc
@@ -14,7 +14,6 @@
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
-#include "base/message_loop/message_loop.h"
 #include "base/native_library.h"
 #include "base/path_service.h"
 #include "base/profiler/native_stack_sampler.h"
@@ -23,8 +22,10 @@
 #include "base/scoped_native_library.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/synchronization/lock.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/threading/platform_thread.h"
+#include "base/threading/simple_thread.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -351,6 +352,44 @@ void WithTargetThread(Function function) {
   WithTargetThread(function, StackConfiguration(StackConfiguration::NORMAL));
 }
 
+// Waits for one of multiple samplings to complete. If |delegates| is provided,
+// the vector must be pre-populated and of the same size as |params|.
+void CreateProfilers(
+    PlatformThreadId target_thread_id,
+    const std::vector<SamplingParams>& params,
+    const std::vector<std::unique_ptr<NativeStackSamplerTestDelegate>>*
+        delegates,
+    std::vector<CallStackProfiles>* profiles,
+    std::vector<std::unique_ptr<StackSamplingProfiler>>* profilers,
+    std::vector<std::unique_ptr<WaitableEvent>>* completed) {

[Mike Wittman, 2017/03/31 18:12:33]

There's a very subtle issue with this function in that it hides destruction
ordering constraints from the caller: if the profiles or completed vectors were
declared before the profilers vector in a test, the profilers could access those
objects after they were destroyed but before the profilers were destroyed. This
would cause flaky crashes that would be very difficult to track down.

We should put these in a struct to enforce proper destruction order:
struct ProfilerState {
  std::unique_ptr<StackSamplingProfiler> profiler;
  CallStackProfiles profiles;
  std::unique_ptr<WaitableEvent> completed;
};

along with a comment calling out the reason for the ordering. Then, return a
std::vector<ProfilerState> from this function.

[bcwhite, 2017/04/03 20:18:13]

Better than that, I think, is to create a struct that contains everything for a
single test profiler and create a vector of pointers to those.

This cleans up a lot of things and means we can get rid of vectors of a single
element.

[Mike Wittman, 2017/04/04 17:59:51]

Nice, that makes things much cleaner.

+  ASSERT_TRUE(!params.empty());
+  ASSERT_TRUE(profiles->empty());
+  ASSERT_TRUE(profilers->empty());
+  ASSERT_TRUE(completed->empty());
+  if (delegates)
+    ASSERT_EQ(params.size(), delegates->size());
+
+  // Vectors have to be appropriately sized in advance so that the addresses of
+  // values don't change.
+  const size_t count = params.size();
+  profiles->reserve(count);
+  profilers->reserve(count);
+  completed->reserve(count);
+
+  for (size_t i = 0; i < count; ++i) {
+    profiles->push_back(CallStackProfiles());
+    completed->push_back(
+        MakeUnique<WaitableEvent>(WaitableEvent::ResetPolicy::AUTOMATIC,
+                                  WaitableEvent::InitialState::NOT_SIGNALED));
+    const StackSamplingProfiler::CompletedCallback callback =
+        Bind(&SaveProfilesAndSignalEvent, Unretained(&profiles->back()),
+             Unretained(completed->back().get()));
+    profilers->push_back(MakeUnique<StackSamplingProfiler>(
+        target_thread_id, params[i], callback,
+        (delegates ? (*delegates)[i].get() : nullptr)));
+  }
+}
+
 // 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.
@@ -374,6 +413,21 @@ void CaptureProfiles(const SamplingParams& params, TimeDelta profiler_wait_time,
   });
 }
 
+// Waits for one of multiple samplings to complete.
+size_t WaitForSamplingComplete(
+    std::vector<std::unique_ptr<WaitableEvent>>* sampling_completed) {
+  // Map unique_ptrs to something that WaitMany can accept.
+  std::vector<WaitableEvent*> sampling_completed_rawptrs(
+      sampling_completed->size());
+  std::transform(
+      sampling_completed->begin(), sampling_completed->end(),
+      sampling_completed_rawptrs.begin(),
+      [](const std::unique_ptr<WaitableEvent>& elem) { return elem.get(); });
+  // Wait for one profiler to finish.
+  return WaitableEvent::WaitMany(sampling_completed_rawptrs.data(),
+                                 sampling_completed_rawptrs.size());
+}
+
 // 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
@@ -444,9 +498,9 @@ void TestLibraryUnload(bool wait_until_unloaded) {
     StackCopiedSignaler(WaitableEvent* stack_copied,
                         WaitableEvent* start_stack_walk,
                         bool wait_to_walk_stack)
-        : stack_copied_(stack_copied), start_stack_walk_(start_stack_walk),
-          wait_to_walk_stack_(wait_to_walk_stack) {
-    }
+        : stack_copied_(stack_copied),
+          start_stack_walk_(start_stack_walk),
+          wait_to_walk_stack_(wait_to_walk_stack) {}
 
     void OnPreStackWalk() override {
       stack_copied_->Signal();
@@ -577,6 +631,28 @@ void TestLibraryUnload(bool wait_until_unloaded) {
   }
 }
 
+class StackSamplingProfilerTest : public testing::Test {
+ public:
+  void SetUp() override {
+    // Reset everything to have a clean test environment. This is done before
+    // the test becasue it's impossible to know what other tests, including
+    // those outside this file, might have inadventently done that affects the
+    // profiler.
+    StackSamplingProfiler::TestAPI::Reset();
+
+    // The idle-shutdown time is too long for convenient (and accurate) testing.
+    // That behavior is checked instead by artificially triggering it through
+    // the TestAPI.
+    StackSamplingProfiler::TestAPI::DisableIdleShutdown();
+  }
+
+  void TearDown() override {
+    // Be a good citizen and clean up after ourselves. This also re-enables the
+    // idle-shutdown behavior.
+    StackSamplingProfiler::TestAPI::Reset();
+  }
+};
+
 }  // namespace
 
 // Checks that the basic expected information is present in a sampled call stack
@@ -586,9 +662,7 @@ void TestLibraryUnload(bool wait_until_unloaded) {
 #else
 #define MAYBE_Basic DISABLED_Basic
 #endif
-TEST(StackSamplingProfilerTest, MAYBE_Basic) {
-  StackSamplingProfiler::ResetAnnotationsForTesting();
-
+TEST_F(StackSamplingProfilerTest, MAYBE_Basic) {
   SamplingParams params;
   params.sampling_interval = TimeDelta::FromMilliseconds(0);
   params.samples_per_burst = 1;
@@ -631,9 +705,7 @@ TEST(StackSamplingProfilerTest, MAYBE_Basic) {
 #else
 #define MAYBE_Annotations DISABLED_Annotations
 #endif
-TEST(StackSamplingProfilerTest, MAYBE_Annotations) {
-  StackSamplingProfiler::ResetAnnotationsForTesting();
-
+TEST_F(StackSamplingProfilerTest, MAYBE_Annotations) {
   SamplingParams params;
   params.sampling_interval = TimeDelta::FromMilliseconds(0);
   params.samples_per_burst = 1;
@@ -667,7 +739,7 @@ TEST(StackSamplingProfilerTest, MAYBE_Annotations) {
 #else
 #define MAYBE_Alloca DISABLED_Alloca
 #endif
-TEST(StackSamplingProfilerTest, MAYBE_Alloca) {
+TEST_F(StackSamplingProfilerTest, MAYBE_Alloca) {
   SamplingParams params;
   params.sampling_interval = TimeDelta::FromMilliseconds(0);
   params.samples_per_burst = 1;
@@ -720,35 +792,6 @@ TEST(StackSamplingProfilerTest, MAYBE_Alloca) {
       << FormatSampleForDiagnosticOutput(sample, profile.modules);
 }
 
-// Checks that the fire-and-forget interface works.
-#if defined(STACK_SAMPLING_PROFILER_SUPPORTED)
-#define MAYBE_StartAndRunAsync StartAndRunAsync
-#else
-#define MAYBE_StartAndRunAsync DISABLED_StartAndRunAsync
-#endif
-TEST(StackSamplingProfilerTest, MAYBE_StartAndRunAsync) {
-  // StartAndRunAsync requires the caller to have a message loop.
-  MessageLoop message_loop;
-
-  SamplingParams params;
-  params.samples_per_burst = 1;
-
-  CallStackProfiles profiles;
-  WithTargetThread([&params, &profiles](PlatformThreadId target_thread_id) {
-    WaitableEvent sampling_thread_completed(
-        WaitableEvent::ResetPolicy::AUTOMATIC,
-        WaitableEvent::InitialState::NOT_SIGNALED);
-    const StackSamplingProfiler::CompletedCallback callback =
-        Bind(&SaveProfilesAndSignalEvent, Unretained(&profiles),
-             Unretained(&sampling_thread_completed));
-    StackSamplingProfiler::StartAndRunAsync(target_thread_id, params, callback);
-    RunLoop().RunUntilIdle();
-    sampling_thread_completed.Wait();
-  });
-
-  ASSERT_EQ(1u, profiles.size());
-}
-
 // Checks that the expected number of profiles and samples are present in the
 // call stack profiles produced.
 #if defined(STACK_SAMPLING_PROFILER_SUPPORTED)
@@ -756,7 +799,7 @@ TEST(StackSamplingProfilerTest, MAYBE_StartAndRunAsync) {
 #else
 #define MAYBE_MultipleProfilesAndSamples DISABLED_MultipleProfilesAndSamples
 #endif
-TEST(StackSamplingProfilerTest, MAYBE_MultipleProfilesAndSamples) {
+TEST_F(StackSamplingProfilerTest, MAYBE_MultipleProfilesAndSamples) {
   SamplingParams params;
   params.burst_interval = params.sampling_interval =
       TimeDelta::FromMilliseconds(0);
@@ -771,6 +814,124 @@ TEST(StackSamplingProfilerTest, MAYBE_MultipleProfilesAndSamples) {
   EXPECT_EQ(3u, profiles[1].samples.size());
 }
 
+// Checks that a profiler can stop/destruct without ever having started.
+#if defined(STACK_SAMPLING_PROFILER_SUPPORTED)
+#define MAYBE_StopWithoutStarting StopWithoutStarting
+#else
+#define MAYBE_StopWithoutStarting DISABLED_StopWithoutStarting
+#endif
+TEST_F(StackSamplingProfilerTest, MAYBE_StopWithoutStarting) {
+  WithTargetThread([](PlatformThreadId target_thread_id) {
+    SamplingParams params;
+    params.sampling_interval = TimeDelta::FromMilliseconds(0);
+    params.samples_per_burst = 1;
+
+    CallStackProfiles profiles;
+    WaitableEvent sampling_completed(WaitableEvent::ResetPolicy::MANUAL,
+                                     WaitableEvent::InitialState::NOT_SIGNALED);
+    const StackSamplingProfiler::CompletedCallback callback =
+        Bind(&SaveProfilesAndSignalEvent, Unretained(&profiles),
+             Unretained(&sampling_completed));
+    StackSamplingProfiler profiler(target_thread_id, params, callback);
+
+    profiler.Stop();  // Constructed but never started.
+    EXPECT_FALSE(sampling_completed.IsSignaled());
+  });
+}
+
+// Checks that its okay to stop a profiler before it finishes even when the
+// sampling thread continues to run.
+#if defined(STACK_SAMPLING_PROFILER_SUPPORTED)
+#define MAYBE_StopSafely StopSafely
+#else
+#define MAYBE_StopSafely DISABLED_StopSafely
+#endif
+TEST_F(StackSamplingProfilerTest, MAYBE_StopSafely) {
+  // Test delegate that counts samples.
+  class SampleRecordedCounter : public NativeStackSamplerTestDelegate {
+   public:
+    SampleRecordedCounter() {}
+
+    void OnPreStackWalk() override {
+      AutoLock lock(lock_);
+      ++count_;
+    }
+
+    size_t Get() {
+      AutoLock lock(lock_);
+      return count_;
+    }
+
+   private:
+    Lock lock_;
+    size_t count_ = 0;
+  };
+
+  WithTargetThread([](PlatformThreadId target_thread_id) {
+    std::vector<SamplingParams> params(2);
+
+    // Providing an initial delay makes it more likely that both will be
+    // scheduled before either starts to run. Once started, samples will
+    // run ordered by their scheduled, interleaved times regardless of
+    // whatever interval the thread wakes up.
+    params[0].initial_delay = TimeDelta::FromMilliseconds(10);
+    params[0].sampling_interval = TimeDelta::FromMilliseconds(1);
+    params[0].samples_per_burst = 100000;
+
+    params[1].initial_delay = TimeDelta::FromMilliseconds(10);
+    params[1].sampling_interval = TimeDelta::FromMilliseconds(1);
+    params[1].samples_per_burst = 100000;
+
+    std::vector<CallStackProfiles> profiles;
+    std::vector<std::unique_ptr<NativeStackSamplerTestDelegate>>
+        samples_recorded;
+    std::vector<std::unique_ptr<WaitableEvent>> sampling_completed;
+    std::vector<std::unique_ptr<StackSamplingProfiler>> profilers;
+    for (size_t i = 0; i < params.size(); ++i)
+      samples_recorded.push_back(MakeUnique<SampleRecordedCounter>());
+    CreateProfilers(target_thread_id, params, &samples_recorded, &profiles,
+                    &profilers, &sampling_completed);
+
+    profilers[0]->Start();
+    profilers[1]->Start();
+
+    // Wait for both to start accumulating samples. Using a WaitableEvent is
+    // possible but gets complicated later on because there's no way of knowing
+    // if 0 or 1 additional sample will be taken after Stop() and thus no way
+    // of knowing how many Wait() calls to make on it.
+    while (
+        static_cast<SampleRecordedCounter*>(samples_recorded[0].get())->Get() ==

[Mike Wittman, 2017/03/31 18:12:33]

I think we should be able to avoid casting entirely by using a template
function. Taking the comment on the CreateProfilers function into account:

template <typename T>
std::vector<ProfilerState> CreateProfilers(
    PlatformThreadId target_thread_id,
    const std::vector<SamplingParams>& params,
    const std::vector<std::unique_ptr<T>>* test_delegates) {
  // ... existing function ...
}

std::vector<ProfilerState> CreateProfilers(
    PlatformThreadId target_thread_id,
    const std::vector<SamplingParams>& params) {
  return CreateProfilers<NativeStackSamplerTestDelegate>(target_thread_id,
      params, nullptr);
}

And convert the non-delegate-users to the second overload.

[bcwhite, 2017/04/03 20:18:13]

No casting necessary with new TestProfilerInfo structure.

+            0 ||
+        static_cast<SampleRecordedCounter*>(samples_recorded[1].get())->Get() ==
+            0)
+      PlatformThread::Sleep(TimeDelta::FromMilliseconds(1));
+
+    // Ensure that the first sampler can be safely stopped while the second
+    // continues to run. The stopped first profiler will still have a
+    // PerformCollectionTask pending that will do nothing when executed because
+    // the collection will have been removed by Stop().
+    profilers[0]->Stop();
+    sampling_completed[0]->Wait();
+    size_t count0 =
+        static_cast<SampleRecordedCounter*>(samples_recorded[0].get())->Get();
+    size_t count1 =
+        static_cast<SampleRecordedCounter*>(samples_recorded[1].get())->Get();
+
+    // Waiting for the second sampler to collect a couple samples ensures that
+    // the pending PerformCollectionTask for the first has executed because
+    // tasks are always ordered by their next scheduled time.
+    while (
+        static_cast<SampleRecordedCounter*>(samples_recorded[1].get())->Get() <
+        count1 + 2)
+      PlatformThread::Sleep(TimeDelta::FromMilliseconds(1));
+
+    // Ensure that the first profiler didn't do anything since it was stopped.
+    EXPECT_EQ(
+        count0,
+        static_cast<SampleRecordedCounter*>(samples_recorded[0].get())->Get());
+  });
+}
+
 // Checks that no call stack profiles are captured if the profiling is stopped
 // during the initial delay.
 #if defined(STACK_SAMPLING_PROFILER_SUPPORTED)
@@ -778,7 +939,7 @@ TEST(StackSamplingProfilerTest, MAYBE_MultipleProfilesAndSamples) {
 #else
 #define MAYBE_StopDuringInitialDelay DISABLED_StopDuringInitialDelay
 #endif
-TEST(StackSamplingProfilerTest, MAYBE_StopDuringInitialDelay) {
+TEST_F(StackSamplingProfilerTest, MAYBE_StopDuringInitialDelay) {
   SamplingParams params;
   params.initial_delay = TimeDelta::FromSeconds(60);
 
@@ -795,7 +956,7 @@ TEST(StackSamplingProfilerTest, MAYBE_StopDuringInitialDelay) {
 #else
 #define MAYBE_StopDuringInterBurstInterval DISABLED_StopDuringInterBurstInterval
 #endif
-TEST(StackSamplingProfilerTest, MAYBE_StopDuringInterBurstInterval) {
+TEST_F(StackSamplingProfilerTest, MAYBE_StopDuringInterBurstInterval) {
   SamplingParams params;
   params.sampling_interval = TimeDelta::FromMilliseconds(0);
   params.burst_interval = TimeDelta::FromSeconds(60);
@@ -809,23 +970,58 @@ TEST(StackSamplingProfilerTest, MAYBE_StopDuringInterBurstInterval) {
   EXPECT_EQ(1u, profiles[0].samples.size());
 }
 
-// Checks that incomplete call stack profiles are captured.
+// Checks that tasks can be stopped before completion and incomplete call stack
+// profiles are captured.
 #if defined(STACK_SAMPLING_PROFILER_SUPPORTED)
 #define MAYBE_StopDuringInterSampleInterval StopDuringInterSampleInterval
 #else
 #define MAYBE_StopDuringInterSampleInterval \
   DISABLED_StopDuringInterSampleInterval
 #endif
-TEST(StackSamplingProfilerTest, MAYBE_StopDuringInterSampleInterval) {
-  SamplingParams params;
-  params.sampling_interval = TimeDelta::FromSeconds(60);
-  params.samples_per_burst = 2;
+TEST_F(StackSamplingProfilerTest, MAYBE_StopDuringInterSampleInterval) {
+  // Test delegate that counts samples.
+  class SampleRecordedEvent : public NativeStackSamplerTestDelegate {
+   public:
+    SampleRecordedEvent()
+        : sample_recorded_(WaitableEvent::ResetPolicy::MANUAL,
+                           WaitableEvent::InitialState::NOT_SIGNALED) {}
 
-  std::vector<CallStackProfile> profiles;
-  CaptureProfiles(params, TimeDelta::FromMilliseconds(50), &profiles);
+    void OnPreStackWalk() override { sample_recorded_.Signal(); }
 
-  ASSERT_EQ(1u, profiles.size());
-  EXPECT_EQ(1u, profiles[0].samples.size());
+    void Wait() { sample_recorded_.Wait(); }

[Mike Wittman, 2017/03/31 18:12:33]

nit: WaitForSampleToOccur

[bcwhite, 2017/04/03 20:18:13]

Done.

+
+   private:
+    WaitableEvent sample_recorded_;
+  };
+
+  WithTargetThread([](PlatformThreadId target_thread_id) {
+    std::vector<SamplingParams> params(1);
+
+    params[0].sampling_interval = AVeryLongTimeDelta();
+    params[0].samples_per_burst = 2;
+
+    std::vector<CallStackProfiles> profiles;
+    std::vector<std::unique_ptr<NativeStackSamplerTestDelegate>>
+        samples_recorded;
+    std::vector<std::unique_ptr<WaitableEvent>> sampling_completed;
+    std::vector<std::unique_ptr<StackSamplingProfiler>> profilers;
+    for (size_t i = 0; i < params.size(); ++i)
+      samples_recorded.push_back(MakeUnique<SampleRecordedEvent>());
+    CreateProfilers(target_thread_id, params, &samples_recorded, &profiles,
+                    &profilers, &sampling_completed);
+
+    profilers[0]->Start();
+
+    // Wait for profiler to start accumulating samples.
+    static_cast<SampleRecordedEvent*>(samples_recorded[0].get())->Wait();
+
+    // Ensure that it can stop safely.
+    profilers[0]->Stop();
+    sampling_completed[0]->Wait();
+
+    ASSERT_EQ(1u, profiles[0].size());
+    EXPECT_EQ(1u, profiles[0][0].samples.size());
+  });
 }
 
 // Checks that we can destroy the profiler while profiling.
@@ -835,7 +1031,7 @@ TEST(StackSamplingProfilerTest, MAYBE_StopDuringInterSampleInterval) {
 #define MAYBE_DestroyProfilerWhileProfiling \
   DISABLED_DestroyProfilerWhileProfiling
 #endif
-TEST(StackSamplingProfilerTest, MAYBE_DestroyProfilerWhileProfiling) {
+TEST_F(StackSamplingProfilerTest, MAYBE_DestroyProfilerWhileProfiling) {
   SamplingParams params;
   params.sampling_interval = TimeDelta::FromMilliseconds(10);
 
@@ -859,7 +1055,42 @@ TEST(StackSamplingProfilerTest, MAYBE_DestroyProfilerWhileProfiling) {
 #else
 #define MAYBE_CanRunMultipleTimes DISABLED_CanRunMultipleTimes
 #endif
-TEST(StackSamplingProfilerTest, MAYBE_CanRunMultipleTimes) {
+TEST_F(StackSamplingProfilerTest, MAYBE_CanRunMultipleTimes) {
+  WithTargetThread([](PlatformThreadId target_thread_id) {
+    SamplingParams params;
+    params.sampling_interval = TimeDelta::FromMilliseconds(0);
+    params.samples_per_burst = 1;
+
+    CallStackProfiles profiles;
+    WaitableEvent sampling_completed(WaitableEvent::ResetPolicy::MANUAL,
+                                     WaitableEvent::InitialState::NOT_SIGNALED);
+    const StackSamplingProfiler::CompletedCallback callback =
+        Bind(&SaveProfilesAndSignalEvent, Unretained(&profiles),
+             Unretained(&sampling_completed));
+    StackSamplingProfiler profiler(target_thread_id, params, callback);
+
+    // Just start and stop to execute code paths.
+    profiler.Start();
+    profiler.Stop();
+    sampling_completed.Wait();
+
+    // Ensure a second request will run and not block.
+    sampling_completed.Reset();
+    profiles.clear();
+    profiler.Start();
+    sampling_completed.Wait();
+    profiler.Stop();
+    ASSERT_EQ(1u, profiles.size());
+  });
+}
+
+// Checks that the different profilers may be run.
+#if defined(STACK_SAMPLING_PROFILER_SUPPORTED)
+#define MAYBE_CanRunMultipleProfilers CanRunMultipleProfilers
+#else
+#define MAYBE_CanRunMultipleProfilers DISABLED_CanRunMultipleProfilers
+#endif
+TEST_F(StackSamplingProfilerTest, MAYBE_CanRunMultipleProfilers) {
   SamplingParams params;
   params.sampling_interval = TimeDelta::FromMilliseconds(0);
   params.samples_per_burst = 1;
@@ -873,60 +1104,306 @@ TEST(StackSamplingProfilerTest, MAYBE_CanRunMultipleTimes) {
   ASSERT_EQ(1u, profiles.size());
 }
 
-// Checks that requests to start profiling while another profile is taking place
-// are ignored.
+// Checks that a sampler can be started while another is running.
 #if defined(STACK_SAMPLING_PROFILER_SUPPORTED)
-#define MAYBE_ConcurrentProfiling ConcurrentProfiling
+#define MAYBE_MultipleStart MultipleStart
 #else
-#define MAYBE_ConcurrentProfiling DISABLED_ConcurrentProfiling
+#define MAYBE_MultipleStart DISABLED_MultipleStart
 #endif
-TEST(StackSamplingProfilerTest, MAYBE_ConcurrentProfiling) {
+TEST_F(StackSamplingProfilerTest, MAYBE_MultipleStart) {
   WithTargetThread([](PlatformThreadId target_thread_id) {
-    SamplingParams params[2];
-    params[0].initial_delay = TimeDelta::FromMilliseconds(10);
-    params[0].sampling_interval = TimeDelta::FromMilliseconds(0);
+    std::vector<SamplingParams> params(2);
+
+    params[0].initial_delay = AVeryLongTimeDelta();
     params[0].samples_per_burst = 1;
 
-    params[1].sampling_interval = TimeDelta::FromMilliseconds(0);
+    params[1].sampling_interval = TimeDelta::FromMilliseconds(1);
     params[1].samples_per_burst = 1;
 
-    CallStackProfiles profiles[2];
-    std::vector<std::unique_ptr<WaitableEvent>> sampling_completed(2);
-    std::vector<std::unique_ptr<StackSamplingProfiler>> profiler(2);
-    for (int i = 0; i < 2; ++i) {
-      sampling_completed[i] =
-          MakeUnique<WaitableEvent>(WaitableEvent::ResetPolicy::AUTOMATIC,
-                                    WaitableEvent::InitialState::NOT_SIGNALED);
-      const StackSamplingProfiler::CompletedCallback callback =
-          Bind(&SaveProfilesAndSignalEvent, Unretained(&profiles[i]),
-               Unretained(sampling_completed[i].get()));
-      profiler[i] = MakeUnique<StackSamplingProfiler>(target_thread_id,
-                                                      params[i], callback);
-    }
+    std::vector<CallStackProfiles> profiles;
+    std::vector<std::unique_ptr<WaitableEvent>> sampling_completed;
+    std::vector<std::unique_ptr<StackSamplingProfiler>> profilers;
+    CreateProfilers(target_thread_id, params, nullptr, &profiles, &profilers,
+                    &sampling_completed);
+
+    profilers[0]->Start();
+    profilers[1]->Start();
+    sampling_completed[1]->Wait();
+    EXPECT_EQ(1u, profiles[1].size());
+  });
+}
+
+// Checks that the sampling thread can shut down.
+#if defined(STACK_SAMPLING_PROFILER_SUPPORTED)
+#define MAYBE_SamplerIdleShutdown SamplerIdleShutdown
+#else
+#define MAYBE_SamplerIdleShutdown DISABLED_SamplerIdleShutdown
+#endif
+TEST_F(StackSamplingProfilerTest, MAYBE_SamplerIdleShutdown) {
+  SamplingParams params;
+  params.sampling_interval = TimeDelta::FromMilliseconds(0);
+  params.samples_per_burst = 1;
 
-    profiler[0]->Start();
-    profiler[1]->Start();
+  std::vector<CallStackProfile> profiles;
+  CaptureProfiles(params, AVeryLongTimeDelta(), &profiles);
+  ASSERT_EQ(1u, profiles.size());
+
+  // Capture thread should still be running at this point.
+  ASSERT_TRUE(StackSamplingProfiler::TestAPI::IsSamplingThreadRunning());
+
+  // Initiate an "idle" shutdown and ensure it happens. Idle-shutdown was

[Mike Wittman, 2017/03/31 18:12:33]

Idle-shutdown is disabled in the test fixture ...

[bcwhite, 2017/04/03 20:18:13]

Done.

+  // disabled above so the test will fail due to a timeout if it does not
+  // exit.
+  StackSamplingProfiler::TestAPI::PerformSamplingThreadIdleShutdown(false);
+
+  // While the shutdown has been initiated, the actual exit of the thread still
+  // happens asynchronously. Watch until the thread actually exits. This test
+  // will time-out in the case of failure.
+  while (StackSamplingProfiler::TestAPI::IsSamplingThreadRunning())
+    PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(1));
+}
+
+// Checks that additional requests will restart a stopped profiler.
+#if defined(STACK_SAMPLING_PROFILER_SUPPORTED)
+#define MAYBE_WillRestartSamplerAfterIdleShutdown \
+  WillRestartSamplerAfterIdleShutdown
+#else
+#define MAYBE_WillRestartSamplerAfterIdleShutdown \
+  DISABLED_WillRestartSamplerAfterIdleShutdown
+#endif
+TEST_F(StackSamplingProfilerTest, MAYBE_WillRestartSamplerAfterIdleShutdown) {
+  SamplingParams params;
+  params.sampling_interval = TimeDelta::FromMilliseconds(0);
+  params.samples_per_burst = 1;
+
+  std::vector<CallStackProfile> profiles;
+  CaptureProfiles(params, AVeryLongTimeDelta(), &profiles);
+  ASSERT_EQ(1u, profiles.size());
+
+  // Capture thread should still be running at this point.
+  ASSERT_TRUE(StackSamplingProfiler::TestAPI::IsSamplingThreadRunning());
+
+  // Post a ShutdownTask on the sampling thread which, when executed, will
+  // mark the thread as EXITING and begin shut down of the thread.
+  StackSamplingProfiler::TestAPI::PerformSamplingThreadIdleShutdown(false);
+
+  // Ensure another capture will start the sampling thread and run.
+  profiles.clear();
+  CaptureProfiles(params, AVeryLongTimeDelta(), &profiles);
+  ASSERT_EQ(1u, profiles.size());
+  EXPECT_TRUE(StackSamplingProfiler::TestAPI::IsSamplingThreadRunning());
+}
+
+// Checks that it's safe to stop a task after it's completed and the sampling
+// thread has shut-down for being idle.
+#if defined(STACK_SAMPLING_PROFILER_SUPPORTED)
+#define MAYBE_StopAfterIdleShutdown StopAfterIdleShutdown
+#else
+#define MAYBE_StopAfterIdleShutdown DISABLED_StopAfterIdleShutdown
+#endif
+TEST_F(StackSamplingProfilerTest, MAYBE_StopAfterIdleShutdown) {
+  WithTargetThread([](PlatformThreadId target_thread_id) {
+    std::vector<SamplingParams> params(1);
+
+    params[0].sampling_interval = TimeDelta::FromMilliseconds(1);
+    params[0].samples_per_burst = 1;
+
+    std::vector<CallStackProfiles> profiles;
+    std::vector<std::unique_ptr<WaitableEvent>> sampling_completed;
+    std::vector<std::unique_ptr<StackSamplingProfiler>> profilers;
+    CreateProfilers(target_thread_id, params, nullptr, &profiles, &profilers,
+                    &sampling_completed);
+
+    profilers[0]->Start();
+    sampling_completed[0]->Wait();
+
+    // Capture thread should still be running at this point.
+    ASSERT_TRUE(StackSamplingProfiler::TestAPI::IsSamplingThreadRunning());
+
+    // Perform an idle shutdown.
+    StackSamplingProfiler::TestAPI::PerformSamplingThreadIdleShutdown(false);
+
+    // Stop should be safe though its impossible to know at this moment if the
+    // sampling thread has completely exited or will just "stop soon".
+    profilers[0]->Stop();
+  });
+}
+
+// Checks that profilers can run both before and after the sampling thread has
+// started.
+#if defined(STACK_SAMPLING_PROFILER_SUPPORTED)
+#define MAYBE_ProfileBeforeAndAfterSamplingThreadRunning \
+  ProfileBeforeAndAfterSamplingThreadRunning
+#else
+#define MAYBE_ProfileBeforeAndAfterSamplingThreadRunning \
+  DISABLED_ProfileBeforeAndAfterSamplingThreadRunning
+#endif
+TEST_F(StackSamplingProfilerTest,
+       MAYBE_ProfileBeforeAndAfterSamplingThreadRunning) {
+  WithTargetThread([](PlatformThreadId target_thread_id) {
+    std::vector<SamplingParams> params(2);
+
+    params[0].initial_delay = AVeryLongTimeDelta();
+    params[0].sampling_interval = TimeDelta::FromMilliseconds(1);
+    params[0].samples_per_burst = 1;
+
+    params[1].initial_delay = TimeDelta::FromMilliseconds(0);
+    params[1].sampling_interval = TimeDelta::FromMilliseconds(1);
+    params[1].samples_per_burst = 1;
+
+    std::vector<CallStackProfiles> profiles;
+    std::vector<std::unique_ptr<WaitableEvent>> sampling_completed;
+    std::vector<std::unique_ptr<StackSamplingProfiler>> profilers;
+    CreateProfilers(target_thread_id, params, nullptr, &profiles, &profilers,
+                    &sampling_completed);
+
+    // First profiler is started when there has never been a sampling thread.
+    EXPECT_FALSE(StackSamplingProfiler::TestAPI::IsSamplingThreadRunning());
+    profilers[0]->Start();
+    // Second profiler is started when sampling thread is already running.
+    EXPECT_TRUE(StackSamplingProfiler::TestAPI::IsSamplingThreadRunning());
+    profilers[1]->Start();
+
+    // Only the second profiler should finish before test times out.
+    size_t completed_profiler = WaitForSamplingComplete(&sampling_completed);
+    EXPECT_EQ(1U, completed_profiler);
+  });
+}
+
+// Checks that an idle-shutdown task will abort if a new profiler starts
+// between when it was posted and when it runs.
+#if defined(STACK_SAMPLING_PROFILER_SUPPORTED)
+#define MAYBE_IdleShutdownAbort IdleShutdownAbort
+#else
+#define MAYBE_IdleShutdownAbort DISABLED_IdleShutdownAbort
+#endif
+TEST_F(StackSamplingProfilerTest, MAYBE_IdleShutdownAbort) {
+  WithTargetThread([](PlatformThreadId target_thread_id) {
+    // CreateProfilers takes vectors so use a vector for this even though its
+    // only a single entry.
+    std::vector<SamplingParams> params(1);
+
+    params[0].sampling_interval = TimeDelta::FromMilliseconds(1);
+    params[0].samples_per_burst = 1;
+
+    std::vector<CallStackProfiles> profiles;
+    std::vector<std::unique_ptr<WaitableEvent>> sampling_completed;
+    std::vector<std::unique_ptr<StackSamplingProfiler>> profilers;
+    CreateProfilers(target_thread_id, params, nullptr, &profiles, &profilers,
+                    &sampling_completed);
+
+    profilers[0]->Start();
+    sampling_completed[0]->Wait();
+    EXPECT_EQ(1u, profiles.size());
+
+    // Perform an idle shutdown but simulate that a new capture is started
+    // before it can actually run.
+    StackSamplingProfiler::TestAPI::PerformSamplingThreadIdleShutdown(true);
+
+    // Though the shutdown-task has been executed, any actual exit of the
+    // thread is asynchronous so there is no way to detect that *didn't* exit
+    // except to wait a reasonable amount of time and then check. Since the
+    // thread was just running ("perform" blocked until it was), it should
+    // finish almost immediately and without any waiting for tasks or events.
+    PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(200));
+    EXPECT_TRUE(StackSamplingProfiler::TestAPI::IsSamplingThreadRunning());
+
+    // Ensure that it's still possible to run another sampler. Restarting the
+    // already defined sampler is fine.
+    profilers[0]->Stop();
+    profiles[0].clear();
+    profilers[0]->Start();
+    sampling_completed[0]->Wait();
+    EXPECT_EQ(1u, profiles[0].size());
+  });
+}
+
+// Checks that synchronized multiple sampling requests execute in parallel.
+#if defined(STACK_SAMPLING_PROFILER_SUPPORTED)
+#define MAYBE_ConcurrentProfiling_InSync ConcurrentProfiling_InSync
+#else
+#define MAYBE_ConcurrentProfiling_InSync DISABLED_ConcurrentProfiling_InSync
+#endif
+TEST_F(StackSamplingProfilerTest, MAYBE_ConcurrentProfiling_InSync) {
+  WithTargetThread([](PlatformThreadId target_thread_id) {
+    std::vector<SamplingParams> params(2);
+
+    // Providing an initial delay makes it more likely that both will be
+    // scheduled before either starts to run. Once started, samples will
+    // run ordered by their scheduled, interleaved times regardless of
+    // whatever interval the thread wakes up. Thus, total execution time
+    // will be 10ms (delay) + 10x1ms (sampling) + 1/2 timer minimum interval.
+    params[0].initial_delay = TimeDelta::FromMilliseconds(10);
+    params[0].sampling_interval = TimeDelta::FromMilliseconds(1);
+    params[0].samples_per_burst = 9;
+
+    params[1].initial_delay = TimeDelta::FromMilliseconds(11);
+    params[1].sampling_interval = TimeDelta::FromMilliseconds(1);
+    params[1].samples_per_burst = 8;
+
+    std::vector<CallStackProfiles> profiles;
+    std::vector<std::unique_ptr<WaitableEvent>> sampling_completed;
+    std::vector<std::unique_ptr<StackSamplingProfiler>> profilers;
+    CreateProfilers(target_thread_id, params, nullptr, &profiles, &profilers,
+                    &sampling_completed);
+
+    profilers[0]->Start();
+    profilers[1]->Start();
 
-    std::vector<WaitableEvent*> sampling_completed_rawptrs(
-        sampling_completed.size());
-    std::transform(
-        sampling_completed.begin(), sampling_completed.end(),
-        sampling_completed_rawptrs.begin(),
-        [](const std::unique_ptr<WaitableEvent>& elem) { return elem.get(); });
     // Wait for one profiler to finish.
-    size_t completed_profiler =
-        WaitableEvent::WaitMany(sampling_completed_rawptrs.data(), 2);
-    EXPECT_EQ(1u, profiles[completed_profiler].size());
+    size_t completed_profiler = WaitForSamplingComplete(&sampling_completed);
+    ASSERT_EQ(1u, profiles[completed_profiler].size());
 
     size_t other_profiler = 1 - completed_profiler;
-    // Give the other profiler a chance to run and observe that it hasn't.
-    EXPECT_FALSE(sampling_completed[other_profiler]->TimedWait(
-        TimeDelta::FromMilliseconds(25)));
-
-    // Start the other profiler again and it should run.
-    profiler[other_profiler]->Start();
+    // Wait for the other profiler to finish.
     sampling_completed[other_profiler]->Wait();
-    EXPECT_EQ(1u, profiles[other_profiler].size());
+    ASSERT_EQ(1u, profiles[other_profiler].size());
+
+    // Ensure each got the correct number of samples.
+    EXPECT_EQ(9u, profiles[0][0].samples.size());
+    EXPECT_EQ(8u, profiles[1][0].samples.size());
+  });
+}
+
+// Checks that several mixed sampling requests execute in parallel.
+#if defined(STACK_SAMPLING_PROFILER_SUPPORTED)
+#define MAYBE_ConcurrentProfiling_Mixed ConcurrentProfiling_Mixed
+#else
+#define MAYBE_ConcurrentProfiling_Mixed DISABLED_ConcurrentProfiling_Mixed
+#endif
+TEST_F(StackSamplingProfilerTest, MAYBE_ConcurrentProfiling_Mixed) {
+  WithTargetThread([](PlatformThreadId target_thread_id) {
+    std::vector<SamplingParams> params(3);
+
+    params[0].initial_delay = TimeDelta::FromMilliseconds(8);
+    params[0].sampling_interval = TimeDelta::FromMilliseconds(4);
+    params[0].samples_per_burst = 10;
+
+    params[1].initial_delay = TimeDelta::FromMilliseconds(9);
+    params[1].sampling_interval = TimeDelta::FromMilliseconds(3);
+    params[1].samples_per_burst = 10;
+
+    params[2].initial_delay = TimeDelta::FromMilliseconds(10);
+    params[2].sampling_interval = TimeDelta::FromMilliseconds(2);
+    params[2].samples_per_burst = 10;
+
+    std::vector<CallStackProfiles> profiles;
+    std::vector<std::unique_ptr<WaitableEvent>> sampling_completed;
+    std::vector<std::unique_ptr<StackSamplingProfiler>> profilers;
+    CreateProfilers(target_thread_id, params, nullptr, &profiles, &profilers,
+                    &sampling_completed);
+
+    for (size_t i = 0; i < profilers.size(); ++i)
+      profilers[i]->Start();
+
+    // Wait for one profiler to finish.
+    size_t completed_profiler = WaitForSamplingComplete(&sampling_completed);
+    EXPECT_EQ(1u, profiles[completed_profiler].size());
+    // Stop and destroy all profilers, always in the same order. Don't crash.
+    for (size_t i = 0; i < profilers.size(); ++i)
+      profilers[i]->Stop();
+    for (size_t i = 0; i < profilers.size(); ++i)
+      profilers[i].reset();
   });
 }
 
@@ -937,7 +1414,7 @@ TEST(StackSamplingProfilerTest, MAYBE_ConcurrentProfiling) {
 #else
 #define MAYBE_OtherLibrary DISABLED_OtherLibrary
 #endif
-TEST(StackSamplingProfilerTest, MAYBE_OtherLibrary) {
+TEST_F(StackSamplingProfilerTest, MAYBE_OtherLibrary) {
   SamplingParams params;
   params.sampling_interval = TimeDelta::FromMilliseconds(0);
   params.samples_per_burst = 1;
@@ -1008,7 +1485,7 @@ TEST(StackSamplingProfilerTest, MAYBE_OtherLibrary) {
 #else
 #define MAYBE_UnloadingLibrary DISABLED_UnloadingLibrary
 #endif
-TEST(StackSamplingProfilerTest, MAYBE_UnloadingLibrary) {
+TEST_F(StackSamplingProfilerTest, MAYBE_UnloadingLibrary) {
   TestLibraryUnload(false);
 }
 
@@ -1019,8 +1496,153 @@ TEST(StackSamplingProfilerTest, MAYBE_UnloadingLibrary) {
 #else
 #define MAYBE_UnloadedLibrary DISABLED_UnloadedLibrary
 #endif
-TEST(StackSamplingProfilerTest, MAYBE_UnloadedLibrary) {
+TEST_F(StackSamplingProfilerTest, MAYBE_UnloadedLibrary) {
   TestLibraryUnload(true);
 }
 
+// Checks that different threads can be sampled in parallel.
+#if defined(STACK_SAMPLING_PROFILER_SUPPORTED)
+#define MAYBE_MultipleSampledThreads MultipleSampledThreads
+#else
+#define MAYBE_MultipleSampledThreads DISABLED_MultipleSampledThreads
+#endif
+TEST_F(StackSamplingProfilerTest, MAYBE_MultipleSampledThreads) {
+  // Create target threads. The extra parethesis around the StackConfiguration
+  // call are to avoid the most-vexing-parse problem.
+  TargetThread target_thread1((StackConfiguration(StackConfiguration::NORMAL)));
+  TargetThread target_thread2((StackConfiguration(StackConfiguration::NORMAL)));
+  PlatformThreadHandle target_thread_handle1, target_thread_handle2;
+  EXPECT_TRUE(
+      PlatformThread::Create(0, &target_thread1, &target_thread_handle1));
+  EXPECT_TRUE(
+      PlatformThread::Create(0, &target_thread2, &target_thread_handle2));
+  target_thread1.WaitForThreadStart();
+  target_thread2.WaitForThreadStart();
+
+  // Providing an initial delay makes it more likely that both will be
+  // scheduled before either starts to run. Once started, samples will
+  // run ordered by their scheduled, interleaved times regardless of
+  // whatever interval the thread wakes up.
+  SamplingParams params1, params2;
+  params1.initial_delay = TimeDelta::FromMilliseconds(10);
+  params1.sampling_interval = TimeDelta::FromMilliseconds(1);
+  params1.samples_per_burst = 9;
+  params2.initial_delay = TimeDelta::FromMilliseconds(10);
+  params2.sampling_interval = TimeDelta::FromMilliseconds(1);
+  params2.samples_per_burst = 8;
+
+  std::vector<CallStackProfile> profiles1, profiles2;
+
+  WaitableEvent sampling_thread_completed1(
+      WaitableEvent::ResetPolicy::MANUAL,
+      WaitableEvent::InitialState::NOT_SIGNALED);
+  const StackSamplingProfiler::CompletedCallback callback1 =
+      Bind(&SaveProfilesAndSignalEvent, Unretained(&profiles1),
+           Unretained(&sampling_thread_completed1));
+  StackSamplingProfiler profiler1(target_thread1.id(), params1, callback1);
+
+  WaitableEvent sampling_thread_completed2(
+      WaitableEvent::ResetPolicy::MANUAL,
+      WaitableEvent::InitialState::NOT_SIGNALED);
+  const StackSamplingProfiler::CompletedCallback callback2 =
+      Bind(&SaveProfilesAndSignalEvent, Unretained(&profiles2),
+           Unretained(&sampling_thread_completed2));
+  StackSamplingProfiler profiler2(target_thread2.id(), params2, callback2);
+
+  // Finally the real work.
+  profiler1.Start();
+  profiler2.Start();
+  sampling_thread_completed1.Wait();
+  sampling_thread_completed2.Wait();
+  ASSERT_EQ(1u, profiles1.size());
+  EXPECT_EQ(9u, profiles1[0].samples.size());
+  ASSERT_EQ(1u, profiles2.size());
+  EXPECT_EQ(8u, profiles2[0].samples.size());
+
+  target_thread1.SignalThreadToFinish();
+  target_thread2.SignalThreadToFinish();
+  PlatformThread::Join(target_thread_handle1);
+  PlatformThread::Join(target_thread_handle2);
+}
+
+// A simple thread that runs a profiler on another thread.
+class ProfilerThread : public SimpleThread {
+ public:
+  ProfilerThread(const std::string& name,
+                 PlatformThreadId thread_id,
+                 const SamplingParams& params)
+      : SimpleThread(name, Options()),
+        run_(WaitableEvent::ResetPolicy::MANUAL,
+             WaitableEvent::InitialState::NOT_SIGNALED),
+        completed_(WaitableEvent::ResetPolicy::MANUAL,
+                   WaitableEvent::InitialState::NOT_SIGNALED),
+        profiler_(thread_id,
+                  params,
+                  Bind(&SaveProfilesAndSignalEvent,
+                       Unretained(&profiles_),
+                       Unretained(&completed_))) {}
+
+  void Run() override {
+    run_.Wait();
+    profiler_.Start();
+  }
+
+  void Go() { run_.Signal(); }
+
+  void Wait() { completed_.Wait(); }
+
+  CallStackProfiles& profiles() { return profiles_; }
+
+ private:
+  WaitableEvent run_;
+
+  CallStackProfiles profiles_;
+  WaitableEvent completed_;
+  StackSamplingProfiler profiler_;
+};
+
+// Checks that different threads can run samplers in parallel.
+#if defined(STACK_SAMPLING_PROFILER_SUPPORTED)
+#define MAYBE_MultipleProfilerThreads MultipleProfilerThreads
+#else
+#define MAYBE_MultipleProfilerThreads DISABLED_MultipleProfilerThreads
+#endif
+TEST_F(StackSamplingProfilerTest, MAYBE_MultipleProfilerThreads) {
+  WithTargetThread([](PlatformThreadId target_thread_id) {
+    // Providing an initial delay makes it more likely that both will be
+    // scheduled before either starts to run. Once started, samples will
+    // run ordered by their scheduled, interleaved times regardless of
+    // whatever interval the thread wakes up.
+    SamplingParams params1, params2;
+    params1.initial_delay = TimeDelta::FromMilliseconds(10);
+    params1.sampling_interval = TimeDelta::FromMilliseconds(1);
+    params1.samples_per_burst = 9;
+    params2.initial_delay = TimeDelta::FromMilliseconds(10);
+    params2.sampling_interval = TimeDelta::FromMilliseconds(1);
+    params2.samples_per_burst = 8;
+
+    // Start the profiler threads and give them a moment to get going.
+    ProfilerThread profiler1("profiler_thread1", target_thread_id, params1);
+    ProfilerThread profiler2("profiler_thread2", target_thread_id, params2);
+    profiler1.Start();
+    profiler2.Start();
+    PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(10));
+
+    // This will (approximately) synchronize the two threads.
+    profiler1.Go();
+    profiler2.Go();
+
+    // Wait for them both to finish and validate collection.
+    profiler1.Wait();
+    profiler2.Wait();
+    ASSERT_EQ(1u, profiler1.profiles().size());
+    EXPECT_EQ(9u, profiler1.profiles()[0].samples.size());
+    ASSERT_EQ(1u, profiler2.profiles().size());
+    EXPECT_EQ(8u, profiler2.profiles()[0].samples.size());
+
+    profiler1.Join();
+    profiler2.Join();
+  });
+}
+
 }  // namespace base