Adds a helper class used to mark/define critical section in a class and then install controls to check that those critical sections are not violated.

This CL is due the thread posted on chromium-dev:
http://groups.google.com/group/chromium-dev/browse_frm/thread/30af0b63b6adb245.

From Gaetano Mendola <mendola bigfoot com>
Review URL: http://codereview.chromium.org/8621

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

12 files changed, 702 insertions, 1 deletions

diff --git a/base/base.xcodeproj/project.pbxproj b/base/base.xcodeproj/project.pbxproj
index 28013bd..152b78c 100644
--- a/base/base.xcodeproj/project.pbxproj
+++ b/base/base.xcodeproj/project.pbxproj
@@ -35,6 +35,9 @@
 /* End PBXAggregateTarget section */
 
 /* Begin PBXBuildFile section */
+		141593B80EA63EBE00E32418 /* thread_collision_warner_unittest.cc in Sources */ = {isa = PBXBuildFile; fileRef = 146C6A6E0EA63D970029E7B6 /* thread_collision_warner_unittest.cc */; };
+		146C6A6B0EA63D4F0029E7B6 /* thread_collision_warner.cc in Sources */ = {isa = PBXBuildFile; fileRef = 146C6A620EA63CAE0029E7B6 /* thread_collision_warner.cc */; };
+		146C6A6C0EA63D4F0029E7B6 /* thread_collision_warner.h in Sources */ = {isa = PBXBuildFile; fileRef = 146C6A610EA63C9F0029E7B6 /* thread_collision_warner.h */; };
 		4D11B59A0E91730200EF7617 /* rand_util.cc in Sources */ = {isa = PBXBuildFile; fileRef = 4D11B5940E9172F800EF7617 /* rand_util.cc */; };
 		4D11B59B0E91730200EF7617 /* rand_util_posix.cc in Sources */ = {isa = PBXBuildFile; fileRef = 4D11B5960E9172F800EF7617 /* rand_util_posix.cc */; };
 		4D11B59C0E91730500EF7617 /* rand_util_unittest.cc in Sources */ = {isa = PBXBuildFile; fileRef = 4D11B5970E9172F800EF7617 /* rand_util_unittest.cc */; };
@@ -373,6 +376,9 @@
 /* End PBXContainerItemProxy section */
 
 /* Begin PBXFileReference section */
+		146C6A610EA63C9F0029E7B6 /* thread_collision_warner.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = thread_collision_warner.h; sourceTree = "<group>"; };
+		146C6A620EA63CAE0029E7B6 /* thread_collision_warner.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = thread_collision_warner.cc; sourceTree = "<group>"; };
+		146C6A6E0EA63D970029E7B6 /* thread_collision_warner_unittest.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = thread_collision_warner_unittest.cc; sourceTree = "<group>"; };
 		4D11B5940E9172F800EF7617 /* rand_util.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = rand_util.cc; sourceTree = "<group>"; };
 		4D11B5950E9172F800EF7617 /* rand_util.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = rand_util.h; sourceTree = "<group>"; };
 		4D11B5960E9172F800EF7617 /* rand_util_posix.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = rand_util_posix.cc; sourceTree = "<group>"; };
@@ -1011,6 +1017,9 @@
 				84B581950EDC6ECC00A6B5B3 /* test_file_util_mac.cc */,
 				93E703230E5D64F00046259B /* thread.cc */,
 				825403800D92D2CF0006B936 /* thread.h */,
+				146C6A6E0EA63D970029E7B6 /* thread_collision_warner_unittest.cc */,
+				146C6A620EA63CAE0029E7B6 /* thread_collision_warner.cc */,
+				146C6A610EA63C9F0029E7B6 /* thread_collision_warner.h */,
 				7BAE38A80E6EFD9900C3F750 /* thread_local.h */,
 				7BAE38A90E6EFD9900C3F750 /* thread_local_posix.cc */,
 				825403820D92D2CF0006B936 /* thread_local_storage.h */,
@@ -1399,6 +1408,8 @@
 				E4CE9D7A0E8C1FD400D5378C /* system_monitor.cc in Sources */,
 				84B581DC0EDC715A00A6B5B3 /* test_file_util_mac.cc in Sources */,
 				93E703240E5D64F00046259B /* thread.cc in Sources */,
+				146C6A6B0EA63D4F0029E7B6 /* thread_collision_warner.cc in Sources */,
+				146C6A6C0EA63D4F0029E7B6 /* thread_collision_warner.h in Sources */,
 				7BAE38AC0E6EFDBA00C3F750 /* thread_local_posix.cc in Sources */,
 				829E36730DC0FBAD00819EBF /* thread_local_storage_posix.cc in Sources */,
 				824654910DC25A8C007C2BAA /* time.cc in Sources */,
@@ -1471,6 +1482,7 @@
 				7B78D3A10E54FE0100609465 /* string_util_unittest.cc in Sources */,
 				7B6AF6350E80211E00F9F9CF /* sys_info_unittest.cc in Sources */,
 				B5D544AB0EAFB7E000272A1C /* sys_string_conversions_unittest.cc in Sources */,
+				141593B80EA63EBE00E32418 /* thread_collision_warner_unittest.cc in Sources */,
 				BA0F69870E79D7980079A8A1 /* thread_local_storage_unittest.cc in Sources */,
 				7BAE38AF0E6EFDC300C3F750 /* thread_local_unittest.cc in Sources */,
 				93E7031B0E5D64390046259B /* thread_unittest.cc in Sources */,
diff --git a/base/base_lib.scons b/base/base_lib.scons
index e2bb18d..6ba8d6e 100644
--- a/base/base_lib.scons
+++ b/base/base_lib.scons
@@ -69,6 +69,7 @@ input_files = [
     'third_party/nspr/prtime.cc',
     'third_party/nss/sha512.cc',
     'thread.cc',
+    'thread_collision_warner.cc',
     'time.cc',
     'time_format.cc',
     'timer.cc',
diff --git a/base/base_unittests.scons b/base/base_unittests.scons
index 433fdcce..7aa039a 100644
--- a/base/base_unittests.scons
+++ b/base/base_unittests.scons
@@ -80,6 +80,7 @@ input_files = [
     'string_tokenizer_unittest.cc',
     'string_util_unittest.cc',
     'sys_info_unittest.cc',
+    'thread_collision_warner_unittest.cc',
     'thread_local_storage_unittest.cc',
     'thread_local_unittest.cc',
     'thread_unittest.cc',
diff --git a/base/build/base.vcproj b/base/build/base.vcproj
index 6202baa..10c7dad 100644
--- a/base/build/base.vcproj
+++ b/base/build/base.vcproj
@@ -850,6 +850,14 @@
 			>
 		</File>
 		<File
+			RelativePath="..\thread_collision_warner.cc"
+			>
+		</File>
+		<File
+			RelativePath="..\thread_collision_warner.h"
+			>
+		</File>
+		<File
 			RelativePath="..\thread_local.h"
 			>
 		</File>
diff --git a/base/build/base_unittests.vcproj b/base/build/base_unittests.vcproj
index e822992..8263e074 100644
--- a/base/build/base_unittests.vcproj
+++ b/base/build/base_unittests.vcproj
@@ -336,6 +336,10 @@
 				>
 			</File>
 			<File
+				RelativePath="..\thread_collision_warner_unittest.cc"
+				>
+			</File>
+			<File
 				RelativePath="..\thread_local_storage_unittest.cc"
 				>
 			</File>
diff --git a/base/condition_variable_unittest.cc b/base/condition_variable_unittest.cc
index c996f2c..8ca4bc1 100644
--- a/base/condition_variable_unittest.cc
+++ b/base/condition_variable_unittest.cc
@@ -13,6 +13,7 @@
 #include "base/platform_thread.h"
 #include "base/scoped_ptr.h"
 #include "base/spin_wait.h"
+#include "base/thread_collision_warner.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/platform_test.h"
 
@@ -79,6 +80,9 @@ class WorkQueue : public PlatformThread::Delegate {
   int shutdown_task_count() const;
 
   void thread_shutting_down();
+
+  //----------------------------------------------------------------------------
+  // Worker threads can call them but not needed to acquire a lock
   Lock* lock();
 
   ConditionVariable* work_is_available();
@@ -120,6 +124,8 @@ class WorkQueue : public PlatformThread::Delegate {
   TimeDelta worker_delay_;  // Time each task takes to complete.
   bool allow_help_requests_;  // Workers can signal more workers.
   bool shutdown_;  // Set when threads need to terminate.
+
+  DFAKE_MUTEX(locked_methods_);
 };
 
 //------------------------------------------------------------------------------
@@ -477,15 +483,18 @@ WorkQueue::~WorkQueue() {
 }
 
 int WorkQueue::GetThreadId() {
+  DFAKE_SCOPED_RECURSIVE_LOCK(locked_methods_);
   DCHECK(!EveryIdWasAllocated());
   return thread_started_counter_++;  // Give out Unique IDs.
 }
 
 bool WorkQueue::EveryIdWasAllocated() const {
+  DFAKE_SCOPED_RECURSIVE_LOCK(locked_methods_);
   return thread_count_ == thread_started_counter_;
 }
 
 TimeDelta WorkQueue::GetAnAssignment(int thread_id) {
+  DFAKE_SCOPED_RECURSIVE_LOCK(locked_methods_);
   DCHECK_LT(0, task_count_);
   assignment_history_[thread_id]++;
   if (0 == --task_count_) {
@@ -495,26 +504,32 @@ TimeDelta WorkQueue::GetAnAssignment(int thread_id) {
 }
 
 void WorkQueue::WorkIsCompleted(int thread_id) {
+  DFAKE_SCOPED_RECURSIVE_LOCK(locked_methods_);
   completion_history_[thread_id]++;
 }
 
 int WorkQueue::task_count() const {
+  DFAKE_SCOPED_RECURSIVE_LOCK(locked_methods_);
   return task_count_;
 }
 
 bool WorkQueue::allow_help_requests() const {
+  DFAKE_SCOPED_RECURSIVE_LOCK(locked_methods_);
   return allow_help_requests_;
 }
 
 bool WorkQueue::shutdown() const {
+  DFAKE_SCOPED_RECURSIVE_LOCK(locked_methods_);
   return shutdown_;
 }
 
 int WorkQueue::shutdown_task_count() const {
+  DFAKE_SCOPED_RECURSIVE_LOCK(locked_methods_);
   return shutdown_task_count_;
 }
 
 void WorkQueue::thread_shutting_down() {
+  DFAKE_SCOPED_RECURSIVE_LOCK(locked_methods_);
   shutdown_task_count_++;
 }
 
diff --git a/base/revocable_store.h b/base/revocable_store.h
index 65fb05e..2ee9705 100644
--- a/base/revocable_store.h
+++ b/base/revocable_store.h
@@ -17,7 +17,7 @@ class RevocableStore {
   // store wishes to revoke its items, it sets |store_| to null.  Items are
   // permitted to release their reference to the |StoreRef| when they no longer
   // require the store.
- class StoreRef : public base::RefCounted<StoreRef> {
+  class StoreRef : public base::RefCountedThreadSafe<StoreRef> {
    public:
     StoreRef(RevocableStore* store) : store_(store) { }
 
diff --git a/base/thread_collision_warner.cc b/base/thread_collision_warner.cc
new file mode 100644
index 0000000..1c51a3e
--- /dev/null
+++ b/base/thread_collision_warner.cc
@@ -0,0 +1,53 @@
+// Copyright (c) 2006-2008 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_collision_warner.h"
+
+#include "base/atomicops.h"
+#include "base/logging.h"
+
+namespace base {
+
+void DCheckAsserter::warn() {
+  NOTREACHED() << "Thread Collision";
+}
+
+void ThreadCollisionWarner::EnterSelf() {
+  // If the active thread is 0 then I'll write the current thread ID
+  // if two or more threads arrive here only one will succeed to
+  // write on valid_thread_id_ the current thread ID.
+  const int current_thread_id = PlatformThread::CurrentId();
+
+  int previous_value = subtle::NoBarrier_CompareAndSwap(&valid_thread_id_,
+                                                        0,
+                                                        current_thread_id);
+  if (previous_value != 0 && previous_value != current_thread_id) {
+    // gotcha! a thread is trying to use the same class and that is
+    // not current thread.
+    asserter_->warn();
+  }
+
+  subtle::NoBarrier_AtomicIncrement(&counter_, 1);
+}
+
+void ThreadCollisionWarner::Enter() {
+  const int current_thread_id = PlatformThread::CurrentId();
+
+  if (subtle::NoBarrier_CompareAndSwap(&valid_thread_id_,
+                                       0,
+                                       current_thread_id) != 0) {
+    // gotcha! another thread is trying to use the same class.
+    asserter_->warn();
+  }
+
+  subtle::NoBarrier_AtomicIncrement(&counter_, 1);
+}
+
+void ThreadCollisionWarner::Leave() {
+  if (subtle::Barrier_AtomicIncrement(&counter_, -1) == 0) {
+    subtle::NoBarrier_Store(&valid_thread_id_, 0);
+  }
+}
+
+}  // namespace base
diff --git a/base/thread_collision_warner.h b/base/thread_collision_warner.h
new file mode 100644
index 0000000..fa8b7ee
--- /dev/null
+++ b/base/thread_collision_warner.h
@@ -0,0 +1,243 @@
+// Copyright (c) 2008 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 THREAD_COLLISION_WARNER_H_
+#define THREAD_COLLISION_WARNER_H_
+
+#include <memory>
+
+#include "base/atomicops.h"
+#include "base/platform_thread.h"
+
+// A helper class alongside macros to be used to verify assumptions about thread
+// safety of a class.
+//
+// Example: Queue implementation non thread-safe but still usable if clients
+//          are synchronized somehow.
+//
+//          In this case the macro DFAKE_SCOPED_LOCK has to be
+//          used, it checks that if a thread is inside the push/pop then
+//          noone else is still inside the pop/push
+//
+// class NonThreadSafeQueue {
+//  public:
+//   ...
+//   void push(int) { DFAKE_SCOPED_LOCK(push_pop_); ... }
+//   int pop() { DFAKE_SCOPED_LOCK(push_pop_); ... }
+//   ...
+//  private:
+//   DFAKE_MUTEX(push_pop_);
+// };
+//
+//
+// Example: Queue implementation non thread-safe but still usable if clients
+//          are synchronized somehow, it calls a method to "protect" from
+//          a "protected" method
+//
+//          In this case the macro DFAKE_SCOPED_RECURSIVE_LOCK 
+//          has to be used, it checks that if a thread is inside the push/pop
+//          then noone else is still inside the pop/push
+//
+// class NonThreadSafeQueue {
+//  public:
+//   void push(int) {
+//     DFAKE_SCOPED_LOCK(push_pop_);
+//     ...
+//   }
+//   int pop() {
+//     DFAKE_SCOPED_RECURSIVE_LOCK(push_pop_);
+//     bar();
+//     ...
+//   }
+//   void bar() { DFAKE_SCOPED_RECURSIVE_LOCK(push_pop_); ... }
+//   ...
+//  private:
+//   DFAKE_MUTEX(push_pop_);
+// };
+//
+//
+// Example: Queue implementation not usable even if clients are synchronized,
+//          so only one thread in the class life cycle can use the two members
+//          push/pop.
+//
+//          In this case the macro DFAKE_SCOPED_LOCK_THREAD_LOCKED pins the
+//          specified
+//          critical section the first time a thread enters push or pop, from
+//          that time on only that thread is allowed to execute push or pop.
+//
+// class NonThreadSafeQueue {
+//  public:
+//   ...
+//   void push(int) { DFAKE_SCOPED_LOCK_THREAD_LOCKED(push_pop_); ... }
+//   int pop() { DFAKE_SCOPED_LOCK_THREAD_LOCKED(push_pop_); ... }
+//   ...
+//  private:
+//   DFAKE_MUTEX(push_pop_);
+// };
+//
+//
+// Example: Class that has to be contructed/destroyed on same thread, it has
+//          a "shareable" method (with external syncronization) and a not
+//          shareable method (even with external synchronization).
+//
+//          In this case 3 Critical sections have to be defined
+//
+// class ExoticClass {
+//  public:
+//   ExoticClass() { DFAKE_SCOPED_LOCK_THREAD_LOCKED(ctor_dtor_); ... }
+//   ~ExoticClass() { DFAKE_SCOPED_LOCK_THREAD_LOCKED(ctor_dtor_); ... }
+//
+//   void Shareable() { DFAKE_SCOPED_LOCK(shareable_section_); ... }
+//   void NotShareable() { DFAKE_SCOPED_LOCK_THREAD_LOCKED(ctor_dtor_); ... }
+//   ...
+//  private:
+//   DFAKE_MUTEX(ctor_dtor_);
+//   DFAKE_MUTEX(shareable_section_);
+// };
+
+
+#if !defined(NDEBUG) || defined(UNIT_TEST)
+
+// Defines a class member that acts like a mutex. It is used only as a
+// verification tool.
+#define DFAKE_MUTEX(obj) \
+     mutable base::ThreadCollisionWarner obj
+// Asserts the call is never called simultaneously in two threads. Used at
+// member function scope.
+#define DFAKE_SCOPED_LOCK(obj) \
+     base::ThreadCollisionWarner::ScopedCheck s_check_##obj(&obj)
+// Asserts the call is never called simultaneously in two threads. Used at
+// member function scope. Same as DFAKE_SCOPED_LOCK but allows recursive locks.
+#define DFAKE_SCOPED_RECURSIVE_LOCK(obj) \
+     base::ThreadCollisionWarner::ScopedRecursiveCheck sr_check_##obj(&obj)
+// Asserts the code is always executed in the same thread.
+#define DFAKE_SCOPED_LOCK_THREAD_LOCKED(obj) \
+     base::ThreadCollisionWarner::Check check_##obj(&obj)
+
+#else
+
+#define DFAKE_MUTEX(obj)
+#define DFAKE_SCOPED_LOCK(obj) ((void)0)
+#define DFAKE_SCOPED_RECURSIVE_LOCK(obj) ((void)0)
+#define DFAKE_SCOPED_LOCK_THREAD_LOCKED(obj) ((void)0)
+
+#endif
+
+namespace base {
+
+// The class ThreadCollisionWarner uses an Asserter to notify the collision
+// AsserterBase is the interfaces and DCheckAsserter is the default asserter
+// used. During the unit tests is used another class that doesn't "DCHECK"
+// in case of collision (check thread_collision_warner_unittests.cc)
+struct AsserterBase {
+  virtual ~AsserterBase() {}
+  virtual void warn() = 0;
+};
+
+struct DCheckAsserter : public AsserterBase {
+  virtual ~DCheckAsserter() {}
+  virtual void warn();
+};
+
+class ThreadCollisionWarner {
+ public:
+  // The parameter asserter is there only for test purpose
+  ThreadCollisionWarner(AsserterBase* asserter = new DCheckAsserter())
+      : valid_thread_id_(0),
+        counter_(0),
+        asserter_(asserter) {}
+
+  ~ThreadCollisionWarner() {
+    delete asserter_;
+  }
+
+  // This class is meant to be used through the macro
+  // DFAKE_SCOPED_LOCK_THREAD_LOCKED
+  // it doesn't leave the critical section, as opposed to ScopedCheck,
+  // because the critical section being pinned is allowed to be used only
+  // from one thread
+  class Check {
+   public:
+    explicit Check(ThreadCollisionWarner* warner)
+        : warner_(warner) {
+      warner_->EnterSelf();
+    }
+
+    ~Check() {}
+
+   private:
+    ThreadCollisionWarner* warner_;
+
+    DISALLOW_COPY_AND_ASSIGN(Check);
+  };
+
+  // This class is meant to be used through the macro
+  // DFAKE_SCOPED_LOCK
+  class ScopedCheck {
+   public:
+    explicit ScopedCheck(ThreadCollisionWarner* warner)
+        : warner_(warner) {
+      warner_->Enter();
+    }
+
+    ~ScopedCheck() {
+      warner_->Leave();
+    }
+
+   private:
+    ThreadCollisionWarner* warner_;
+
+    DISALLOW_COPY_AND_ASSIGN(ScopedCheck);
+  };
+
+  // This class is meant to be used through the macro
+  // DFAKE_SCOPED_RECURSIVE_LOCK
+  class ScopedRecursiveCheck {
+   public:
+    explicit ScopedRecursiveCheck(ThreadCollisionWarner* warner)
+        : warner_(warner) {
+      warner_->EnterSelf();
+    }
+
+    ~ScopedRecursiveCheck() {
+      warner_->Leave();
+    }
+
+   private:
+    ThreadCollisionWarner* warner_;
+
+    DISALLOW_COPY_AND_ASSIGN(ScopedRecursiveCheck);
+  };
+
+ private:
+  // This method stores the current thread identifier and does a DCHECK
+  // if a another thread has already done it, it is safe if same thread
+  // calls this multiple time (recursion allowed).
+  void EnterSelf();
+
+  // Same as EnterSelf but recursion is not allowed.
+  void Enter();
+
+  // Removes the thread_id stored in order to allow other threads to
+  // call EnterSelf or Enter.
+  void Leave();
+
+  // This stores the thread id that is inside the critical section, if the 
+  // value is 0 then no thread is inside.
+  volatile int valid_thread_id_;
+
+  // Counter to trace how many time a critical section was "pinned"
+  // (when allowed) in order to unpin it when counter_ reaches 0.
+  volatile subtle::Atomic32 counter_;
+
+  // Here only for class unit tests purpose, during the test I need to not
+  // DCHECK but notify the collision with something else.
+  AsserterBase* asserter_;
+
+  DISALLOW_COPY_AND_ASSIGN(ThreadCollisionWarner);
+};
+
+}  // namespace base
+
+#endif  // THREAD_COLLISION_WARNER_H_
diff --git a/base/thread_collision_warner_unittest.cc b/base/thread_collision_warner_unittest.cc
new file mode 100644
index 0000000..7994d13
--- /dev/null
+++ b/base/thread_collision_warner_unittest.cc
@@ -0,0 +1,360 @@
+// Copyright (c) 2008 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/compiler_specific.h"
+#include "base/lock.h"
+#include "base/platform_thread.h"
+#include "base/simple_thread.h"
+#include "base/thread_collision_warner.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+// '' : local class member function does not have a body
+MSVC_PUSH_DISABLE_WARNING(4822)
+
+namespace {
+
+// This is the asserter used with ThreadCollisionWarner instead of the default
+// DCheckAsserter. The method fail_state is used to know if a collision took
+// place.
+class AssertReporter : public base::AsserterBase {
+ public:
+  AssertReporter()
+      : failed_(false) {}
+
+  virtual void warn() {
+    failed_ = true;
+  }
+
+  virtual ~AssertReporter() {}
+
+  bool fail_state() const { return failed_; }
+  void reset() { failed_ = false; }
+
+ private:
+  bool failed_;
+};
+
+}  // namespace
+
+TEST(ThreadCollisionTest, BookCriticalSection) {
+  AssertReporter* local_reporter = new AssertReporter();
+
+  base::ThreadCollisionWarner warner(local_reporter);
+  EXPECT_FALSE(local_reporter->fail_state());
+
+  {  // Pin section.
+    DFAKE_SCOPED_LOCK_THREAD_LOCKED(warner);
+    EXPECT_FALSE(local_reporter->fail_state());
+    {  // Pin section.
+      DFAKE_SCOPED_LOCK_THREAD_LOCKED(warner);
+      EXPECT_FALSE(local_reporter->fail_state());
+    }
+  }
+}
+
+TEST(ThreadCollisionTest, ScopedRecursiveBookCriticalSection) {
+  AssertReporter* local_reporter = new AssertReporter();
+
+  base::ThreadCollisionWarner warner(local_reporter);
+  EXPECT_FALSE(local_reporter->fail_state());
+
+  {  // Pin section.
+    DFAKE_SCOPED_RECURSIVE_LOCK(warner);
+    EXPECT_FALSE(local_reporter->fail_state());
+    {  // Pin section again (allowed by DFAKE_SCOPED_RECURSIVE_LOCK)
+      DFAKE_SCOPED_RECURSIVE_LOCK(warner);
+      EXPECT_FALSE(local_reporter->fail_state());
+    }  // Unpin section.
+  }  // Unpin section.
+
+  // Check that section is not pinned
+  {  // Pin section.
+    DFAKE_SCOPED_LOCK(warner);
+    EXPECT_FALSE(local_reporter->fail_state());
+  }  // Unpin section.
+}
+
+TEST(ThreadCollisionTest, ScopedBookCriticalSection) {
+  AssertReporter* local_reporter = new AssertReporter();
+
+  base::ThreadCollisionWarner warner(local_reporter);
+  EXPECT_FALSE(local_reporter->fail_state());
+
+  {  // Pin section.
+    DFAKE_SCOPED_LOCK(warner);
+    EXPECT_FALSE(local_reporter->fail_state());
+  }  // Unpin section.
+
+  {  // Pin section.
+    DFAKE_SCOPED_LOCK(warner);
+    EXPECT_FALSE(local_reporter->fail_state());
+    {  // Pin section again (not allowed by DFAKE_SCOPED_LOCK)
+      DFAKE_SCOPED_LOCK(warner);
+      EXPECT_TRUE(local_reporter->fail_state());
+      // Reset the status of warner for further tests.
+      local_reporter->reset();
+    }  // Unpin section.
+  }  // Unpin section.
+
+  {  // Pin section.
+    DFAKE_SCOPED_LOCK(warner);
+    EXPECT_FALSE(local_reporter->fail_state());
+  }  // Unpin section.
+}
+
+TEST(ThreadCollisionTest, MTBookCriticalSectionTest) {
+  class NonThreadSafeQueue {
+   public:
+    explicit NonThreadSafeQueue(base::AsserterBase* asserter)
+        : push_pop_(asserter) { }
+
+    void push(int value) {
+      DFAKE_SCOPED_LOCK_THREAD_LOCKED(push_pop_);
+    }
+
+    int pop() {
+      DFAKE_SCOPED_LOCK_THREAD_LOCKED(push_pop_);
+      return 0;
+    }
+
+   private:
+    DFAKE_MUTEX(push_pop_);
+
+    DISALLOW_COPY_AND_ASSIGN(NonThreadSafeQueue);
+  };
+
+  class QueueUser : public base::DelegateSimpleThread::Delegate {
+   public:
+    explicit QueueUser(NonThreadSafeQueue& queue)
+        : queue_(queue) {}
+
+    virtual void Run() {
+      queue_.push(0);
+      queue_.pop();
+    }
+
+   private:
+    NonThreadSafeQueue& queue_;
+  };
+
+  AssertReporter* local_reporter = new AssertReporter();
+
+  NonThreadSafeQueue queue(local_reporter);
+
+  QueueUser queue_user_a(queue);
+  QueueUser queue_user_b(queue);
+
+  base::DelegateSimpleThread thread_a(&queue_user_a, "queue_user_thread_a");
+  base::DelegateSimpleThread thread_b(&queue_user_b, "queue_user_thread_b");
+
+  thread_a.Start();
+  thread_b.Start();
+
+  thread_a.Join();
+  thread_b.Join();
+
+  EXPECT_TRUE(local_reporter->fail_state());
+}
+
+TEST(ThreadCollisionTest, MTScopedBookCriticalSectionTest) {
+  // Queue with a 5 seconds push execution time, hopefuly the two used threads
+  // in the test will enter the push at same time.
+  class NonThreadSafeQueue {
+   public:
+    explicit NonThreadSafeQueue(base::AsserterBase* asserter)
+        : push_pop_(asserter) { }
+
+    void push(int value) {
+      DFAKE_SCOPED_LOCK(push_pop_);
+      PlatformThread::Sleep(5000);
+    }
+
+    int pop() {
+      DFAKE_SCOPED_LOCK(push_pop_);
+      return 0;
+    }
+
+   private:
+    DFAKE_MUTEX(push_pop_);
+
+    DISALLOW_COPY_AND_ASSIGN(NonThreadSafeQueue);
+  };
+
+  class QueueUser : public base::DelegateSimpleThread::Delegate {
+   public:
+    explicit QueueUser(NonThreadSafeQueue& queue)
+        : queue_(queue) {}
+
+    virtual void Run() {
+      queue_.push(0);
+      queue_.pop();
+    }
+
+   private:
+    NonThreadSafeQueue& queue_;
+  };
+
+  AssertReporter* local_reporter = new AssertReporter();
+
+  NonThreadSafeQueue queue(local_reporter);
+
+  QueueUser queue_user_a(queue);
+  QueueUser queue_user_b(queue);
+
+  base::DelegateSimpleThread thread_a(&queue_user_a, "queue_user_thread_a");
+  base::DelegateSimpleThread thread_b(&queue_user_b, "queue_user_thread_b");
+
+  thread_a.Start();
+  thread_b.Start();
+
+  thread_a.Join();
+  thread_b.Join();
+
+  EXPECT_TRUE(local_reporter->fail_state());
+}
+
+TEST(ThreadCollisionTest, MTSynchedScopedBookCriticalSectionTest) {
+  // Queue with a 5 seconds push execution time, hopefuly the two used threads
+  // in the test will enter the push at same time.
+  class NonThreadSafeQueue {
+   public:
+    explicit NonThreadSafeQueue(base::AsserterBase* asserter)
+        : push_pop_(asserter) { }
+
+    void push(int value) {
+      DFAKE_SCOPED_LOCK(push_pop_);
+      PlatformThread::Sleep(5000);
+    }
+
+    int pop() {
+      DFAKE_SCOPED_LOCK(push_pop_);
+      return 0;
+    }
+
+   private:
+    DFAKE_MUTEX(push_pop_);
+
+    DISALLOW_COPY_AND_ASSIGN(NonThreadSafeQueue);
+  };
+
+  // This time the QueueUser class protects the non thread safe queue with
+  // a lock.
+  class QueueUser : public base::DelegateSimpleThread::Delegate {
+   public:
+    QueueUser(NonThreadSafeQueue& queue, Lock& lock)
+        : queue_(queue),
+          lock_(lock) {}
+
+    virtual void Run() {
+      {
+        AutoLock auto_lock(lock_);
+        queue_.push(0);
+      }
+      {
+        AutoLock auto_lock(lock_);
+        queue_.pop();
+      }
+    }
+   private:
+    NonThreadSafeQueue& queue_;
+    Lock& lock_;
+  };
+
+  AssertReporter* local_reporter = new AssertReporter();
+
+  NonThreadSafeQueue queue(local_reporter);
+
+  Lock lock;
+
+  QueueUser queue_user_a(queue, lock);
+  QueueUser queue_user_b(queue, lock);
+
+  base::DelegateSimpleThread thread_a(&queue_user_a, "queue_user_thread_a");
+  base::DelegateSimpleThread thread_b(&queue_user_b, "queue_user_thread_b");
+
+  thread_a.Start();
+  thread_b.Start();
+
+  thread_a.Join();
+  thread_b.Join();
+
+  EXPECT_FALSE(local_reporter->fail_state());
+}
+
+TEST(ThreadCollisionTest, MTSynchedScopedRecursiveBookCriticalSectionTest) {
+  // Queue with a 5 seconds push execution time, hopefuly the two used threads
+  // in the test will enter the push at same time.
+  class NonThreadSafeQueue {
+   public:
+    explicit NonThreadSafeQueue(base::AsserterBase* asserter)
+        : push_pop_(asserter) { }
+
+    void push(int) {
+      DFAKE_SCOPED_RECURSIVE_LOCK(push_pop_);
+      bar();
+      PlatformThread::Sleep(5000);
+    }
+
+    int pop() {
+      DFAKE_SCOPED_RECURSIVE_LOCK(push_pop_);
+      return 0;
+    }
+
+    void bar() {
+      DFAKE_SCOPED_RECURSIVE_LOCK(push_pop_);
+    }
+
+   private:
+    DFAKE_MUTEX(push_pop_);
+
+    DISALLOW_COPY_AND_ASSIGN(NonThreadSafeQueue);
+  };
+
+  // This time the QueueUser class protects the non thread safe queue with
+  // a lock.
+  class QueueUser : public base::DelegateSimpleThread::Delegate {
+   public:
+    QueueUser(NonThreadSafeQueue& queue, Lock& lock)
+        : queue_(queue),
+          lock_(lock) {}
+
+    virtual void Run() {
+      {
+        AutoLock auto_lock(lock_);
+        queue_.push(0);
+      }
+      {
+        AutoLock auto_lock(lock_);
+        queue_.bar();
+      }
+      {
+        AutoLock auto_lock(lock_);
+        queue_.pop();
+      }
+    }
+   private:
+    NonThreadSafeQueue& queue_;
+    Lock& lock_;
+  };
+
+  AssertReporter* local_reporter = new AssertReporter();
+
+  NonThreadSafeQueue queue(local_reporter);
+
+  Lock lock;
+
+  QueueUser queue_user_a(queue, lock);
+  QueueUser queue_user_b(queue, lock);
+
+  base::DelegateSimpleThread thread_a(&queue_user_a, "queue_user_thread_a");
+  base::DelegateSimpleThread thread_b(&queue_user_b, "queue_user_thread_b");
+
+  thread_a.Start();
+  thread_b.Start();
+
+  thread_a.Join();
+  thread_b.Join();
+
+  EXPECT_FALSE(local_reporter->fail_state());
+}
diff --git a/chrome/browser/printing/print_view_manager.cc b/chrome/browser/printing/print_view_manager.cc
index 26ae627..9771389 100644
--- a/chrome/browser/printing/print_view_manager.cc
+++ b/chrome/browser/printing/print_view_manager.cc
@@ -29,6 +29,9 @@ PrintViewManager::PrintViewManager(WebContents& owner)
   memset(&print_params_, 0, sizeof(print_params_));
 }
 
+PrintViewManager::~PrintViewManager() {
+}
+
 void PrintViewManager::Destroy() {
   DisconnectFromCurrentPrintJob();
 }
diff --git a/chrome/browser/printing/print_view_manager.h b/chrome/browser/printing/print_view_manager.h
index b460f19..21226c4 100644
--- a/chrome/browser/printing/print_view_manager.h
+++ b/chrome/browser/printing/print_view_manager.h
@@ -24,6 +24,7 @@ class PrintViewManager : public NotificationObserver,
                          public PrintedPagesSource {
  public:
   PrintViewManager(WebContents& owner);
+  virtual ~PrintViewManager();
 
   // Destroys the "Print..." dialog, makes sure the pages are finished rendering
   // and release the print job.