Ensure SequencedWorkerPool::Shutdown() blocks for already-started SKIP_ON_SHUTDOWN tasks

base/threading/sequenced_worker_pool_unittest.cc

Index: base/threading/sequenced_worker_pool_unittest.cc
diff --git a/base/threading/sequenced_worker_pool_unittest.cc b/base/threading/sequenced_worker_pool_unittest.cc
index 54ca01dba0c173b64c82124a971ed4b056c0acc6..306e0287b10649863623dd713a7b8ff639767801 100644
--- a/base/threading/sequenced_worker_pool_unittest.cc
+++ b/base/threading/sequenced_worker_pool_unittest.cc
@@ -399,11 +399,12 @@ TEST_F(SequencedWorkerPoolTest, DiscardOnShutdown) {
                  &blocker, kNumWorkerThreads));
   pool()->Shutdown();
 
-  std::vector<int> result = tracker()->WaitUntilTasksComplete(4);
+  std::vector<int> result =
+      tracker()->WaitUntilTasksComplete(kNumWorkerThreads + 1);
 
   // The kNumWorkerThread items should have completed, plus the BLOCK_SHUTDOWN
   // one, in no particular order.
-  ASSERT_EQ(4u, result.size());
+  ASSERT_EQ(kNumWorkerThreads + 1, result.size());
   for (size_t i = 0; i < kNumWorkerThreads; i++) {
     EXPECT_TRUE(std::find(result.begin(), result.end(), static_cast<int>(i)) !=
                 result.end());
@@ -458,6 +459,58 @@ TEST_F(SequencedWorkerPoolTest, ContinueOnShutdown) {
   EXPECT_EQ(3u, result.size());
 }
 
+// Tests that SKIP_ON_SHUTDOWN tasks that have been started block Shutdown
+// until they stop, but tasks not yet started do not.
+TEST_F(SequencedWorkerPoolTest, SkipOnShutdown) {
+  // Start tasks to take all the threads and block them.
+  EnsureAllWorkersCreated();
+  ThreadBlocker blocker;
+
+  // Now block all the threads with SKIP_ON_SHUTDOWN. Shutdown() should not
+  // return until these tasks have completed.
+  for (size_t i = 0; i < kNumWorkerThreads; i++) {
+    pool()->PostWorkerTaskWithShutdownBehavior(
+        FROM_HERE,
+        base::Bind(&TestTracker::BlockTask, tracker(), i, &blocker),
+        SequencedWorkerPool::SKIP_ON_SHUTDOWN);
+  }
+  tracker()->WaitUntilTasksBlocked(kNumWorkerThreads);
+
+  // Now post an additional task as SKIP_ON_SHUTDOWN, which should not be
+  // executed once Shutdown() has been called.
+  pool()->PostWorkerTaskWithShutdownBehavior(
+      FROM_HERE,
+      base::Bind(&TestTracker::BlockTask,
+                 tracker(), 0, &blocker),
+      SequencedWorkerPool::SKIP_ON_SHUTDOWN);
+
+  // This callback will only be invoked if SKIP_ON_SHUTDOWN tasks that have
+  // been started block shutdown.
+  SetWillWaitForShutdownCallback(
+      base::Bind(&EnsureTasksToCompleteCountAndUnblock,
+                 scoped_refptr<TestTracker>(tracker()), 0,
+                 &blocker, kNumWorkerThreads));
+
+  // No tasks should have completed yet.
+  EXPECT_EQ(0u, tracker()->WaitUntilTasksComplete(0).size());
+
+  // This should not block. If this test hangs, it means it failed.
+  pool()->Shutdown();

[jar (doing other things), 2012/07/20 02:21:17]

Do you think there is a another test where we post a BLOCK_SHUTDOWN task after
calling shutdown?

[Ryan Sleevi, 2012/07/20 02:53:59]

No, but I've added one.

+
+  // Shutdown should not return until all of the tasks have completed.
+  std::vector<int> result =
+      tracker()->WaitUntilTasksComplete(kNumWorkerThreads);
+
+  // Only taks marked SKIP_ON_SHUTDOWN that were already started should be

[jar (doing other things), 2012/07/20 02:21:17]

nit: taks-->tasks

+  // allowed to complete. No additional non-blocking tasks should have been
+  // started.
+  ASSERT_EQ(kNumWorkerThreads, result.size());
+  for (size_t i = 0; i < kNumWorkerThreads; i++) {
+    EXPECT_TRUE(std::find(result.begin(), result.end(), static_cast<int>(i)) !=
+                result.end());
+  }
+}
+
 // Ensure all worker threads are created, and then trigger a spurious
 // work signal. This shouldn't cause any other work signals to be
 // triggered. This is a regression test for http://crbug.com/117469.