Use SequenceChecker to allow Timer to run in SequencedWorkerPool

base/timer/timer_unittest.cc

Index: base/timer/timer_unittest.cc
diff --git a/base/timer/timer_unittest.cc b/base/timer/timer_unittest.cc
index 35e4315ea46368b392fd7e861398ba0440eb8a79..1a39821b76280b7442f6436d2a5a8c7fc563e28e 100644
--- a/base/timer/timer_unittest.cc
+++ b/base/timer/timer_unittest.cc
@@ -2,14 +2,17 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/callback.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
+#include "base/synchronization/waitable_event.h"
+#include "base/test/sequenced_worker_pool_owner.h"
 #include "base/test/test_simple_task_runner.h"
 #include "base/timer/timer.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+using base::SequencedTaskRunner;
 using base::TimeDelta;
-using base::SingleThreadTaskRunner;
 
 namespace {
 
@@ -37,7 +40,7 @@ class OneShotTimerTester {
                  &OneShotTimerTester::Run);
   }
 
-  void SetTaskRunner(scoped_refptr<SingleThreadTaskRunner> task_runner) {
+  void SetTaskRunner(scoped_refptr<SequencedTaskRunner> task_runner) {
     quit_message_loop_ = false;
     timer_.SetTaskRunner(task_runner);
   }
@@ -528,4 +531,98 @@ TEST(TimerTest, ContinuationReset) {
   }
 }
 
+const size_t kNumWorkerThreads = 3;
+
+// Fixture for tests requiring a worker pool. Includes a WaitableEvent so
+// that cases may Wait() on one thread and Signal() (explicitly, or implicitly
+// via helper methods) on another.
+class TimerSequenceTest : public testing::Test {
+ public:
+  TimerSequenceTest()
+      : event_(false /*manual_reset*/, true /*initially_signaled*/) {}

[gab, 2015/12/03 22:26:08]

Add space padding in /* comment */

[jsbell, 2015/12/05 01:15:39]

Done.

+
+  void SetUp() override { ResetPool(); }
+
+  void TearDown() override { pool()->Shutdown(); }
+
+  // Block until Signal() is called on another thread.
+  void Wait() { event_.Wait(); }
+
+  void Signal() { event_.Signal(); }
+
+  // Explicitly create the timer. Not done in the constructor so that test
+  // cases demonstrate creation on a specific thread.
+  void CreateTimer() { timer_.reset(new base::OneShotTimer); }
+
+  // Start the timer; when the timer fires it will Signal(), so Wait() can
+  // be called on another thread to await this.
+  void StartTimerWillSignal() {
+    timer_->Start(FROM_HERE, TimeDelta::FromMilliseconds(1), this,
+                  &TimerSequenceTest::Signal);
+  }
+
+  // Delete the timer then Signal() so that this can be posted as a task
+  // to another thread and the poster can Wait().
+  void DeleteTimerAndSignal() {
+    timer_.reset();
+    Signal();
+  }
+
+ protected:
+  const scoped_refptr<base::SequencedWorkerPool>& pool() {
+    return pool_owner_->pool();
+  }
+
+  // Destroys the SequencedWorkerPool instance, blocking until it is fully shut
+  // down, and creates a new instance.
+  void ResetPool() {
+    pool_owner_.reset(
+        new base::SequencedWorkerPoolOwner(kNumWorkerThreads, "test"));
+  }
+
+  scoped_ptr<base::OneShotTimer> timer_;
+
+ private:
+  base::WaitableEvent event_;
+
+  base::MessageLoop message_loop_;
+  scoped_ptr<base::SequencedWorkerPoolOwner> pool_owner_;
+};

[gab, 2015/12/03 22:26:08]

DISALLOW_COPY_AND_ASSIGN

[jsbell, 2015/12/05 01:15:39]

Done.

+
+TEST_F(TimerSequenceTest, OneShotTimerTaskOnPoolThread) {
+  scoped_refptr<base::SequencedTaskRunner> task_runner =
+      pool()->GetSequencedTaskRunner(pool()->GetSequenceToken());
+
+  // Timer is created on this thread.
+  CreateTimer();
+
+  // Task will execute on a pool thread.
+  timer_->SetTaskRunner(task_runner.get());
+  StartTimerWillSignal();
+  Wait();
+
+  // Timer will be destroyed on this thread.
+  timer_.reset();
+}
+
+TEST_F(TimerSequenceTest, OneShotTimerUsedOnPoolThread) {
+  scoped_refptr<base::SequencedTaskRunner> task_runner =
+      pool()->GetSequencedTaskRunner(pool()->GetSequenceToken());
+
+  // Timer is created on this thread.
+  CreateTimer();
+
+  // Task will execute on a pool thread.
+  task_runner->PostTask(FROM_HERE,
+                        base::Bind(&TimerSequenceTest::StartTimerWillSignal,
+                                   base::Unretained(this)));
+  Wait();
+
+  // Timer must be destroyed on pool thread, too.
+  task_runner->PostTask(FROM_HERE,
+                        base::Bind(&TimerSequenceTest::DeleteTimerAndSignal,
+                                   base::Unretained(this)));
+  Wait();
+}
+

[gab, 2015/12/03 22:26:09]

Feels a test that would:
1) Create a |timer| from pool1
2) timer->SetTaskRunner(pool2)

with following test case:
3) timer->Start(&Task, base::Time::FromHours(1))
4) timer->AbandonScheduledTask()

and:
3) timer->Start(&Task, base::Time::FromMilliseconds(1))
4) Wait for Task to Signal (it verifies it ran on the proper pool)?


Does that sound right? Feels this isn't covered currently.

[jsbell, 2015/12/05 01:15:39]

Those sound great - done and done. Any others?

 }  // namespace