base: Change WeakPtr to use SequenceChecker instead of ThreadChecker.

base/sequence_checker_unittest.cc

-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Copyright 2013 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/basictypes.h"
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/location.h"
+#include "base/logging.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/message_loop.h"
 #include "base/sequence_checker.h"
-
-#include <cstddef>
-
-#include "base/compiler_specific.h"
-#include "base/memory/ref_counted.h"
-#include "base/test/null_task_runner.h"
+#include "base/test/sequenced_worker_pool_owner.h"
+#include "base/threading/simple_thread.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+// Duplicated from base/sequence_checker.h so that we can be good citizens
+// there and undef the macro.
+#if (!defined(NDEBUG) || defined(DCHECK_ALWAYS_ON))
+#define ENABLE_SEQUENCE_CHECKER 1
+#else
+#define ENABLE_SEQUENCE_CHECKER 0
+#endif
+
 namespace base {
 
 namespace {
 
-// Call various methods of SequenceChecker to make sure nothing blows
-// up in either debug or release mode.
-TEST(SequenceCheckerTest, Basic) {
-  SequenceChecker sequence_checker(new NullTaskRunner());
-  sequence_checker.CalledOnValidSequence();
-  sequence_checker.ChangeSequence(NULL);
-  sequence_checker.CalledOnValidSequence();
-}
+const size_t kNumWorkerThreads = 3;
+
+// Simple class to exercise the basics of SequenceChecker.
+// DoStuff should verify that it's called on a valid sequenced thread.

[akalin, 2013/07/12 01:09:21]

add comment like "SequenceCheckedObject can be destroyed on any thread (like
WeakPtr)".

[tommycli, 2013/07/12 19:16:25]

Done.

+class SequenceCheckerClass : public SequenceChecker {

[akalin, 2013/07/12 01:09:21]

in addition to having the SequenceChecker be a member, how 'bout naming this
class SequenceCheckedObject? Having 'Class' in a class name is a bit of a code
smell.

[tommycli, 2013/07/12 19:16:25]

Done.

+ public:
+  SequenceCheckerClass() {}
+  ~SequenceCheckerClass() {}
+
+  // Verifies that it was called on the same thread as the constructor.
+  void DoStuff() {
+    DCHECK(CalledOnValidSequencedThread());
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(SequenceCheckerClass);
+};
+
+class CallDoStuffThread : public base::SimpleThread {

[akalin, 2013/07/12 01:09:21]

rather than create a different thread for each thing you want to do, why not
create a base::Thread object (which comes with a message loop) then post tasks
to it? I feel like that would be simpler.

[tommycli, 2013/07/12 19:16:25]

Done.

+ public:
+  explicit CallDoStuffThread(SequenceCheckerClass* sequence_checker_class)
+      : SimpleThread("call_do_stuff_thread"),
+        sequence_checker_class_(sequence_checker_class) {

[akalin, 2013/07/12 01:09:21]

then rename sequence_checker_class etc. to sequence_checked_object

[tommycli, 2013/07/12 19:16:25]

Done.

+  }
+
+  virtual void Run() OVERRIDE {
+    sequence_checker_class_->DoStuff();
+  }
+
+ private:
+  SequenceCheckerClass* sequence_checker_class_;
+
+  DISALLOW_COPY_AND_ASSIGN(CallDoStuffThread);
+};
+
+class DeleteThreadCheckerClassThread : public base::SimpleThread {
+ public:
+  explicit DeleteThreadCheckerClassThread(
+      SequenceCheckerClass* sequence_checker_class)

[akalin, 2013/07/12 01:09:21]

have this take a scoped_ptr<SequenceCheckerClass> and use ptr.Pass() when
passing to this class

[tommycli, 2013/07/12 19:16:25]

Done.

+      : SimpleThread("delete_sequence_checker_class_thread"),
+        sequence_checker_class_(sequence_checker_class) {
+  }
+
+  virtual void Run() OVERRIDE {
+    sequence_checker_class_.reset();
+  }
+
+ private:
+  scoped_ptr<SequenceCheckerClass> sequence_checker_class_;
+
+  DISALLOW_COPY_AND_ASSIGN(DeleteThreadCheckerClassThread);
+};
+
+class SequenceCheckerTest : public testing::Test {
+ public:
+  SequenceCheckerTest() {
+    ResetPool();

[akalin, 2013/07/12 01:09:21]

may as well put this in SetUp() (to mirror the shutdown call)

[tommycli, 2013/07/12 19:16:25]

Done.

+  }
+
+  virtual ~SequenceCheckerTest() {}
+
+  virtual void SetUp() OVERRIDE {}
+
+  virtual void TearDown() OVERRIDE {
+    pool()->Shutdown();
+  }
+
+ protected:
+  const scoped_refptr<SequencedWorkerPool>& pool() {
+    return pool_owner_->pool();
+  }
+
+  void PostDoStuffToWorkerPool(SequenceCheckerClass* sequence_checker,
+                               const std::string& token_name) {
+    pool()->PostNamedSequencedWorkerTask(
+        token_name,
+        FROM_HERE,
+        base::Bind(&SequenceCheckerClass::DoStuff,
+                   base::Unretained(sequence_checker)));
+  }
+
+  // Destroys the SequencedWorkerPool instance, blocking until it is fully shut
+  // down, and creates a new instance.
+  void ResetPool() {
+    pool_owner_.reset(new SequencedWorkerPoolOwner(kNumWorkerThreads, "test"));
+  }
+
+  void DifferentSequenceTokensDeathTest();
+  void WorkerPoolAndSimpleThreadDeathTest();
+  void TwoDifferentWorkerPoolsDeathTest();
+
+ private:
+  MessageLoop message_loop_;

[akalin, 2013/07/12 01:09:21]

comment as to what needs the message loop (I guess the SWP at least)

[tommycli, 2013/07/12 19:16:25]

Done.

+  scoped_ptr<SequencedWorkerPoolOwner> pool_owner_;
+};
 
 }  // namespace
 
+TEST(SequenceCheckerNoWorkerPoolTest, CallsAllowedOnSameThread) {
+  scoped_ptr<SequenceCheckerClass> sequence_checker_class(
+      new SequenceCheckerClass);
+
+  // Verify that DoStuff doesn't assert.
+  sequence_checker_class->DoStuff();
+
+  // Verify that the destructor doesn't assert.
+  sequence_checker_class.reset();
+}
+
+TEST(SequenceCheckerNoWorkerPoolTest, DestructorAllowedOnDifferentThread) {
+  scoped_ptr<SequenceCheckerClass> sequence_checker_class(
+      new SequenceCheckerClass);
+
+  // Verify that the destructor doesn't assert
+  // when called on a different thread.
+  DeleteThreadCheckerClassThread delete_on_thread(
+      sequence_checker_class.release());
+
+  delete_on_thread.Start();
+  delete_on_thread.Join();
+}
+
+TEST(SequenceCheckerNoWorkerPoolTest, DetachFromSequence) {
+  scoped_ptr<SequenceCheckerClass> sequence_checker_class(
+      new SequenceCheckerClass);
+
+  // Verify that DoStuff doesn't assert when called on a different thread after
+  // a call to DetachFromSequence.
+  sequence_checker_class->DetachFromSequence();
+  CallDoStuffThread call_on_thread(sequence_checker_class.get());
+
+  call_on_thread.Start();
+  call_on_thread.Join();
+}
+
+#if GTEST_HAS_DEATH_TEST || !ENABLE_SEQUENCE_CHECKER
+
+void MethodOnDifferentThreadImpl() {
+  scoped_ptr<SequenceCheckerClass> sequence_checker_class(
+      new SequenceCheckerClass);
+
+  // DoStuff should assert in debug builds only when called on a
+  // different thread.
+  CallDoStuffThread call_on_thread(sequence_checker_class.get());
+
+  call_on_thread.Start();
+  call_on_thread.Join();
+}
+
+#if ENABLE_SEQUENCE_CHECKER
+TEST(SequenceCheckerNoWorkerPoolDeathTest,
+     MethodNotAllowedOnDifferentThreadInDebug) {
+  ASSERT_DEATH({
+    MethodOnDifferentThreadImpl();
+  }, "");
+}
+#else
+TEST(SequenceCheckerNoWorkerPoolTest, MethodAllowedOnDifferentThreadInRelease) {
+  MethodOnDifferentThreadImpl();
+}
+#endif  // ENABLE_SEQUENCE_CHECKER
+
+void DetachThenCallFromDifferentThreadImpl() {
+  scoped_ptr<SequenceCheckerClass> sequence_checker_class(
+      new SequenceCheckerClass);
+
+  // DoStuff doesn't assert when called on a different thread
+  // after a call to DetachFromSequence.
+  sequence_checker_class->DetachFromSequence();
+  CallDoStuffThread call_on_thread(sequence_checker_class.get());
+
+  call_on_thread.Start();
+  call_on_thread.Join();
+
+  // DoStuff should assert in debug builds only after moving to
+  // another thread.
+  sequence_checker_class->DoStuff();
+}
+
+#if ENABLE_SEQUENCE_CHECKER
+TEST(SequenceCheckerNoWorkerPoolDeathTest, DetachFromSequenceInDebug) {
+  ASSERT_DEATH({
+    DetachThenCallFromDifferentThreadImpl();
+  }, "");
+}
+#else
+TEST(SequenceCheckerNoWorkerPoolTest, DetachFromThreadInRelease) {
+  DetachThenCallFromDifferentThreadImpl();
+}
+#endif  // ENABLE_SEQUENCE_CHECKER
+
+#endif  // GTEST_HAS_DEATH_TEST || !ENABLE_SEQUENCE_CHECKER
+
+TEST_F(SequenceCheckerTest, SameSequenceTokenValid) {
+  scoped_ptr<SequenceCheckerClass> sequence_checker_class(
+      new SequenceCheckerClass);
+
+  sequence_checker_class->DetachFromSequence();
+  PostDoStuffToWorkerPool(sequence_checker_class.get(), "A");
+  PostDoStuffToWorkerPool(sequence_checker_class.get(), "A");
+  PostDoStuffToWorkerPool(sequence_checker_class.get(), "A");
+  PostDoStuffToWorkerPool(sequence_checker_class.get(), "A");
+  pool()->FlushForTesting();
+
+  DeleteThreadCheckerClassThread delete_on_thread(
+      sequence_checker_class.release());
+  delete_on_thread.Start();
+  delete_on_thread.Join();
+}
+
+TEST_F(SequenceCheckerTest, DetachSequenceTokenValid) {
+  scoped_ptr<SequenceCheckerClass> sequence_checker_class(
+      new SequenceCheckerClass);
+
+  sequence_checker_class->DetachFromSequence();
+  PostDoStuffToWorkerPool(sequence_checker_class.get(), "A");
+  PostDoStuffToWorkerPool(sequence_checker_class.get(), "A");
+  pool()->FlushForTesting();
+
+  sequence_checker_class->DetachFromSequence();
+  PostDoStuffToWorkerPool(sequence_checker_class.get(), "B");
+  PostDoStuffToWorkerPool(sequence_checker_class.get(), "B");
+  pool()->FlushForTesting();
+
+  DeleteThreadCheckerClassThread delete_on_thread(
+      sequence_checker_class.release());
+  delete_on_thread.Start();
+  delete_on_thread.Join();
+}
+
+#if GTEST_HAS_DEATH_TEST || !ENABLE_SEQUENCE_CHECKER
+
+void SequenceCheckerTest::DifferentSequenceTokensDeathTest() {
+  scoped_ptr<SequenceCheckerClass> sequence_checker_class(
+      new SequenceCheckerClass);
+
+  sequence_checker_class->DetachFromSequence();
+  PostDoStuffToWorkerPool(sequence_checker_class.get(), "A");
+  PostDoStuffToWorkerPool(sequence_checker_class.get(), "A");
+  PostDoStuffToWorkerPool(sequence_checker_class.get(), "B");
+  PostDoStuffToWorkerPool(sequence_checker_class.get(), "B");
+  pool()->FlushForTesting();
+
+  DeleteThreadCheckerClassThread delete_on_thread(
+      sequence_checker_class.release());
+  delete_on_thread.Start();
+  delete_on_thread.Join();
+}
+
+#if ENABLE_SEQUENCE_CHECKER
+TEST_F(SequenceCheckerTest, DifferentSequenceTokensDeathTestInDebug) {
+  ASSERT_DEATH({
+    DifferentSequenceTokensDeathTest();
+  }, "");
+}
+#else
+TEST_F(SequenceCheckerTest,
+     DifferentSequenceTokensDeathTestInRelease) {
+  DifferentSequenceTokensDeathTest();
+}
+#endif  // ENABLE_SEQUENCE_CHECKER
+
+void SequenceCheckerTest::WorkerPoolAndSimpleThreadDeathTest() {
+  scoped_ptr<SequenceCheckerClass> sequence_checker_class(
+      new SequenceCheckerClass);
+
+  sequence_checker_class->DetachFromSequence();
+  PostDoStuffToWorkerPool(sequence_checker_class.get(), "A");
+  PostDoStuffToWorkerPool(sequence_checker_class.get(), "A");
+  pool()->FlushForTesting();
+
+  CallDoStuffThread call_on_thread(sequence_checker_class.get());
+  call_on_thread.Start();
+  call_on_thread.Join();
+}
+
+#if ENABLE_SEQUENCE_CHECKER
+TEST_F(SequenceCheckerTest, WorkerPoolAndSimpleThreadDeathTestInDebug) {
+  ASSERT_DEATH({
+    WorkerPoolAndSimpleThreadDeathTest();
+  }, "");
+}
+#else
+TEST_F(SequenceCheckerTest,
+     WorkerPoolAndSimpleThreadDeathTestInRelease) {
+  WorkerPoolAndSimpleThreadDeathTest();
+}
+#endif  // ENABLE_SEQUENCE_CHECKER
+
+void SequenceCheckerTest::TwoDifferentWorkerPoolsDeathTest() {
+  scoped_ptr<SequenceCheckerClass> sequence_checker_class(
+      new SequenceCheckerClass);
+
+  sequence_checker_class->DetachFromSequence();
+  PostDoStuffToWorkerPool(sequence_checker_class.get(), "A");
+  PostDoStuffToWorkerPool(sequence_checker_class.get(), "A");
+  pool()->FlushForTesting();
+
+  SequencedWorkerPoolOwner second_pool_owner(kNumWorkerThreads, "test2");
+  second_pool_owner.pool()->PostNamedSequencedWorkerTask(
+      "A",
+      FROM_HERE,
+      base::Bind(&SequenceCheckerClass::DoStuff,
+                 base::Unretained(sequence_checker_class.get())));
+  second_pool_owner.pool()->FlushForTesting();
+  second_pool_owner.pool()->Shutdown();
+}
+
+#if ENABLE_SEQUENCE_CHECKER
+TEST_F(SequenceCheckerTest, TwoDifferentWorkerPoolsDeathTestInDebug) {
+  ASSERT_DEATH({
+    TwoDifferentWorkerPoolsDeathTest();
+  }, "");
+}
+#else
+TEST_F(SequenceCheckerTest,
+     TwoDifferentWorkerPoolsDeathTestInRelease) {
+  TwoDifferentWorkerPoolsDeathTest();
+}
+#endif  // ENABLE_SEQUENCE_CHECKER
+
+#endif  // GTEST_HAS_DEATH_TEST || !ENABLE_SEQUENCE_CHECKER
+
+// Just in case we ever get lumped together with other compilation units.
+#undef ENABLE_SEQUENCE_CHECKER
+
 }  // namespace base