Sometimes you want to enforce that some but not all of your methods are called on the same thread as the constructor. ThreadChecker allows you to do this.

BUG=38475
TEST=base_unittests --gtest_filter=NonThreadSafeTest.* and base_unittests --gtest_filter=ThreadChecker.*

Review URL: http://codereview.chromium.org/3148032

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@57140 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 296 insertions, 15 deletions

diff --git a/base/base.gyp b/base/base.gyp
index 2070986..23e8c6b 100644
--- a/base/base.gyp
+++ b/base/base.gyp
@@ -108,6 +108,7 @@
         'message_loop_proxy_impl_unittest.cc',
         'message_loop_unittest.cc',
         'message_pump_glib_unittest.cc',
+        'non_thread_safe_unittest.cc',
         'object_watcher_unittest.cc',
         'observer_list_unittest.cc',
         'path_service_unittest.cc',
@@ -143,6 +144,7 @@
         'sys_info_unittest.cc',
         'sys_string_conversions_mac_unittest.mm',
         'sys_string_conversions_unittest.cc',
+        'thread_checker_unittest.cc',
         'thread_collision_warner_unittest.cc',
         'thread_local_storage_unittest.cc',
         'thread_local_unittest.cc',
diff --git a/base/base.gypi b/base/base.gypi
index e355172..2a6e64a 100644
--- a/base/base.gypi
+++ b/base/base.gypi
@@ -256,6 +256,8 @@
           'template_util.h',
           'thread.cc',
           'thread.h',
+          'thread_checker.cc',
+          'thread_checker.h',
           'thread_collision_warner.cc',
           'thread_collision_warner.h',
           'thread_local.h',
diff --git a/base/non_thread_safe.cc b/base/non_thread_safe.cc
index d8a91c6..6889101 100644
--- a/base/non_thread_safe.cc
+++ b/base/non_thread_safe.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -9,12 +9,8 @@
 
 #include "base/logging.h"
 
-NonThreadSafe::NonThreadSafe()
-    : valid_thread_id_(PlatformThread::CurrentId()) {
-}
-
 bool NonThreadSafe::CalledOnValidThread() const {
-  return valid_thread_id_ == PlatformThread::CurrentId();
+  return thread_checker_.CalledOnValidThread();
 }
 
 NonThreadSafe::~NonThreadSafe() {
diff --git a/base/non_thread_safe.h b/base/non_thread_safe.h
index feb5120..e435884 100644
--- a/base/non_thread_safe.h
+++ b/base/non_thread_safe.h
@@ -1,12 +1,13 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
-#ifndef BASE_NON_THREAD_SAFE_H__
-#define BASE_NON_THREAD_SAFE_H__
+#ifndef BASE_NON_THREAD_SAFE_H_
+#define BASE_NON_THREAD_SAFE_H_
 #pragma once
 
 #include "base/platform_thread.h"
+#include "base/thread_checker.h"
 
 // A helper class used to help verify that methods of a class are
 // called from the same thread.  One can inherit from this class and use
@@ -29,25 +30,21 @@
 #ifndef NDEBUG
 class NonThreadSafe {
  public:
-  NonThreadSafe();
   ~NonThreadSafe();
 
   bool CalledOnValidThread() const;
 
  private:
-  PlatformThreadId valid_thread_id_;
+  ThreadChecker thread_checker_;
 };
 #else
 // Do nothing in release mode.
 class NonThreadSafe {
  public:
-  NonThreadSafe() {}
-  ~NonThreadSafe() {}
-
   bool CalledOnValidThread() const {
     return true;
   }
 };
 #endif  // NDEBUG
 
-#endif  // BASE_NON_THREAD_SAFE_H__
+#endif  // BASE_NON_THREAD_SAFE_H_
diff --git a/base/non_thread_safe_unittest.cc b/base/non_thread_safe_unittest.cc
new file mode 100644
index 0000000..0603987
--- /dev/null
+++ b/base/non_thread_safe_unittest.cc
@@ -0,0 +1,108 @@
+// Copyright (c) 2010 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/logging.h"
+#include "base/non_thread_safe.h"
+#include "base/scoped_ptr.h"
+#include "base/simple_thread.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+#ifndef NDEBUG
+
+// Simple class to exersice the basics of NonThreadSafe.
+// Both the destructor and DoStuff should verify that they were
+// called on the same thread as the constructor.
+class NonThreadSafeClass : public NonThreadSafe {
+ public:
+  NonThreadSafeClass() {}
+
+  // Verifies that it was called on the same thread as the constructor.
+  void DoStuff() {
+    DCHECK(CalledOnValidThread());
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(NonThreadSafeClass);
+};
+
+// Calls NonThreadSafeClass::DoStuff on another thread.
+class CallDoStuffOnThread : public base::SimpleThread {
+ public:
+  CallDoStuffOnThread(NonThreadSafeClass* non_thread_safe_class)
+      : SimpleThread("call_do_stuff_on_thread"),
+        non_thread_safe_class_(non_thread_safe_class) {
+  }
+
+  virtual void Run() {
+    non_thread_safe_class_->DoStuff();
+  }
+
+ private:
+  NonThreadSafeClass* non_thread_safe_class_;
+
+  DISALLOW_COPY_AND_ASSIGN(CallDoStuffOnThread);
+};
+
+// Deletes NonThreadSafeClass on a different thread.
+class DeleteNonThreadSafeClassOnThread : public base::SimpleThread {
+ public:
+  DeleteNonThreadSafeClassOnThread(NonThreadSafeClass* non_thread_safe_class)
+      : SimpleThread("delete_non_thread_safe_class_on_thread"),
+        non_thread_safe_class_(non_thread_safe_class) {
+  }
+
+  virtual void Run() {
+    non_thread_safe_class_.reset();
+  }
+
+ private:
+  scoped_ptr<NonThreadSafeClass> non_thread_safe_class_;
+
+  DISALLOW_COPY_AND_ASSIGN(DeleteNonThreadSafeClassOnThread);
+};
+
+TEST(NonThreadSafeTest, CallsAllowedOnSameThread) {
+  scoped_ptr<NonThreadSafeClass> non_thread_safe_class(
+      new NonThreadSafeClass);
+
+  // Verify that DoStuff doesn't assert.
+  non_thread_safe_class->DoStuff();
+
+  // Verify that the destructor doesn't assert.
+  non_thread_safe_class.reset();
+}
+
+#if GTEST_HAS_DEATH_TEST
+
+TEST(NonThreadSafeDeathTest, MethodNotAllowedOnDifferentThread) {
+  ASSERT_DEBUG_DEATH({
+      scoped_ptr<NonThreadSafeClass> non_thread_safe_class(
+          new NonThreadSafeClass);
+
+      // Verify that DoStuff asserts when called on a different thread.
+      CallDoStuffOnThread call_on_thread(non_thread_safe_class.get());
+
+      call_on_thread.Start();
+      call_on_thread.Join();
+    }, "");
+}
+
+TEST(NonThreadSafeDeathTest, DestructorNotAllowedOnDifferentThread) {
+  ASSERT_DEBUG_DEATH({
+      scoped_ptr<NonThreadSafeClass> non_thread_safe_class(
+          new NonThreadSafeClass);
+
+      // Verify that the destructor asserts when called on a different thread.
+      DeleteNonThreadSafeClassOnThread delete_on_thread(
+          non_thread_safe_class.release());
+
+      delete_on_thread.Start();
+      delete_on_thread.Join();
+    }, "");
+}
+
+#endif  // GTEST_HAS_DEATH_TEST
+
+#endif  // NDEBUG
diff --git a/base/thread_checker.cc b/base/thread_checker.cc
new file mode 100644
index 0000000..124b76c
--- /dev/null
+++ b/base/thread_checker.cc
@@ -0,0 +1,16 @@
+// Copyright (c) 2010 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/thread_checker.h"
+
+// This code is only done in debug builds.
+#ifndef NDEBUG
+ThreadChecker::ThreadChecker() : valid_thread_id_(PlatformThread::CurrentId()) {
+}
+
+bool ThreadChecker::CalledOnValidThread() const {
+  return valid_thread_id_ == PlatformThread::CurrentId();
+}
+
+#endif  // NDEBUG
diff --git a/base/thread_checker.h b/base/thread_checker.h
new file mode 100644
index 0000000..4e1d792
--- /dev/null
+++ b/base/thread_checker.h
@@ -0,0 +1,53 @@
+// Copyright (c) 2010 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.
+
+#ifndef BASE_THREAD_CHECKER_H_
+#define BASE_THREAD_CHECKER_H_
+#pragma once
+
+#include "base/platform_thread.h"
+
+// Before using this class, please consider using NonThreadSafe as it
+// makes it much easier to determine the nature of your class.
+//
+// A helper class used to help verify that some methods of a class are
+// called from the same thread.  One can inherit from this class and use
+// CalledOnValidThread() to verify.
+//
+// Inheriting from class indicates that one must be careful when using the class
+// with multiple threads. However, it is up to the class document to indicate
+// how it can be used with threads.
+//
+// Example:
+// class MyClass : public ThreadChecker {
+//  public:
+//   void Foo() {
+//     DCHECK(CalledOnValidThread());
+//     ... (do stuff) ...
+//   }
+// }
+//
+// In Release mode, CalledOnValidThread will always return true.
+//
+#ifndef NDEBUG
+class ThreadChecker {
+ public:
+  ThreadChecker();
+
+  bool CalledOnValidThread() const;
+
+ private:
+  const PlatformThreadId valid_thread_id_;
+};
+#else
+// Do nothing in release mode.
+class ThreadChecker {
+ public:
+  bool CalledOnValidThread() const {
+    return true;
+  }
+};
+#endif  // NDEBUG
+
+#endif  // BASE_THREAD_CHECKER_H_
diff --git a/base/thread_checker_unittest.cc b/base/thread_checker_unittest.cc
new file mode 100644
index 0000000..fe49e21
--- /dev/null
+++ b/base/thread_checker_unittest.cc
@@ -0,0 +1,107 @@
+// Copyright (c) 2010 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/logging.h"
+#include "base/thread_checker.h"
+#include "base/scoped_ptr.h"
+#include "base/simple_thread.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+#ifndef NDEBUG
+
+// Simple class to exersice the basics of ThreadChecker.
+// Both the destructor and DoStuff should verify that they were
+// called on the same thread as the constructor.
+class ThreadCheckerClass : public ThreadChecker {
+ public:
+  ThreadCheckerClass() {}
+
+  // Verifies that it was called on the same thread as the constructor.
+  void DoStuff() {
+    DCHECK(CalledOnValidThread());
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ThreadCheckerClass);
+};
+
+// Calls ThreadCheckerClass::DoStuff on another thread.
+class CallDoStuffOnThread : public base::SimpleThread {
+ public:
+  CallDoStuffOnThread(ThreadCheckerClass* thread_checker_class)
+      : SimpleThread("call_do_stuff_on_thread"),
+        thread_checker_class_(thread_checker_class) {
+  }
+
+  virtual void Run() {
+    thread_checker_class_->DoStuff();
+  }
+
+ private:
+  ThreadCheckerClass* thread_checker_class_;
+
+  DISALLOW_COPY_AND_ASSIGN(CallDoStuffOnThread);
+};
+
+// Deletes ThreadCheckerClass on a different thread.
+class DeleteThreadCheckerClassOnThread : public base::SimpleThread {
+ public:
+  DeleteThreadCheckerClassOnThread(ThreadCheckerClass* thread_checker_class)
+      : SimpleThread("delete_thread_checker_class_on_thread"),
+        thread_checker_class_(thread_checker_class) {
+  }
+
+  virtual void Run() {
+    thread_checker_class_.reset();
+  }
+
+ private:
+  scoped_ptr<ThreadCheckerClass> thread_checker_class_;
+
+  DISALLOW_COPY_AND_ASSIGN(DeleteThreadCheckerClassOnThread);
+};
+
+TEST(ThreadCheckerTest, CallsAllowedOnSameThread) {
+  scoped_ptr<ThreadCheckerClass> thread_checker_class(
+      new ThreadCheckerClass);
+
+  // Verify that DoStuff doesn't assert.
+  thread_checker_class->DoStuff();
+
+  // Verify that the destructor doesn't assert.
+  thread_checker_class.reset();
+}
+
+TEST(ThreadCheckerTest, DestructorAllowedOnDifferentThread) {
+  scoped_ptr<ThreadCheckerClass> thread_checker_class(
+      new ThreadCheckerClass);
+
+  // Verify that the destructor doesn't assert
+  // when called on a different thread.
+  DeleteThreadCheckerClassOnThread delete_on_thread(
+      thread_checker_class.release());
+
+  delete_on_thread.Start();
+  delete_on_thread.Join();
+}
+
+#if GTEST_HAS_DEATH_TEST
+
+TEST(ThreadCheckerDeathTest, MethodNotAllowedOnDifferentThread) {
+  ASSERT_DEBUG_DEATH({
+      scoped_ptr<ThreadCheckerClass> thread_checker_class(
+          new ThreadCheckerClass);
+
+      // Verify that DoStuff asserts when called on a different thread.
+      CallDoStuffOnThread call_on_thread(thread_checker_class.get());
+
+      call_on_thread.Start();
+      call_on_thread.Join();
+    }, "");
+}
+
+#endif  // GTEST_HAS_DEATH_TEST
+
+#endif  // NDEBUG