Revert 213906 "base: Re-apply WeakPtr support for SequencedWorke..."

Speculative due to base_unittests failures.

> base: Re-apply WeakPtr support for SequencedWorkerPools, fixing deadlock
> 
> This reverts the revert found here: https://codereview.chromium.org/19882002
> 
> It also fixes the deadlock that caused the revert, reported here:
> http://crbug.com/261448
> 
> Patchset 1 is simply what was originally committed (and reverted). Subsequent patchsets show the fix of the deadlock problem.
> 
> BUG=165590
> TBR=darin
> 
> Review URL: https://chromiumcodereview.appspot.com/20163004

TBR=tommycli@chromium.org

Review URL: https://codereview.chromium.org/20771002

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

base/base.gyp

@@ -571,6 +571,7 @@
         'scoped_observer.h',
         'security_unittest.cc',
         'sequence_checker_unittest.cc',
+        'sequence_checker_impl_unittest.cc',
         'sha1_unittest.cc',
         'stl_util_unittest.cc',
         'strings/nullable_string16_unittest.cc',

base/memory/weak_ptr.cc

@@ -10,21 +10,21 @@ namespace internal {
 WeakReference::Flag::Flag() : is_valid_(true) {
   // Flags only become bound when checked for validity, or invalidated,
   // so that we can check that later validity/invalidation operations on
-  // the same Flag take place on the same sequenced thread.
-  sequence_checker_.DetachFromSequence();
+  // the same Flag take place on the same thread.
+  thread_checker_.DetachFromThread();
 }
 
 void WeakReference::Flag::Invalidate() {
   // The flag being invalidated with a single ref implies that there are no
   // weak pointers in existence. Allow deletion on other thread in this case.
-  DCHECK(sequence_checker_.CalledOnValidSequencedThread() || HasOneRef())
-      << "WeakPtrs must be invalidated on the same sequenced thread.";
+  DCHECK(thread_checker_.CalledOnValidThread() || HasOneRef())
+      << "WeakPtrs must be checked and invalidated on the same thread.";
   is_valid_ = false;
 }
 
 bool WeakReference::Flag::IsValid() const {
-  DCHECK(sequence_checker_.CalledOnValidSequencedThread())
-      << "WeakPtrs must be checked on the same sequenced thread.";
+  DCHECK(thread_checker_.CalledOnValidThread())
+      << "WeakPtrs must be checked and invalidated on the same thread.";
   return is_valid_;
 }
 

base/memory/weak_ptr.h

@@ -67,8 +67,8 @@
 #include "base/base_export.h"
 #include "base/logging.h"
 #include "base/memory/ref_counted.h"
-#include "base/sequence_checker.h"
 #include "base/template_util.h"
+#include "base/threading/thread_checker.h"
 
 namespace base {
 
@@ -91,14 +91,14 @@ class BASE_EXPORT WeakReference {
     bool IsValid() const;
 
     // Remove this when crbug.com/234964 is addressed.
-    void DetachFromThreadHack() { sequence_checker_.DetachFromSequence(); }
+    void DetachFromThreadHack() { thread_checker_.DetachFromThread(); }
 
    private:
     friend class base::RefCountedThreadSafe<Flag>;
 
     ~Flag();
 
-    SequenceChecker sequence_checker_;
+    ThreadChecker thread_checker_;
     bool is_valid_;
   };
 

base/message_loop/message_pump_io_ios.h

@@ -12,7 +12,6 @@
 #include "base/memory/weak_ptr.h"
 #include "base/message_loop/message_pump_mac.h"
 #include "base/observer_list.h"
-#include "base/threading/thread_checker.h"
 
 namespace base {
 

base/sequence_checker.h

@@ -28,11 +28,12 @@ class SequencedTaskRunner;
 // the right version for your build configuration.
 class SequenceCheckerDoNothing {
  public:
-  bool CalledOnValidSequencedThread() const {
+  bool CalledOnValidSequence() const {
     return true;
   }
 
-  void DetachFromSequence() {}
+  void ChangeSequence(
+      const scoped_refptr<SequencedTaskRunner>& sequenced_task_runner) {}
 };
 
 // SequenceChecker is a helper class used to help verify that some
@@ -43,6 +44,10 @@ class SequenceCheckerDoNothing {
 // Example:
 // class MyClass {
 //  public:
+//   explicit MyClass(
+//       const scoped_refptr<SequencedTaskRunner>& sequenced_task_runner)
+//       : sequence_checker_(sequenced_task_runner) {}
+//
 //   void Foo() {
 //     DCHECK(sequence_checker_.CalledOnValidSequence());
 //     ... (do stuff) ...
@@ -55,9 +60,16 @@ class SequenceCheckerDoNothing {
 // In Release mode, CalledOnValidSequence will always return true.
 #if ENABLE_SEQUENCE_CHECKER
 class SequenceChecker : public SequenceCheckerImpl {
+ public:
+  explicit SequenceChecker(
+      const scoped_refptr<SequencedTaskRunner>& sequenced_task_runner)
+      : SequenceCheckerImpl(sequenced_task_runner) {}
 };
 #else
 class SequenceChecker : public SequenceCheckerDoNothing {
+ public:
+  explicit SequenceChecker(
+      const scoped_refptr<SequencedTaskRunner>& sequenced_task_runner) {}
 };
 #endif  // ENABLE_SEQUENCE_CHECKER
 

base/sequence_checker_impl.cc

@@ -4,43 +4,28 @@
 
 #include "base/sequence_checker_impl.h"
 
+#include "base/sequenced_task_runner.h"
+
 namespace base {
 
-SequenceCheckerImpl::SequenceCheckerImpl()
-    : sequence_token_assigned_(false) {
-  AutoLock auto_lock(lock_);
-  EnsureSequenceTokenAssigned();
-}
+SequenceCheckerImpl::SequenceCheckerImpl(
+    const scoped_refptr<SequencedTaskRunner>& sequenced_task_runner)
+    : sequenced_task_runner_(sequenced_task_runner) {}
 
 SequenceCheckerImpl::~SequenceCheckerImpl() {}
 
-bool SequenceCheckerImpl::CalledOnValidSequencedThread() const {
+bool SequenceCheckerImpl::CalledOnValidSequence() const {
   AutoLock auto_lock(lock_);
-  EnsureSequenceTokenAssigned();
-
-  // If this thread is not associated with a SequencedWorkerPool,
-  // SequenceChecker behaves as a ThreadChecker. See header for details.
-  if (!sequence_token_.IsValid())
-    return thread_checker_.CalledOnValidThread();
-
-  return sequence_token_.Equals(
-      SequencedWorkerPool::GetSequenceTokenForCurrentThread());
+  return sequenced_task_runner_.get() ?
+      sequenced_task_runner_->RunsTasksOnCurrentThread() :
+      thread_checker_.CalledOnValidThread();
 }
 
-void SequenceCheckerImpl::DetachFromSequence() {
+void SequenceCheckerImpl::ChangeSequence(
+    const scoped_refptr<SequencedTaskRunner>& sequenced_task_runner) {
   AutoLock auto_lock(lock_);
+  sequenced_task_runner_ = sequenced_task_runner;
   thread_checker_.DetachFromThread();
-  sequence_token_assigned_ = false;
-  sequence_token_ = SequencedWorkerPool::SequenceToken();
-}
-
-void SequenceCheckerImpl::EnsureSequenceTokenAssigned() const {
-  lock_.AssertAcquired();
-  if (sequence_token_assigned_)
-    return;
-
-  sequence_token_assigned_ = true;
-  sequence_token_ = SequencedWorkerPool::GetSequenceTokenForCurrentThread();
 }
 
 }  // namespace base

base/sequence_checker_impl.h

@@ -7,42 +7,48 @@
 
 #include "base/base_export.h"
 #include "base/basictypes.h"
+#include "base/memory/ref_counted.h"
 #include "base/synchronization/lock.h"
-#include "base/threading/sequenced_worker_pool.h"
 #include "base/threading/thread_checker_impl.h"
 
 namespace base {
 
+class SequencedTaskRunner;
+
 // SequenceCheckerImpl is used to help verify that some methods of a
 // class are called in sequence -- that is, called from the same
 // SequencedTaskRunner. It is a generalization of ThreadChecker; in
-// particular, it behaves exactly like ThreadChecker if constructed
-// on a thread that is not part of a SequencedWorkerPool.
+// particular, it behaves exactly like ThreadChecker if its passed a
+// NULL SequencedTaskRunner.
 class BASE_EXPORT SequenceCheckerImpl {
  public:
-  SequenceCheckerImpl();
+  // |sequenced_task_runner| can be NULL.  In that case, this object
+  // behaves exactly like a ThreadChecker bound to the current thread,
+  // i.e.  CalledOnValidSequence() behaves like CalledOnValidThread().
+  explicit SequenceCheckerImpl(
+      const scoped_refptr<SequencedTaskRunner>& sequenced_task_runner);
   ~SequenceCheckerImpl();
 
-  // Returns whether the we are being called on the same sequence token
-  // as previous calls. If there is no associated sequence, then returns
-  // whether we are being called on the underlying ThreadChecker's thread.
-  bool CalledOnValidSequencedThread() const;
+  // Returns whether the we are being called on the underyling
+  // SequencedTaskRunner.  If we're not bound to a
+  // |sequenced_task_runner|, returns whether we are being called on
+  // the underlying ThreadChecker's thread.
+  bool CalledOnValidSequence() const;
 
-  // Unbinds the checker from the currently associated sequence. The
-  // checker will be re-bound on the next call to CalledOnValidSequence().
-  void DetachFromSequence();
+  // Changes the underyling SequencedTaskRunner.
+  // |sequenced_task_runner| can be NULL.  In that case, this object
+  // behaves exactly like a ThreadChecker that has been detached from
+  // its thread, i.e. we will be bound to the thread on which we next
+  // call CalledOnValidSequence().
+  void ChangeSequence(
+      const scoped_refptr<SequencedTaskRunner>& sequenced_task_runner);
 
  private:
-  void EnsureSequenceTokenAssigned() const;
-
   // Guards all variables below.
   mutable Lock lock_;
-
-  // Used if |sequence_token_| is not valid.
+  scoped_refptr<SequencedTaskRunner> sequenced_task_runner_;
+  // Used if |sequenced_task_runner_| is NULL.
   ThreadCheckerImpl thread_checker_;
-  mutable bool sequence_token_assigned_;
-
-  mutable SequencedWorkerPool::SequenceToken sequence_token_;
 
   DISALLOW_COPY_AND_ASSIGN(SequenceCheckerImpl);
 };